| Message ID | 201702282045.44470.cehoyos@ag.or.at |
|---|---|
| State | Superseded |
| Headers | show |
On Tue, Feb 28, 2017 at 08:45:44PM +0100, Carl Eugen Hoyos wrote: > Hi! > > Attached patch fixes ticket #6206 here. > > Please comment, Carl Eugen previous ffmpeg versions dont play these files [...]
2017-03-01 4:40 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>: > On Tue, Feb 28, 2017 at 08:45:44PM +0100, Carl Eugen Hoyos wrote: >> Hi! >> >> Attached patch fixes ticket #6206 here. >> >> Please comment, Carl Eugen > > previous ffmpeg versions dont play these files What do you suggest? Carl Eugen
On Wed, Mar 01, 2017 at 09:43:57AM +0100, Carl Eugen Hoyos wrote: > 2017-03-01 4:40 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>: > > On Tue, Feb 28, 2017 at 08:45:44PM +0100, Carl Eugen Hoyos wrote: > >> Hi! > >> > >> Attached patch fixes ticket #6206 here. > >> > >> Please comment, Carl Eugen > > > > previous ffmpeg versions dont play these files > > What do you suggest? support this at the demuxer side, backport such support then wait till its supported widely before switching the default in the muxer [...]
On Wed, Mar 1, 2017 at 1:07 PM, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Wed, Mar 01, 2017 at 09:43:57AM +0100, Carl Eugen Hoyos wrote: >> 2017-03-01 4:40 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>: >> > On Tue, Feb 28, 2017 at 08:45:44PM +0100, Carl Eugen Hoyos wrote: >> >> Hi! >> >> >> >> Attached patch fixes ticket #6206 here. >> >> >> >> Please comment, Carl Eugen >> > >> > previous ffmpeg versions dont play these files >> >> What do you suggest? > > support this at the demuxer side, backport such support then > wait till its supported widely before switching the default in the > muxer FFV1 didn't have any mapping in matroska before this, so what exactly is there to support? Also, format support is technically a feature, so back-porting seems ill-advised. - Hendrik
2017-03-01 13:18 GMT+01:00 Hendrik Leppkes <h.leppkes@gmail.com>:
> FFV1 didn't have any mapping in matroska before this
I believe it is safe to say that many people used ffv1 in mkv
and they would be surprised if their new files suddenly didn't
play in vlc.
Carl Eugen
On Wed, Mar 01, 2017 at 01:18:09PM +0100, Hendrik Leppkes wrote: > On Wed, Mar 1, 2017 at 1:07 PM, Michael Niedermayer > <michael@niedermayer.cc> wrote: > > On Wed, Mar 01, 2017 at 09:43:57AM +0100, Carl Eugen Hoyos wrote: > >> 2017-03-01 4:40 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>: > >> > On Tue, Feb 28, 2017 at 08:45:44PM +0100, Carl Eugen Hoyos wrote: > >> >> Hi! > >> >> > >> >> Attached patch fixes ticket #6206 here. > >> >> > >> >> Please comment, Carl Eugen > >> > > >> > previous ffmpeg versions dont play these files > >> > >> What do you suggest? > > > > support this at the demuxer side, backport such support then > > wait till its supported widely before switching the default in the > > muxer > > FFV1 didn't have any mapping in matroska before this, so what exactly > is there to support? mkv supports all avi identifers, i assume that is what was used before > Also, format support is technically a feature, so back-porting seems > ill-advised. I think backporting format identifers, fourccs and such is safe. And this one should be back ported [...]
Michael Niedermayer wrote: >> FFV1 didn't have any mapping in matroska before this, so >> what exactly is there to support? > >mkv supports all avi identifers, i assume that is what was >used before > >> Also, format support is technically a feature, so back- >> porting seems ill-advised. > >I think backporting format identifers, fourccs and such is >safe. >And this one should be back ported I fully agree that this one should be back-ported. Best regards, Reto
On Wed, 1 Mar 2017 13:07:12 +0100 Michael Niedermayer <michael@niedermayer.cc> wrote: > On Wed, Mar 01, 2017 at 09:43:57AM +0100, Carl Eugen Hoyos wrote: > > 2017-03-01 4:40 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>: > > > On Tue, Feb 28, 2017 at 08:45:44PM +0100, Carl Eugen Hoyos wrote: > > >> Hi! > > >> > > >> Attached patch fixes ticket #6206 here. > > >> > > >> Please comment, Carl Eugen > > > > > > previous ffmpeg versions dont play these files > > > > What do you suggest? > > support this at the demuxer side, backport such support then > wait till its supported widely before switching the default in the > muxer So if I understand this right: - the patch will change the muxer output to use V_FFV1 instead of using avi-compat muxing - you demand that we wait with this because older ffmpeg versions would choke on the new version So what about all the other muxers that exist in the wild and that will break on this file? Will you fix them too? I think this suggestion of waiting until FFmpeg can eat the new output and only then changing the output is possibly slightly self-centric, and also assumes everyone is going to update their copy of FFmpeg. If you're afraid that this change could break demuxers, you probably shouldn't change it at all.
On Wed, Mar 01, 2017 at 01:36:26PM +0100, wm4 wrote: > On Wed, 1 Mar 2017 13:07:12 +0100 > Michael Niedermayer <michael@niedermayer.cc> wrote: > > > On Wed, Mar 01, 2017 at 09:43:57AM +0100, Carl Eugen Hoyos wrote: > > > 2017-03-01 4:40 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>: > > > > On Tue, Feb 28, 2017 at 08:45:44PM +0100, Carl Eugen Hoyos wrote: > > > >> Hi! > > > >> > > > >> Attached patch fixes ticket #6206 here. > > > >> > > > >> Please comment, Carl Eugen > > > > > > > > previous ffmpeg versions dont play these files > > > > > > What do you suggest? > > > > support this at the demuxer side, backport such support then > > wait till its supported widely before switching the default in the > > muxer > > So if I understand this right: > - the patch will change the muxer output to use V_FFV1 instead of using > avi-compat muxing > - you demand that we wait with this because older ffmpeg versions would > choke on the new version > > So what about all the other muxers that exist in the wild and that will > break on this file? Will you fix them too? I think this suggestion of > waiting until FFmpeg can eat the new output and only then changing the > output is possibly slightly self-centric, and also assumes everyone is > going to update their copy of FFmpeg. You seem to be reading things in the mail that it didnt say. A reply to "What do you suggest?" is a suggestion not a demand and The suggestion does not refer to FFmpeg and it was not intended to imply to refer to FFmpeg, especially the "wait till its supported widely" was intended to refer to all software. Its bad if we generate files by default noone can play back I dont know how widespread V_FFV1 support is but i assume its not widespread yet at all [...]
On Wed, 1 Mar 2017 14:26:34 +0100 Michael Niedermayer <michael@niedermayer.cc> wrote: > On Wed, Mar 01, 2017 at 01:36:26PM +0100, wm4 wrote: > > On Wed, 1 Mar 2017 13:07:12 +0100 > > Michael Niedermayer <michael@niedermayer.cc> wrote: > > > > > On Wed, Mar 01, 2017 at 09:43:57AM +0100, Carl Eugen Hoyos wrote: > > > > 2017-03-01 4:40 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>: > > > > > On Tue, Feb 28, 2017 at 08:45:44PM +0100, Carl Eugen Hoyos wrote: > > > > >> Hi! > > > > >> > > > > >> Attached patch fixes ticket #6206 here. > > > > >> > > > > >> Please comment, Carl Eugen > > > > > > > > > > previous ffmpeg versions dont play these files > > > > > > > > What do you suggest? > > > > > > support this at the demuxer side, backport such support then > > > wait till its supported widely before switching the default in the > > > muxer > > > > So if I understand this right: > > - the patch will change the muxer output to use V_FFV1 instead of using > > avi-compat muxing > > - you demand that we wait with this because older ffmpeg versions would > > choke on the new version > > > > So what about all the other muxers that exist in the wild and that will > > break on this file? Will you fix them too? I think this suggestion of > > waiting until FFmpeg can eat the new output and only then changing the > > output is possibly slightly self-centric, and also assumes everyone is > > going to update their copy of FFmpeg. > > You seem to be reading things in the mail that it didnt say. Well, I didn't read the following (what's quoted) into it. > A reply to "What do you suggest?" is a suggestion not a demand > and > The suggestion does not refer to FFmpeg and it was not intended to > imply to refer to FFmpeg, especially the > "wait till its supported widely" was intended to refer to all software. Indeed, backporting support FFmpeg will not help that much. So we either live with the fact that this change will break tons of consumers, or we just don't make it. > Its bad if we generate files by default noone can play back > I dont know how widespread V_FFV1 support is but i assume its not > widespread yet at all Agreed.
Le 01/03/2017 à 09:43, Carl Eugen Hoyos a écrit : > 2017-03-01 4:40 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>: >> On Tue, Feb 28, 2017 at 08:45:44PM +0100, Carl Eugen Hoyos wrote: >>> Hi! >>> >>> Attached patch fixes ticket #6206 here. >>> >>> Please comment, Carl Eugen >> previous ffmpeg versions dont play these files > What do you suggest? My suggestion: - support by the demuxer - for the muxer, support only if a specific explicit option is set, default stays AVI compatibility layer (no break in playback by old players). then in 12 (or 24, or 36 months), the time that new FFmpeg versions propagate everywhere (we also open tickets for other demuxers in order to get the support of such files), the default is changed to V_FFV1. Sorry I don't know enough FFmpeg for doing the corresponding patch myself.
On Wed, 1 Mar 2017 15:00:27 +0100 Jerome Martinez <jerome@mediaarea.net> wrote: > Le 01/03/2017 à 09:43, Carl Eugen Hoyos a écrit : > > 2017-03-01 4:40 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>: > >> On Tue, Feb 28, 2017 at 08:45:44PM +0100, Carl Eugen Hoyos wrote: > >>> Hi! > >>> > >>> Attached patch fixes ticket #6206 here. > >>> > >>> Please comment, Carl Eugen > >> previous ffmpeg versions dont play these files > > What do you suggest? > > My suggestion: > - support by the demuxer > - for the muxer, support only if a specific explicit option is set, > default stays AVI compatibility layer (no break in playback by old players). Sounds like a good idea. > then in 12 (or 24, or 36 months), the time that new FFmpeg versions > propagate everywhere (we also open tickets for other demuxers in order > to get the support of such files), the default is changed to V_FFV1. There are other demuxers than the FFmpeg one. Actually I suspect most trouble will be with ancient mkvmerge versions floating around and that everyone seems to want to use. > Sorry I don't know enough FFmpeg for doing the corresponding patch myself. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Le 01/03/2017 à 15:51, wm4 a écrit : > On Wed, 1 Mar 2017 15:00:27 +0100 > Jerome Martinez <jerome@mediaarea.net> wrote: > > [...] >> then in 12 (or 24, or 36 months), the time that new FFmpeg versions >> propagate everywhere (we also open tickets for other demuxers in order >> to get the support of such files), the default is changed to V_FFV1. > There are other demuxers than the FFmpeg one. Actually I suspect most > trouble will be with ancient mkvmerge versions floating around and that > everyone seems to want to use. I think the people loving ancient mkvmerge versions are not the ones using lossless formats, so it is not blocking from my point of view.
On 2/28/2017 4:45 PM, Carl Eugen Hoyos wrote: > Hi! > > Attached patch fixes ticket #6206 here. > > Please comment, Carl Eugen This doesn't consider the CodecPrivate contents for FFV1 v3 and above. It's only valid for v1 and v2. > > > 0001-lavf-matroska-Support-new-codec-id-V_FFV1-for-FFV1.patch > > > From 10caa3898fa76f5a76dce7e3795a7a0d475870c1 Mon Sep 17 00:00:00 2001 > From: Carl Eugen Hoyos <cehoyos@ag.or.at> > Date: Tue, 28 Feb 2017 20:36:12 +0100 > Subject: [PATCH] lavf/matroska: Support new codec id V_FFV1 for FFV1. > > Fixes ticket #6206. > --- > libavformat/matroska.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/libavformat/matroska.c b/libavformat/matroska.c > index fda96fb..7905fd1 100644 > --- a/libavformat/matroska.c > +++ b/libavformat/matroska.c > @@ -77,6 +77,7 @@ const CodecTags ff_mkv_codec_tags[]={ > {"S_HDMV/TEXTST" , AV_CODEC_ID_HDMV_TEXT_SUBTITLE}, > > {"V_DIRAC" , AV_CODEC_ID_DIRAC}, > + {"V_FFV1" , AV_CODEC_ID_FFV1}, > {"V_MJPEG" , AV_CODEC_ID_MJPEG}, > {"V_MPEG1" , AV_CODEC_ID_MPEG1VIDEO}, > {"V_MPEG2" , AV_CODEC_ID_MPEG2VIDEO}, > -- 1.7.10.4 > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
2017-03-01 17:56 GMT+01:00 James Almer <jamrial@gmail.com>: > On 2/28/2017 4:45 PM, Carl Eugen Hoyos wrote: >> Hi! >> >> Attached patch fixes ticket #6206 here. >> >> Please comment, Carl Eugen > > This doesn't consider the CodecPrivate contents for FFV1 v3 > and above. It's only valid for v1 and v2. Is this comment meant for the muxer or the demuxer part? How can I reproduce the issue you see? For me, everything works as specified for default strictness. Carl Eugen
On 3/1/2017 7:18 PM, Carl Eugen Hoyos wrote: > 2017-03-01 17:56 GMT+01:00 James Almer <jamrial@gmail.com>: >> On 2/28/2017 4:45 PM, Carl Eugen Hoyos wrote: >>> Hi! >>> >>> Attached patch fixes ticket #6206 here. >>> >>> Please comment, Carl Eugen >> >> This doesn't consider the CodecPrivate contents for FFV1 v3 >> and above. It's only valid for v1 and v2. > > Is this comment meant for the muxer or the demuxer part? > > How can I reproduce the issue you see? > For me, everything works as specified for default strictness. Ah, i see there's generic code to read and write CodecPrivate elements to and from raw extradata for native codecids where no special handling is required. In any case, the spec says "For FFV1 versions 2 or less, Private Data SHOULD NOT be written." The ffv1 encoder creates extradata for v2 and newer, so the muxer should have a custom case for ffv1 in order to detect v2 streams and avoid writing said extradata. > > Carl Eugen > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
2017-03-02 0:26 GMT+01:00 James Almer <jamrial@gmail.com>: > On 3/1/2017 7:18 PM, Carl Eugen Hoyos wrote: >>> This doesn't consider the CodecPrivate contents for FFV1 v3 >>> and above. It's only valid for v1 and v2. >> >> Is this comment meant for the muxer or the demuxer part? >> >> How can I reproduce the issue you see? >> For me, everything works as specified for default strictness. > > Ah, i see there's generic code to read and write CodecPrivate elements > to and from raw extradata for native codecids where no special handling > is required. > > In any case, the spec says > > "For FFV1 versions 2 or less, Private Data SHOULD NOT be written." > > The ffv1 encoder creates extradata for v2 and newer, so the muxer > should have a custom case for ffv1 in order to detect v2 streams and > avoid writing said extradata. Since this behaviour can only be produced with non-strict, I don't think it is of much relevance. Anyway, this is removed from the current patch. Carl Eugen
Le primidi 11 ventôse, an CCXXV, James Almer a écrit : > Ah, i see there's generic code to read and write CodecPrivate elements > to and from raw extradata for native codecids where no special handling > is required. > > In any case, the spec says > > "For FFV1 versions 2 or less, Private Data SHOULD NOT be written." > > The ffv1 encoder creates extradata for v2 and newer, so the muxer > should have a custom case for ffv1 in order to detect v2 streams and > avoid writing said extradata. I have not looked myself at the situation, I only read what you wrote her. According to it, it seems here the FFV1 encoder is violating the specification and needs to be fixed. The way I see it, we should try to make muxers as generic and codec-independent as possible. Most cases where we have codec-specific code in muxers are when someone bungled the design, usually an industrial committee (but do not ask me how the Ogg people managed to make something even worse). There are several ways to interpret the quote of the spec above: 1a: Encoders SHOULD NOT return extradata to the application. 1b: Applications SHOULD discard extradata returned by the encoder. 2a: Applications SHOULD NOT give extradata to the muxer. 2b: Muxers SHOULD discard extradata given by the application. IMHO, the only reasonable one is 1a; it achieves automatically 2a and makes 1b and 2b completely unnecessary. Regards,
On 03/01/2017 03:00 PM, Jerome Martinez wrote: > My suggestion: > - support by the demuxer > - for the muxer, support only if a specific explicit option is set, > default stays AVI compatibility layer (no break in playback by old > players). This sounds like a very practical approach to me: New files can be played, but one is not accidentially/surprisingly creating incompatible files unless they explicitly choose to do so. :) Regards, Pb
On 3/2/2017 6:28 AM, Nicolas George wrote: > Le primidi 11 ventôse, an CCXXV, James Almer a écrit : >> Ah, i see there's generic code to read and write CodecPrivate elements >> to and from raw extradata for native codecids where no special handling >> is required. >> >> In any case, the spec says >> >> "For FFV1 versions 2 or less, Private Data SHOULD NOT be written." >> >> The ffv1 encoder creates extradata for v2 and newer, so the muxer >> should have a custom case for ffv1 in order to detect v2 streams and >> avoid writing said extradata. > > I have not looked myself at the situation, I only read what you wrote > her. According to it, it seems here the FFV1 encoder is violating the > specification and needs to be fixed. I can't say if the encoder exporting extradata for version 2 is right or wrong, as muxers or bsfs could still use it for whatever reason, but at least according to the draft ffv1 spec by Michael, it's to be stored at the container level *only* on versions 3 and above. https://tools.ietf.org/html/draft-niedermayer-cellar-ffv1-01#section-4.1 Michael probably knows best if version 2 is supposed to export this extradata or not, and if what's currently exported is correct to being with or not (I see for example that the encoder stores a crc value for all versions that the decoder in turn purposely skips for version 2). > > The way I see it, we should try to make muxers as generic and > codec-independent as possible. Most cases where we have codec-specific > code in muxers are when someone bungled the design, usually an > industrial committee (but do not ask me how the Ogg people managed to > make something even worse). > > There are several ways to interpret the quote of the spec above: > > 1a: Encoders SHOULD NOT return extradata to the application. > > 1b: Applications SHOULD discard extradata returned by the encoder. > > 2a: Applications SHOULD NOT give extradata to the muxer. > > 2b: Muxers SHOULD discard extradata given by the application. > > IMHO, the only reasonable one is 1a; it achieves automatically 2a and > makes 1b and 2b completely unnecessary. The quote is from the container spec, so it defines muxing and demuxing guidelines only. If anything, the quote is closest to 2b. https://github.com/Matroska-Org/matroska-specification/blob/master/codec_specs.md#v_ffv1 > > Regards, > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On 02.03.2017 15:22, James Almer wrote: > On 3/2/2017 6:28 AM, Nicolas George wrote: >> Le primidi 11 ventôse, an CCXXV, James Almer a écrit : >>> Ah, i see there's generic code to read and write CodecPrivate elements >>> to and from raw extradata for native codecids where no special handling >>> is required. >>> >>> In any case, the spec says >>> >>> "For FFV1 versions 2 or less, Private Data SHOULD NOT be written." >>> >>> The ffv1 encoder creates extradata for v2 and newer, so the muxer >>> should have a custom case for ffv1 in order to detect v2 streams and >>> avoid writing said extradata. >> >> I have not looked myself at the situation, I only read what you wrote >> her. According to it, it seems here the FFV1 encoder is violating the >> specification and needs to be fixed. > > I can't say if the encoder exporting extradata for version 2 is right or > wrong, as muxers or bsfs could still use it for whatever reason, but at > least according to the draft ffv1 spec by Michael, it's to be stored at > the container level *only* on versions 3 and above. > https://tools.ietf.org/html/draft-niedermayer-cellar-ffv1-01#section-4.1 The IETF draft explicitly excludes to specify FFV1 version 2: https://tools.ietf.org/html/draft-niedermayer-cellar-ffv1-01#section-4.8.1 IIRC there have been some edits to remove references to version 2 in the IETF draft. In the older document at http://www.ffmpeg.org/%7Emichael/ffv1.html#configuration-record the section contains "version >= 2". Regards, Tobias
On 3/2/2017 5:44 AM, Carl Eugen Hoyos wrote: > 2017-03-02 0:26 GMT+01:00 James Almer <jamrial@gmail.com>: >> On 3/1/2017 7:18 PM, Carl Eugen Hoyos wrote: > >>>> This doesn't consider the CodecPrivate contents for FFV1 v3 >>>> and above. It's only valid for v1 and v2. >>> >>> Is this comment meant for the muxer or the demuxer part? >>> >>> How can I reproduce the issue you see? >>> For me, everything works as specified for default strictness. >> >> Ah, i see there's generic code to read and write CodecPrivate elements >> to and from raw extradata for native codecids where no special handling >> is required. >> >> In any case, the spec says >> >> "For FFV1 versions 2 or less, Private Data SHOULD NOT be written." >> >> The ffv1 encoder creates extradata for v2 and newer, so the muxer >> should have a custom case for ffv1 in order to detect v2 streams and >> avoid writing said extradata. > > Since this behaviour can only be produced with non-strict, I don't > think it is of much relevance. Strict experimental is only needed at the encoder level, not the muxer level. This behavior can be reproduced just fine by remuxing existing ffv1 v2 files into Matroska using the default strictness. For that matter, Matroska (in V_MS/VFW/FOURCC mode), Nut and AVI probably shouldn't store v2 extradata either as mentioned by the spec, but they currently seem to do it. > Anyway, this is removed from the current patch. Fair enough. > > Carl Eugen > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On 3/2/2017 11:47 AM, Tobias Rapp wrote: > On 02.03.2017 15:22, James Almer wrote: >> On 3/2/2017 6:28 AM, Nicolas George wrote: >>> Le primidi 11 ventôse, an CCXXV, James Almer a écrit : >>>> Ah, i see there's generic code to read and write CodecPrivate elements >>>> to and from raw extradata for native codecids where no special handling >>>> is required. >>>> >>>> In any case, the spec says >>>> >>>> "For FFV1 versions 2 or less, Private Data SHOULD NOT be written." >>>> >>>> The ffv1 encoder creates extradata for v2 and newer, so the muxer >>>> should have a custom case for ffv1 in order to detect v2 streams and >>>> avoid writing said extradata. >>> >>> I have not looked myself at the situation, I only read what you wrote >>> her. According to it, it seems here the FFV1 encoder is violating the >>> specification and needs to be fixed. >> >> I can't say if the encoder exporting extradata for version 2 is right or >> wrong, as muxers or bsfs could still use it for whatever reason, but at >> least according to the draft ffv1 spec by Michael, it's to be stored at >> the container level *only* on versions 3 and above. >> https://tools.ietf.org/html/draft-niedermayer-cellar-ffv1-01#section-4.1 > > The IETF draft explicitly excludes to specify FFV1 version 2: > https://tools.ietf.org/html/draft-niedermayer-cellar-ffv1-01#section-4.8.1 > > IIRC there have been some edits to remove references to version 2 in the IETF draft. In the older document at http://www.ffmpeg.org/%7Emichael/ffv1.html#configuration-record the section contains "version >= 2". > > Regards, > Tobias Well, that makes things a bit more complex. Maybe the Matroska spec should follow and also avoid mentioning v2? That way we could just pretend those files don't exist and not bother trying to drop their extradata during muxing.
> On Mar 2, 2017, at 9:59 AM, James Almer <jamrial@gmail.com> wrote: > > On 3/2/2017 11:47 AM, Tobias Rapp wrote: >> On 02.03.2017 15:22, James Almer wrote: >>> On 3/2/2017 6:28 AM, Nicolas George wrote: >>>> Le primidi 11 ventôse, an CCXXV, James Almer a écrit : >>>>> Ah, i see there's generic code to read and write CodecPrivate elements >>>>> to and from raw extradata for native codecids where no special handling >>>>> is required. >>>>> >>>>> In any case, the spec says >>>>> >>>>> "For FFV1 versions 2 or less, Private Data SHOULD NOT be written." >>>>> >>>>> The ffv1 encoder creates extradata for v2 and newer, so the muxer >>>>> should have a custom case for ffv1 in order to detect v2 streams and >>>>> avoid writing said extradata. >>>> >>>> I have not looked myself at the situation, I only read what you wrote >>>> her. According to it, it seems here the FFV1 encoder is violating the >>>> specification and needs to be fixed. >>> >>> I can't say if the encoder exporting extradata for version 2 is right or >>> wrong, as muxers or bsfs could still use it for whatever reason, but at >>> least according to the draft ffv1 spec by Michael, it's to be stored at >>> the container level *only* on versions 3 and above. >>> https://tools.ietf.org/html/draft-niedermayer-cellar-ffv1-01#section-4.1 >> >> The IETF draft explicitly excludes to specify FFV1 version 2: >> https://tools.ietf.org/html/draft-niedermayer-cellar-ffv1-01#section-4.8.1 >> >> IIRC there have been some edits to remove references to version 2 in the IETF draft. In the older document at http://www.ffmpeg.org/%7Emichael/ffv1.html#configuration-record the section contains "version >= 2". >> >> Regards, >> Tobias > > Well, that makes things a bit more complex. > Maybe the Matroska spec should follow and also avoid mentioning v2? > That way we could just pretend those files don't exist and not bother > trying to drop their extradata during muxing. I agree. In the FFV1 spec, it is careful to say that it intentionally does not describe FFV1 version 2. I will submit a patch to the Matroska specification to avoid using relative language based on version 2. Dave Rice
On Thu, Mar 02, 2017 at 04:43:27PM -0500, Dave Rice wrote: > > > On Mar 2, 2017, at 9:59 AM, James Almer <jamrial@gmail.com> wrote: > > > > On 3/2/2017 11:47 AM, Tobias Rapp wrote: > >> On 02.03.2017 15:22, James Almer wrote: > >>> On 3/2/2017 6:28 AM, Nicolas George wrote: > >>>> Le primidi 11 ventôse, an CCXXV, James Almer a écrit : > >>>>> Ah, i see there's generic code to read and write CodecPrivate elements > >>>>> to and from raw extradata for native codecids where no special handling > >>>>> is required. > >>>>> > >>>>> In any case, the spec says > >>>>> > >>>>> "For FFV1 versions 2 or less, Private Data SHOULD NOT be written." > >>>>> > >>>>> The ffv1 encoder creates extradata for v2 and newer, so the muxer > >>>>> should have a custom case for ffv1 in order to detect v2 streams and > >>>>> avoid writing said extradata. > >>>> > >>>> I have not looked myself at the situation, I only read what you wrote > >>>> her. According to it, it seems here the FFV1 encoder is violating the > >>>> specification and needs to be fixed. > >>> > >>> I can't say if the encoder exporting extradata for version 2 is right or > >>> wrong, as muxers or bsfs could still use it for whatever reason, but at > >>> least according to the draft ffv1 spec by Michael, it's to be stored at > >>> the container level *only* on versions 3 and above. > >>> https://tools.ietf.org/html/draft-niedermayer-cellar-ffv1-01#section-4.1 > >> > >> The IETF draft explicitly excludes to specify FFV1 version 2: > >> https://tools.ietf.org/html/draft-niedermayer-cellar-ffv1-01#section-4.8.1 > >> > >> IIRC there have been some edits to remove references to version 2 in the IETF draft. In the older document at http://www.ffmpeg.org/%7Emichael/ffv1.html#configuration-record the section contains "version >= 2". > >> > >> Regards, > >> Tobias > > > > Well, that makes things a bit more complex. > > Maybe the Matroska spec should follow and also avoid mentioning v2? > > That way we could just pretend those files don't exist and not bother > > trying to drop their extradata during muxing. > > I agree. In the FFV1 spec, it is careful to say that it intentionally does not describe FFV1 version 2. I will submit a patch to the Matroska specification to avoid using relative language based on version 2. agree too and thx also anyone wanting to know about version 2, the best reference is probably what the source does [...]
diff --git a/libavformat/matroska.c b/libavformat/matroska.c index fda96fb..7905fd1 100644 --- a/libavformat/matroska.c +++ b/libavformat/matroska.c @@ -77,6 +77,7 @@ const CodecTags ff_mkv_codec_tags[]={ {"S_HDMV/TEXTST" , AV_CODEC_ID_HDMV_TEXT_SUBTITLE}, {"V_DIRAC" , AV_CODEC_ID_DIRAC}, + {"V_FFV1" , AV_CODEC_ID_FFV1}, {"V_MJPEG" , AV_CODEC_ID_MJPEG}, {"V_MPEG1" , AV_CODEC_ID_MPEG1VIDEO}, {"V_MPEG2" , AV_CODEC_ID_MPEG2VIDEO},
Hi! Attached patch fixes ticket #6206 here. Please comment, Carl Eugen From 10caa3898fa76f5a76dce7e3795a7a0d475870c1 Mon Sep 17 00:00:00 2001 From: Carl Eugen Hoyos <cehoyos@ag.or.at> Date: Tue, 28 Feb 2017 20:36:12 +0100 Subject: [PATCH] lavf/matroska: Support new codec id V_FFV1 for FFV1. Fixes ticket #6206. --- libavformat/matroska.c | 1 + 1 file changed, 1 insertion(+)