| Message ID | 1c54159a-1309-2d12-ec89-e6f5d35c141e@mediaarea.net |
|---|---|
| State | Superseded |
| Headers | show |
On Fri, Jan 05, 2018 at 10:26:42PM +0100, Jerome Martinez wrote: > On 05/01/2018 16:14, Tobias Rapp wrote: > >On 05.01.2018 11:18, Jerome Martinez wrote: > >>0001-FFV1-make-RGB48-support-as-non-experimental.patch > >> > >>From cd1bfe68a1a809700f25e593ac21ca3876d265ad Mon Sep 17 00:00:00 2001 > >>From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Martinez?= <jerome@mediaarea.net> > >>Date: Fri, 5 Jan 2018 11:09:01 +0100 > >>Subject: [PATCH] FFV1: make RGB48 support as non-experimental > > > >make => mark > > I reused a commit text from history, both words words were in it. > Anyway, attached is the updated .patch with the suggested subject line. > > > > >BTW: common subject line format would be something like "avcodec/ffv1enc: > >mark RGB48 support as non-experimental" > > ffv1enc.c | 4 ---- > 1 file changed, 4 deletions(-) > acfc60c913b311b148f2eeef2d2d6ea9e37afcf7 0001-avcodec-ffv1enc-mark-RGB48-support-as-non-experiment.patch > From 303f63fb7e6172fdb7de66da1f8a4006b79a535f Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Martinez?= <jerome@mediaarea.net> > Date: Fri, 5 Jan 2018 11:09:01 +0100 > Subject: [PATCH] avcodec/ffv1enc: mark RGB48 support as non-experimental > > Resulting bitstream was tested with a conformance checker > using the last draft of FFV1 specifications. But has the way this is stored been optimized ? Once its marked as non exerimental all future decoders must support the exact way. It can no longer then be changed, so we need to be really sure the design is optimal first. Are we ? who has checked alternatives? what where the reasons why the alternatives were not choosen? for example consider get_context(), what it does with >8bit may or may not be optimal iam interrested to work on that in fact, ive a quite long and growing list of other volunteer jobs to do though ... [...]
On 06/01/2018 02:05, Michael Niedermayer wrote: > >> ffv1enc.c | 4 ---- >> 1 file changed, 4 deletions(-) >> acfc60c913b311b148f2eeef2d2d6ea9e37afcf7 0001-avcodec-ffv1enc-mark-RGB48-support-as-non-experiment.patch >> From 303f63fb7e6172fdb7de66da1f8a4006b79a535f Mon Sep 17 00:00:00 2001 >> From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Martinez?= <jerome@mediaarea.net> >> Date: Fri, 5 Jan 2018 11:09:01 +0100 >> Subject: [PATCH] avcodec/ffv1enc: mark RGB48 support as non-experimental >> >> Resulting bitstream was tested with a conformance checker >> using the last draft of FFV1 specifications. > But has the way this is stored been optimized ? > > Once its marked as non exerimental all future decoders must support the exact > way. Although this is considered experimental in the encoder, the implementation adheres to the description in the specification. The bitstream specification does not provide a bitdepth limit so with the current draft of the specification, another FFV1 encoder could already encode 16-bit (and more) content. The work on the specification has been careful to not break compatibility with former drafts and at this point the FFV1 bitstream specification for versions 0, 1, and 3 should be considered already non-experimental for all bit depths. All current decoders should already support the exact way it is currently specified. To make a change in the specification at this point would have cascading consequences as we’d have to retract the part of the specification which states that micro_version 4 of version 3 is the first stable variant. Worse, it is impossible to indicate a bitstream change in FFV1 version 1, which permits RGB 16-bit content too, so it would be difficult for a decoder to detect a bitstream not conforming to the bitstream created by the current version of FFmpeg encoder. > It can no longer then be changed, so we need to be really sure the design > is optimal first. > Are we ? > who has checked alternatives? what where the reasons why the alternatives > were not choosen? > for example consider get_context(), what it does with >8bit may or may not > be optimal > iam interrested to work on that in fact, ive a quite long and growing list > of other volunteer jobs to do though ... bitdepths >8bit have been well-used for years since many of them have long been marked as non-experimental (for instance 10bit is frequently used with lossless encoding of broadcast media and video from analog tape sources). In my opinion get_context() is specified for all bitdpeths and non-experimental for FFV1 versions 0, 1, and 3 by the specification work and it should not be changed in these versions. For the encoder there may still be an opportunity to optimize while continuing to conform to the FFV1 versions 0, 1, and 3 bitstream specification, even if the encoder marks RGB48 as non-experimental. Additionally FFV1 version 4 or later could consider further optimization requesting a change in the FFV1 bitstream as version 4 has no stable micro_version and the entire version is in an experimental status.
On Sat, Jan 06, 2018 at 04:54:18PM +0100, Jerome Martinez wrote: > On 06/01/2018 02:05, Michael Niedermayer wrote: > > > >> ffv1enc.c | 4 ---- > >> 1 file changed, 4 deletions(-) > >>acfc60c913b311b148f2eeef2d2d6ea9e37afcf7 0001-avcodec-ffv1enc-mark-RGB48-support-as-non-experiment.patch > >> From 303f63fb7e6172fdb7de66da1f8a4006b79a535f Mon Sep 17 00:00:00 2001 > >>From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Martinez?= <jerome@mediaarea.net> > >>Date: Fri, 5 Jan 2018 11:09:01 +0100 > >>Subject: [PATCH] avcodec/ffv1enc: mark RGB48 support as non-experimental > >> > >>Resulting bitstream was tested with a conformance checker > >>using the last draft of FFV1 specifications. > >But has the way this is stored been optimized ? > > > >Once its marked as non exerimental all future decoders must support the exact > >way. > > Although this is considered experimental in the encoder, the implementation > adheres to the description in the specification. The bitstream specification > does not provide a bitdepth limit so with the current draft of the > specification, another FFV1 encoder could already encode 16-bit (and more) > content. The work on the specification has been careful to not break > compatibility with former drafts and at this point the FFV1 bitstream > specification for versions 0, 1, and 3 should be considered already > non-experimental for all bit depths. All current decoders should already > support the exact way it is currently specified. > > To make a change in the specification at this point would have cascading > consequences as we’d have to retract the part of the specification which > states that micro_version 4 of version 3 is the first stable variant. Worse, > it is impossible to indicate a bitstream change in FFV1 version 1, which > permits RGB 16-bit content too, so it would be difficult for a decoder to > detect a bitstream not conforming to the bitstream created by the current > version of FFmpeg encoder. Are you not making this look alot more complex than it is ? Or maybe we talk about slightly different things here with the next version we can introduce any method of storing 16bit or 9-15 bit and we certainly do not support in the implementation all possible bit depths From what i remember I had always wanted to improve the way that more than 8bit is handled, not just 16. Although 16 is a bit special Consider this: If we improve get_context() in the next version for >8bit we still have to support 9-15 bit with the old definition if we now declare 16bit non experimental then we also must support that with an old get_context() in the decoder. the 16bit path is seperate from the lower bit depth. So this is an Additional codepath that we have to carry in the future isnt it smarter now that if we want to improve get_context() that we dont now extend what can be generated with the current get_context ? or are such current get_context() style files already widespread ? if so then its probably best to accept it and keep supporting it > > > > > It can no longer then be changed, so we need to be really sure the design > >is optimal first. > >Are we ? > >who has checked alternatives? what where the reasons why the alternatives > >were not choosen? > >for example consider get_context(), what it does with >8bit may or may not > >be optimal > >iam interrested to work on that in fact, ive a quite long and growing list > >of other volunteer jobs to do though ... > > bitdepths >8bit have been well-used for years since many of them have long > been marked as non-experimental (for instance 10bit is frequently used with > lossless encoding of broadcast media and video from analog tape sources). In > my opinion get_context() is specified for all bitdpeths and non-experimental > for FFV1 versions 0, 1, and 3 by the specification work and it should not be > changed in these versions. what get_context does was designed for 8bit, it should still be good enough for 10bit and maybe 12 but as the bit depth gets larger i suspect get_context becomes less optimal and there is more potential to improve compression by changing it. > > For the encoder there may still be an opportunity to optimize while > continuing to conform to the FFV1 versions 0, 1, and 3 bitstream > specification, even if the encoder marks RGB48 as non-experimental. > Additionally FFV1 version 4 or later could consider further optimization > requesting a change in the FFV1 bitstream as version 4 has no stable > micro_version and the entire version is in an experimental status. > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On 06/01/2018 23:21, Michael Niedermayer wrote: > On Sat, Jan 06, 2018 at 04:54:18PM +0100, Jerome Martinez wrote: >> On 06/01/2018 02:05, Michael Niedermayer wrote: >>>> ffv1enc.c | 4 ---- >>>> 1 file changed, 4 deletions(-) >>>> acfc60c913b311b148f2eeef2d2d6ea9e37afcf7 0001-avcodec-ffv1enc-mark-RGB48-support-as-non-experiment.patch >>>> From 303f63fb7e6172fdb7de66da1f8a4006b79a535f Mon Sep 17 00:00:00 2001 >>>> From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Martinez?= <jerome@mediaarea.net> >>>> Date: Fri, 5 Jan 2018 11:09:01 +0100 >>>> Subject: [PATCH] avcodec/ffv1enc: mark RGB48 support as non-experimental >>>> >>>> Resulting bitstream was tested with a conformance checker >>>> using the last draft of FFV1 specifications. >>> But has the way this is stored been optimized ? >>> >>> Once its marked as non exerimental all future decoders must support the exact >>> way. >> Although this is considered experimental in the encoder, the implementation >> adheres to the description in the specification. The bitstream specification >> does not provide a bitdepth limit so with the current draft of the >> specification, another FFV1 encoder could already encode 16-bit (and more) >> content. The work on the specification has been careful to not break >> compatibility with former drafts and at this point the FFV1 bitstream >> specification for versions 0, 1, and 3 should be considered already >> non-experimental for all bit depths. All current decoders should already >> support the exact way it is currently specified. >> >> To make a change in the specification at this point would have cascading >> consequences as we’d have to retract the part of the specification which >> states that micro_version 4 of version 3 is the first stable variant. Worse, >> it is impossible to indicate a bitstream change in FFV1 version 1, which >> permits RGB 16-bit content too, so it would be difficult for a decoder to >> detect a bitstream not conforming to the bitstream created by the current >> version of FFmpeg encoder. > Are you not making this look alot more complex than it is ? > Or maybe we talk about slightly different things here > > with the next version we can introduce any method of storing 16bit or 9-15 bit > and we certainly do not support in the implementation all possible bit depths > > From what i remember I had always wanted to improve the way that > more than 8bit is handled, not just 16. Although 16 is a bit special > > Consider this: > If we improve get_context() in the next version for >8bit > we still have to support 9-15 bit with the old definition > if we now declare 16bit non experimental then we also must support that with > an old get_context() in the decoder. > the 16bit path is seperate from the lower bit depth. So this is an Additional > codepath that we have to carry in the future > > isnt it smarter now that if we want to improve get_context() that we > dont now extend what can be generated with the current get_context ? > > or are such current get_context() style files already widespread ? > if so then its probably best to accept it and keep supporting it I am lost. Looks like we talk about 2 different subjects: FFV1 bitstream specifications and FFmpeg implementation. Let separate subjects: FFV1 bitstream specifications: Since at least 2012 [1] get_context (in the bitstream) is defined and flagged as stable for **all** bit depths for versions 0, 1, 3. It is still the case today [2]. There was a consensus on discussing about FFV1 bitstream on CELLAR mailing list There was a consensus for not changing the bitstream for version 0, 1, 3 so we can standardize it ASAP without breaking current implementations (reason we document bugs in the main encoder, but the idea is not to accept more changes) If the FFV1 bitstream for version 0, 1, or 3 must be changed in current stable version, it would be a major break in the consensus and it should be discussed on CELLAR list (in copy as they are concerned), but I doubt the consensus would be for breaking the bitstream compatibility as the discussion from day 0 on CELLAR is to document current bitstream without changing it for versions 0, 1, 3. The fact that there was no (publicly available) RGB48 encoder in the wild is not an excuse for breaking the compatibility with current draft of specifications. Same if someone decides to do an encoder for e.g. RGB3000. It is possible to change the bitstream with version 4 (experimental version), and it is a good place for changing get_context for 16-bit content (whatever is the color space, BTW). FFmpeg implementation: FFV1 YCbCr 16-bit is already flagged as non experimental, so there is already some non experimental 16-bit support in FFmpeg FFV1 encoder. 16-bit is not new and for RGB48 the actual encoding after RGB to YCbCr transformation is just 1 bit more (17-bit). FFmpeg experimental flag is for the status of the encoder, not for the status of the bitstream (the bitstream for versions 0, 1, 3 is already considered stable for RGB48 and others) FFV1 RGB48 support in FFmpeg is conform to FFV1 bitstream specifications. Optimizing FFmpeg for better compression of FFV1 RGB48 as specified in spec is not blocked after this change. The only reason for keeping the experimental flag for RGB48 support is if the resulting file does not comply to the FFV1 specification, and this is not the case (tested with a conformance checker complying to FFV1 specifications). As a result, the comment about get_contet for RGB48 is blocking for removing experimental status of version 4, but the suggested patch does not touch on version 4. [1] https://github.com/FFmpeg/FFV1/blob/cbb560873e54bfe2d2042e898a210fe2abd079e0/ffv1.lyx#L659 [2] https://github.com/FFmpeg/FFV1/blob/fdc65b73282633cc8867c185e83a6d21e5c65f3f/ffv1.md#context [3] https://github.com/FFmpeg/FFV1/blob/fdc65b73282633cc8867c185e83a6d21e5c65f3f/ffv1.md#micro_version
Hi On Sun, Jan 07, 2018 at 06:39:34PM +0100, Jerome Martinez wrote: > On 06/01/2018 23:21, Michael Niedermayer wrote: > >On Sat, Jan 06, 2018 at 04:54:18PM +0100, Jerome Martinez wrote: > >>On 06/01/2018 02:05, Michael Niedermayer wrote: > >>>> ffv1enc.c | 4 ---- > >>>> 1 file changed, 4 deletions(-) > >>>>acfc60c913b311b148f2eeef2d2d6ea9e37afcf7 0001-avcodec-ffv1enc-mark-RGB48-support-as-non-experiment.patch > >>>> From 303f63fb7e6172fdb7de66da1f8a4006b79a535f Mon Sep 17 00:00:00 2001 > >>>>From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Martinez?= <jerome@mediaarea.net> > >>>>Date: Fri, 5 Jan 2018 11:09:01 +0100 > >>>>Subject: [PATCH] avcodec/ffv1enc: mark RGB48 support as non-experimental > >>>> > >>>>Resulting bitstream was tested with a conformance checker > >>>>using the last draft of FFV1 specifications. > >>>But has the way this is stored been optimized ? > >>> > >>>Once its marked as non exerimental all future decoders must support the exact > >>>way. > >>Although this is considered experimental in the encoder, the implementation > >>adheres to the description in the specification. The bitstream specification > >>does not provide a bitdepth limit so with the current draft of the > >>specification, another FFV1 encoder could already encode 16-bit (and more) > >>content. The work on the specification has been careful to not break > >>compatibility with former drafts and at this point the FFV1 bitstream > >>specification for versions 0, 1, and 3 should be considered already > >>non-experimental for all bit depths. All current decoders should already > >>support the exact way it is currently specified. > >> > >>To make a change in the specification at this point would have cascading > >>consequences as we’d have to retract the part of the specification which > >>states that micro_version 4 of version 3 is the first stable variant. Worse, > >>it is impossible to indicate a bitstream change in FFV1 version 1, which > >>permits RGB 16-bit content too, so it would be difficult for a decoder to > >>detect a bitstream not conforming to the bitstream created by the current > >>version of FFmpeg encoder. > >Are you not making this look alot more complex than it is ? > >Or maybe we talk about slightly different things here > > > >with the next version we can introduce any method of storing 16bit or 9-15 bit > >and we certainly do not support in the implementation all possible bit depths > > > > From what i remember I had always wanted to improve the way that > >more than 8bit is handled, not just 16. Although 16 is a bit special > > > >Consider this: > >If we improve get_context() in the next version for >8bit > >we still have to support 9-15 bit with the old definition > >if we now declare 16bit non experimental then we also must support that with > >an old get_context() in the decoder. > >the 16bit path is seperate from the lower bit depth. So this is an Additional > >codepath that we have to carry in the future > > > >isnt it smarter now that if we want to improve get_context() that we > >dont now extend what can be generated with the current get_context ? > > > >or are such current get_context() style files already widespread ? > >if so then its probably best to accept it and keep supporting it > > I am lost. > Looks like we talk about 2 different subjects: FFV1 bitstream specifications > and FFmpeg implementation. > Let separate subjects: > [...] > FFmpeg implementation: > FFV1 YCbCr 16-bit is already flagged as non experimental, so there is > already some non experimental 16-bit support in FFmpeg FFV1 encoder. 16-bit > is not new and for RGB48 the actual encoding after RGB to YCbCr > transformation is just 1 bit more (17-bit). This is correct but i think misleading a bit the 17bit case this is about uses a seperate codepath. So if its enabled we generate new files that have never been generated before in some sense AFAIK If we change the bitstream in a future version, which i belive we should consider. Then we have an additional "old" version of the 17bit path that needs to be supported in implementations. > FFmpeg experimental flag is for the status of the encoder, not for the > status of the bitstream (the bitstream for versions 0, 1, 3 is already > considered stable for RGB48 and others) I think i added the flag check. The reason behind the check was so that the bitstream syntax can be changed without the need to support every iteration of the bitstream. I had hoped someone would fund research and testing of further improvments to the various fine details of higher bit depth encoding beyond 8bit. IIRC No further funding was provided, Noone worked on it as a volunteer, No changes where proposed ... But it wasnt intended as a "bitstream is in stone, encoder might mismatch the spec" at the time. We have a draft spec now that does not limit bitdepth in any way, this may have been a mistakke but i dont see this as a big problem honestly. I do not propose to change this. But i would not oppose it if people want to change it If someone creates a 19bit per sample file based on the draft spec it also will not decode with most real world implementations. There is no question that with a unlimited bitdepth, real implementations will never support some depths I just dont want to create a new variant of files _IF_ we plan to work on improving the "bitstream syntax" in the next version. Of course if these files are already out in the wild then we must support this in the implementation. And then most of this discussion is meaningless [...]
On 07/01/2018 20:08, Michael Niedermayer wrote: > [...] > This is correct but i think misleading a bit > the 17bit case this is about uses a seperate codepath. For your (FFmpeg) encoder and decoder. Not mine (same code path is used for 8/10/12/16/... bit RGB and YUV with my encoder and decoder). Again, we mix up bitstream specification and FFmpeg implementation. In the bitstream specifications, there is currently only one path (except for handling files in the wild which are not conform to the unique path, we take care of them for historical reasons). > So if its enabled > we generate new files that have never been generated before in some sense > AFAIK - MediaConch conformance checker can check files up to 30 bits per component, and already implements the way it is written in spec for all supported bit depths, because the spec was flagged as stable whatever is the bit depth for a long time, without having anyone saying that 16 or 17-bit for RGB (reminder: it is already set as "stable" for YCbCr 16-bit) may have a different paths later for versions 0, 1, 3. If you change the bitstream of RGB48 or any stream with bitdepth <=30, you break it despite the fact it is validating correctly and would be a major issue for this tool (breaking trust about the tool or FFV1, as there would be 2 variants of FFV1 RGB48). - I have an early (not yet public, for testing the spec only for the moment) version of an encoder conforming to the spec for all bit depths up to 30-bit per component. - I have heard about other FFV1 encoders able to encode RGB48 > > If we change the bitstream in a future version, which i belive we > should consider. Then we have an additional "old" version of the 17bit path > that needs to be supported in implementations. question: How would you detect old version, from all encoders (not only FFmpeg)? there is nothing permitting that in the bitstream AFAIK (for v1: no micro_version; for v3: micro version >4 must be compatible with v4 as v4 is flagged stable). So it is "bitstream is in stone" now, for at least versions 0, 1, 3. Reason I speak about R&D with version 4 bitstream (I don't speak about FFmpeg), which is still flagged as experimental for all files. > > >> FFmpeg experimental flag is for the status of the encoder, not for the >> status of the bitstream (the bitstream for versions 0, 1, 3 is already >> considered stable for RGB48 and others) > I think i added the flag check. The reason behind the check was so that the > bitstream syntax can be changed without the need to support every iteration > of the bitstream. > I had hoped someone would fund research and testing of further improvments > to the various fine details of higher bit depth encoding beyond 8bit. > IIRC No further funding was provided, Noone worked on it as a volunteer, > No changes where proposed ... This is planned on my side after FFV1 versions 0, 1, 3 are finalized. With version 4 (the experimental version). but it is possible only if we all are in sync and don't do something against the consensus, and as far I as I understand the consensus is that versions 0, 1, 3 are "bitstream is in stone" now (and actually from the beginning of CELLAR) based on the draft of FFV1 specifications, which is detailed before going to IETF but not changed. All the communication, at IETF and conferences, around FFV1 versions 0, 1, 3 is that the bitstream specification is stable, and discussing about breaking this consensus may harm FFV1 spread. > > But it wasnt intended as a "bitstream is in stone, encoder might mismatch > the spec" at the time. I disagree: the idea behind CELLAR is that all encoders **must** match the spec, or there is misunderstanding to clarify. Else everyone does his own version of FFV1, and we can not work together on an unique lossless compression format. The spec is expected to be the reference (for versions 0, 1, 3 for the moment) and encoders are expected to match the spec. > > We have a draft spec now that does not limit bitdepth in any way, this may > have been a mistakke but i dont see this as a big problem honestly. I do not > propose to change this. But i would not oppose it if people want to change it > > If someone creates a 19bit per sample file based on the draft spec it also > will not decode with most real world implementations. It will with my conformance checker. You can not know what is "real world", as a couple of encoders may be in house only. we still mix up bitstream specification and one implementation. The idea behind CELLAR is to have FFV1 a standardized video format, so we can not decide for 1 encoder to change the bitstream. We need to find a consensus on CELLAR mailing-list first. The consensus is what is written in the draft and there was no post on CELLAR mailing-list for limiting to some bit depths. > There is no question that with a unlimited bitdepth, real implementations > will never support some depths Whatever is the support from the encoders you know, you can not know what was created by others based on the bitstream specification. so the bitstream is specified (for versions 0, 1, 3) for all bit depths and we can not change that now, as it is written for a long time in spec that the spec is flagged stable for these versions. The question is not what is the support from decoder x, the question is that if there is a plan to break the compatibility between current version of FFV1 draft and future versions of this document. > > I just dont want to create a new variant of files _IF_ we plan to work on > improving the "bitstream syntax" in the next version. I don't understand: yes, we plan to improve the bitstream, and this is the reason we plan a version 4, 5, 6... But it does not mean that versions 0, 1, 3 are not used by FFmpeg or any other encoder for all bit depths. But again, here (ffmpeg-devel), my goal was to speak about FFmpeg encoder, it was not intended to speak about FFV1 specification, and it is weird for me to have to speak about FFV1 specification on ffmpeg-devel (it should be on CELLAR only). Being sure that FFmpeg is creating a bitstream conforming to FFV1 specifications (so versions 0, 1, 3) should be the only criteria. > Of course if these files are already out in the wild then we must support > this in the implementation. And then most of this discussion is meaningless I confirm that RGB48 files are already in the wild, as it is needed by some archives, and I confirm that they rely on MediaConch for testing the conformance compared to the specification. I still don't see a reason to block 1 encoder to create RGB48 with version 1 or 3, as the spec is clear for a long time about how the bitstream is for such bit depth. Note that the FFV1 RGB48 decoder is not flagged as experimental, so any old version of FFmpeg should be able to decode new files you want to create (I don't see how they could do that as they don't know the get_context you want to implement) if you don't want to have complain that an old stable version of FFmpeg is not able to decode a FFV1 RGB48 file you just created with the new FFmpeg implementation. Again, the experimental flag is for FFmpeg encoder, not the FFV1 bitstream, the FFV1 bitstream is flagged as stable in FFV1 specification, including RGB48. Additionally, FFmpeg FFV1 decoder is currently able to decode RGB48 as specified in FFV1 specification and created by encoders conforming to the spec (FFmpeg "experimental" included) without having to set "-strict experimental" on the command line. For these reasons, I still argue to remove the experimental flag for this encoder about RGB48 as the output is conform to FFV1 specifications and current and older versions of FFmpeg would not be able to decode FFV1 RGB48 if get_context is changed. Jérôme
On Sun, Jan 07, 2018 at 09:20:15PM +0100, Jerome Martinez wrote: > On 07/01/2018 20:08, Michael Niedermayer wrote: > >[...] > >This is correct but i think misleading a bit > >the 17bit case this is about uses a seperate codepath. > > For your (FFmpeg) encoder and decoder. Not mine (same code path is used for > 8/10/12/16/... bit RGB and YUV with my encoder and decoder). > Again, we mix up bitstream specification and FFmpeg implementation. The seperation of 16bit and >16bit results out of the data type size fitting or not fitting in "int16". A space/cache efficient implementation, possibly one using SIMD would use seperate codepaths for 16 and > 16 on a normal architecture thats based on 8bit bytes. Its part of spec design what "an efficient implementation" would/could do [...] > > > >If we change the bitstream in a future version, which i belive we > >should consider. Then we have an additional "old" version of the 17bit path > >that needs to be supported in implementations. > > question: How would you detect old version, from all encoders (not only > FFmpeg)? there is nothing permitting that in the bitstream AFAIK (for v1: no > micro_version; for v3: micro version >4 must be compatible with v4 as v4 is > flagged stable). > So it is "bitstream is in stone" now, for at least versions 0, 1, 3. Reason > I speak about R&D with version 4 bitstream (I don't speak about FFmpeg), > which is still flagged as experimental for all files. very simple, the lowest version that we can make that change in without causing any pain to people using ffv1 and without breaking the spec. [...] > > > >But it wasnt intended as a "bitstream is in stone, encoder might mismatch > >the spec" at the time. > > I disagree: the idea behind CELLAR is that all encoders **must** match the you disagree what i mean in a years old commit in ffmpeg i authored ? > spec, or there is misunderstanding to clarify. Else everyone does his own > version of FFV1, and we can not work together on an unique lossless > compression format. The spec is expected to be the reference (for versions > 0, 1, 3 for the moment) and encoders are expected to match the spec. you make no sense This code was intended to be used for developing the next version, that didnt happen. If it did happen the code in the implementation would have changed and once it was looking optimal a suggestion with test results would have been posted to CELLAR to update the draft/spec. If that update did happen then once its "stable" the check would have been removed from the encoder implementation so people could use it. [...] > > >There is no question that with a unlimited bitdepth, real implementations > >will never support some depths > > Whatever is the support from the encoders you know, you can not know what > was created by others based on the bitstream specification. so the bitstream > is specified (for versions 0, 1, 3) for all bit depths and we can not change > that now, as it is written for a long time in spec that the spec is flagged > stable for these versions. I can know what is possible with real implementations build by others because they are constrained by the same physical laws and space and matter available in the observable universe. > > The question is not what is the support from decoder x, the question is that > if there is a plan to break the compatibility between current version of > FFV1 draft and future versions of this document. I think its more like a circular reference The draft/spec intended to describe the existing status quo and while in the implementation something was marked experimental this detail was lost in the spec [...] > I confirm that RGB48 files are already in the wild, as it is needed by some > archives, and I confirm that they rely on MediaConch for testing the > conformance compared to the specification. Thanks, then its probably best if we support this format in en and decoder implementations. As its out there already [...]
On Sun, 7 Jan 2018 21:20:15 +0100, Jerome Martinez <jerome@mediaarea.net> wrote: > - I have an early (not yet public, for testing the spec only for the > moment) version of an encoder conforming to the spec for all bit depths > up to 30-bit per component. > - I have heard about other FFV1 encoders able to encode RGB48 i am curious what are the names of the other ffv1 encoders? > I confirm that RGB48 files are already in the wild, as it is needed please upload or link to one of these RGB48 samples and ffmpeg will support it. i think i can clear up the confusion between you and michael. michael does not want to add in a defined way to do other bitdepths without the spec people agreeing on a spec for this. michael does want to support your higher bitdepth ffv1 samples. the confusion was because he thought you wanted to modify the spec / encoder and not just the ability to decode such files. hope this helps. -compn
On Fri, Jan 05, 2018 at 10:26:42PM +0100, Jerome Martinez wrote: > On 05/01/2018 16:14, Tobias Rapp wrote: > >On 05.01.2018 11:18, Jerome Martinez wrote: > >>0001-FFV1-make-RGB48-support-as-non-experimental.patch > >> > >>From cd1bfe68a1a809700f25e593ac21ca3876d265ad Mon Sep 17 00:00:00 2001 > >>From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Martinez?= <jerome@mediaarea.net> > >>Date: Fri, 5 Jan 2018 11:09:01 +0100 > >>Subject: [PATCH] FFV1: make RGB48 support as non-experimental > > > >make => mark > > I reused a commit text from history, both words words were in it. > Anyway, attached is the updated .patch with the suggested subject line. > > > > >BTW: common subject line format would be something like "avcodec/ffv1enc: > >mark RGB48 support as non-experimental" > > ffv1enc.c | 4 ---- > 1 file changed, 4 deletions(-) > acfc60c913b311b148f2eeef2d2d6ea9e37afcf7 0001-avcodec-ffv1enc-mark-RGB48-support-as-non-experiment.patch > From 303f63fb7e6172fdb7de66da1f8a4006b79a535f Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Martinez?= <jerome@mediaarea.net> > Date: Fri, 5 Jan 2018 11:09:01 +0100 > Subject: [PATCH] avcodec/ffv1enc: mark RGB48 support as non-experimental > > Resulting bitstream was tested with a conformance checker > using the last draft of FFV1 specifications. > --- > libavcodec/ffv1enc.c | 4 ---- > 1 file changed, 4 deletions(-) will apply this with a somewhat expanded commit message thanks [...]
diff --git a/libavcodec/ffv1enc.c b/libavcodec/ffv1enc.c index 09df4c0c57..c0c1558ffe 100644 --- a/libavcodec/ffv1enc.c +++ b/libavcodec/ffv1enc.c @@ -630,10 +630,6 @@ FF_ENABLE_DEPRECATION_WARNINGS s->bits_per_raw_sample = 16; s->use32bit = 1; s->version = FFMAX(s->version, 1); - if (avctx->strict_std_compliance > FF_COMPLIANCE_EXPERIMENTAL) { - av_log(avctx, AV_LOG_ERROR, "16bit RGB is experimental and under development, only use it for experiments\n"); - return AVERROR_INVALIDDATA; - } break; case AV_PIX_FMT_0RGB32: s->colorspace = 1;