| Message ID | 63dcf4c8-53bf-ef81-140a-025d6181818c@mediaarea.net |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, Feb 01, 2018 at 01:43:00PM +0100, Jerome Martinez wrote: > Add support for 16-bit/component RGB with Alpha encoding and decoding in > FFV1, both RGBA64 and GBRAP16 for encoding, GBRAP16 for decoding. > > Resulting bitstream was tested about lossless encoding/decoding by the > compression from DPX to FFV1 then decompression from FFV1 to DPX, see > commands below (resulting framemd5 hashes are all same). > Resulting bitstream is decodable by another decoder (with same resulting > framemd5 hash). > Resulting bitstream passed through a conformance checker compared to current > FFV1 specification IETF draft. > > About the patch: > - some modified lines are not used (the ones not used when f->use32bit is > 1), but it makes the code more coherent (especially because decode_rgb_frame > signature is same for both 16-bit and 32-bit version) and prepares the > support of RGBA with 10/12/14 bits/component. > - GBRAP16 was chosen for decoding because GBRP16 is already used when no > alpha, and the code is more prepared for planar pix_fmt when bit depth is > >8. > - "s->transparency = desc->nb_components == 4 || desc->nb_components == 2;" > is a copy of a line a bit above about the detection of transparency, I > preferred to reuse it as is even if "YA" 16-bit/component is not (yet) > supported. > > FFmpeg commands used for tests: > ./ffmpeg -i in.dpx -c:v ffv1 out.mkv > ./ffmpeg -i in.dpx -pix_fmt gbrap16 -strict -2 -c:v ffv1 out2.mkv > ./ffmpeg -i out.mkv out.dpx > > ./ffmpeg -i in.dpx -f framemd5 in.dpx.framemd5 > ./ffmpeg -i out.mkv -pix_fmt rgba64be -f framemd5 out.mkv.framemd5 > ./ffmpeg -i out2.mkv -pix_fmt rgba64be -f framemd5 out2.mkv.framemd5 > ./ffmpeg -i out.dpx -f framemd5 out.dpx.framemd5 > > Test file used (renamed to in.dpx): > https://mediaarea.net/temp/uncropped_DPX_4K_16bit_Overscan15pros.dpx I would prefer if the algorithm would be tuned to 16bit data before adding more formats to the encoder which require all decoders to support them. Dont you agree that this would be the better strategy ? [...]
Michael Niedermayer wrote: >I would prefer if the algorithm would be tuned to 16bit data >before adding more formats to the encoder which require all >decoders to support them. > >Dont you agree that this would be the better strategy ? To my understanding FFV1 v3 is actually stable. Or am I missing something? Some of us are indeed trying to work on v4, but this is another topic, I guess. Best regards, Reto
On 03/02/2018 00:10, Michael Niedermayer wrote: > On Thu, Feb 01, 2018 at 01:43:00PM +0100, Jerome Martinez wrote: >> Add support for 16-bit/component RGB with Alpha encoding and decoding in >> FFV1, both RGBA64 and GBRAP16 for encoding, GBRAP16 for decoding. >> >> Resulting bitstream was tested about lossless encoding/decoding by the >> compression from DPX to FFV1 then decompression from FFV1 to DPX, see >> commands below (resulting framemd5 hashes are all same). >> Resulting bitstream is decodable by another decoder (with same resulting >> framemd5 hash). >> Resulting bitstream passed through a conformance checker compared to current >> FFV1 specification IETF draft. >> >> About the patch: >> - some modified lines are not used (the ones not used when f->use32bit is >> 1), but it makes the code more coherent (especially because decode_rgb_frame >> signature is same for both 16-bit and 32-bit version) and prepares the >> support of RGBA with 10/12/14 bits/component. >> - GBRAP16 was chosen for decoding because GBRP16 is already used when no >> alpha, and the code is more prepared for planar pix_fmt when bit depth is >>> 8. >> - "s->transparency = desc->nb_components == 4 || desc->nb_components == 2;" >> is a copy of a line a bit above about the detection of transparency, I >> preferred to reuse it as is even if "YA" 16-bit/component is not (yet) >> supported. >> >> FFmpeg commands used for tests: >> ./ffmpeg -i in.dpx -c:v ffv1 out.mkv >> ./ffmpeg -i in.dpx -pix_fmt gbrap16 -strict -2 -c:v ffv1 out2.mkv >> ./ffmpeg -i out.mkv out.dpx >> >> ./ffmpeg -i in.dpx -f framemd5 in.dpx.framemd5 >> ./ffmpeg -i out.mkv -pix_fmt rgba64be -f framemd5 out.mkv.framemd5 >> ./ffmpeg -i out2.mkv -pix_fmt rgba64be -f framemd5 out2.mkv.framemd5 >> ./ffmpeg -i out.dpx -f framemd5 out.dpx.framemd5 >> >> Test file used (renamed to in.dpx): >> https://mediaarea.net/temp/uncropped_DPX_4K_16bit_Overscan15pros.dpx > I would prefer if the algorithm would be tuned to 16bit data before > adding more formats to the encoder which require all decoders to support > them. > > Dont you agree that this would be the better strategy ? ccing CELLAR. My remarks are the same as with RGB48 support (including that the compression performance of RGB48 so RGBA64 is already very good without touching on the algorithm, and IMO tuning should be for v4 for all bit depths), with addition that since the last debate on that on ffmpeg-devel there was no patch proposal so no consensus on CELLAR for limiting the specifications to what exists in FFmpeg implementation (so current consensus is that FFV1 specs are for all bit depths for all supported color spaces), and since the last debate FFV1 specs draft were sent to IETF tracker so it is close to the end. This patch is just adding the support of RGBA64 conforming to the current specs and without big changes (no complex stuff, just mapping to the right pix_fmt), and the current specs are the ones with very high chances to be the standard (up to now nobody suggested on CELLAR, the place for the spec, to limit the support to a set of bit depths / color spaces, and nobody suggested a tuning for some bit depths), with the main advantage that the specs are clear about all bit depths for all color spaces supported (it is good that it is generic). Will this patch be accepted after the IETF flags the current specs as stable if there is no changes on the wording about the support of all bit depths? on my side, I can not spend time on FFV1 v4 R&D (tuning and more) when I spend time with such blocking (for me) issue about v3 (i.e. agreement in specs draft on all bit depths for all supported color spaces but no agreement in practice on all bit depths for all supported color spaces). So for answering directly to the question, no I don't agree that changing v3 bitstream with specific tuning of the bitstream depending of the bit depth is a better strategy, actually this is the opposite (I think that the best strategy is to support as many bit depths as possible in implementations with current v3 specs and that all tuning should be in the version flagged as experimental, not the one flagged as stable even before working on IETF version, if we change a bitstream marked as stable we break the trust in the spec being stable, IMO any tuning of the bitstream should be done in v4, and any performance improvement without breaking the bitstream could be done after this patch without problem). Jérôme
On Sat, Feb 03, 2018 at 10:57:44AM +0100, Jerome Martinez wrote: > On 03/02/2018 00:10, Michael Niedermayer wrote: > >On Thu, Feb 01, 2018 at 01:43:00PM +0100, Jerome Martinez wrote: > >>Add support for 16-bit/component RGB with Alpha encoding and decoding in > >>FFV1, both RGBA64 and GBRAP16 for encoding, GBRAP16 for decoding. > >> > >>Resulting bitstream was tested about lossless encoding/decoding by the > >>compression from DPX to FFV1 then decompression from FFV1 to DPX, see > >>commands below (resulting framemd5 hashes are all same). > >>Resulting bitstream is decodable by another decoder (with same resulting > >>framemd5 hash). > >>Resulting bitstream passed through a conformance checker compared to current > >>FFV1 specification IETF draft. > >> > >>About the patch: > >>- some modified lines are not used (the ones not used when f->use32bit is > >>1), but it makes the code more coherent (especially because decode_rgb_frame > >>signature is same for both 16-bit and 32-bit version) and prepares the > >>support of RGBA with 10/12/14 bits/component. > >>- GBRAP16 was chosen for decoding because GBRP16 is already used when no > >>alpha, and the code is more prepared for planar pix_fmt when bit depth is > >>>8. > >>- "s->transparency = desc->nb_components == 4 || desc->nb_components == 2;" > >>is a copy of a line a bit above about the detection of transparency, I > >>preferred to reuse it as is even if "YA" 16-bit/component is not (yet) > >>supported. > >> > >>FFmpeg commands used for tests: > >>./ffmpeg -i in.dpx -c:v ffv1 out.mkv > >>./ffmpeg -i in.dpx -pix_fmt gbrap16 -strict -2 -c:v ffv1 out2.mkv > >>./ffmpeg -i out.mkv out.dpx > >> > >>./ffmpeg -i in.dpx -f framemd5 in.dpx.framemd5 > >>./ffmpeg -i out.mkv -pix_fmt rgba64be -f framemd5 out.mkv.framemd5 > >>./ffmpeg -i out2.mkv -pix_fmt rgba64be -f framemd5 out2.mkv.framemd5 > >>./ffmpeg -i out.dpx -f framemd5 out.dpx.framemd5 > >> > >>Test file used (renamed to in.dpx): > >>https://mediaarea.net/temp/uncropped_DPX_4K_16bit_Overscan15pros.dpx > >I would prefer if the algorithm would be tuned to 16bit data before > >adding more formats to the encoder which require all decoders to support > >them. > > > >Dont you agree that this would be the better strategy ? > > ccing CELLAR. > > My remarks are the same as with RGB48 support (including that the > compression performance of RGB48 so RGBA64 is already very good without > touching on the algorithm, and IMO tuning should be for v4 for all bit > depths), with addition that since the last debate on that on ffmpeg-devel > there was no patch proposal so no consensus on CELLAR for limiting the > specifications to what exists in FFmpeg implementation (so current consensus > is that FFV1 specs are for all bit depths for all supported color spaces), > and since the last debate FFV1 specs draft were sent to IETF tracker so it > is close to the end. > > This patch is just adding the support of RGBA64 conforming to the current > specs and without big changes (no complex stuff, just mapping to the right > pix_fmt), and the current specs are the ones with very high chances to be > the standard (up to now nobody suggested on CELLAR, the place for the spec, > to limit the support to a set of bit depths / color spaces, and nobody > suggested a tuning for some bit depths), with the main advantage that the > specs are clear about all bit depths for all color spaces supported (it is > good that it is generic). Will this patch be accepted after the IETF flags > the current specs as stable if there is no changes on the wording about the > support of all bit depths? > > on my side, I can not spend time on FFV1 v4 R&D (tuning and more) when I > spend time with such blocking (for me) issue about v3 (i.e. agreement in > specs draft on all bit depths for all supported color spaces but no > agreement in practice on all bit depths for all supported color spaces). > > So for answering directly to the question, no I don't agree that changing v3 > bitstream with specific tuning of the bitstream depending of the bit depth > is a better strategy, That was not meant. To clarify my suggestion, the algorithm should be tuned for high bit depth before using it for long term storage. This would be v4 (or later). Personally i would wait for v4 and not use v3 for high bit depth. Which is why i think its not smart to extend the v3 implementation with more high depth support. > actually this is the opposite (I think that the best > strategy is to support as many bit depths as possible in implementations > with current v3 specs and that all tuning should be in the version flagged > as experimental, not the one flagged as stable even before working on IETF > version, if we change a bitstream marked as stable we break the trust in the > spec being stable, IMO any tuning of the bitstream should be done in v4, and > any performance improvement without breaking the bitstream could be done > after this patch without problem). IIRC teh v3 draft was intended to describe the format generated by the existing implementation. The existing implementation allowed 16bit RGB only when the user specified flags indicating that they are creating non standard files. "16bit RGB is experimental and under development, only use it for experiments" There was no standard 16bit RGB or higher depth supported, these where in development on the specification and implementation side. IIUC people created such files somehow beliving that the IMO clear warning somehow suggested that this was stable. And with that we are a bit stuck with this for v3 you are technically correct that the current draft allows it and the additions but iam not sure its really a good idea to add more cases under this. Thanks [...]
Michael Niedermayer wrote: >To clarify my suggestion, >the algorithm should be tuned for high bit depth before using >it for long term storage. This would be v4 (or later). >Personally i would wait for v4 and not use v3 for high bit >depth. Which is why i think its not smart to extend the v3 >implementation with more high depth support. The issue is that in the real world we need to use the format now. Otherwise the film archives must use MXF/DPX instead of Матрёшка/FFV1. That's the point! I presented to the industry this solution in August 2016 at "The Reel Thing" technical symposium in Hollywood, and an article on it was published in April 2017 in FIAF's "Journal of Film Preservation". The archives cannot wait forever! And they are indeed important users of FFmpeg, in my opinion. I already paid EUR 7000 for the FFV1 use in the archival world, and will pay EUR 5000 additionally in the next months. >IIUC people created such files somehow beliving that the IMO >clear warning somehow suggested that this was stable. And with >that we are a bit stuck with this for v3 I presented last November at the "No Time to Wait" symposium (nomen est omen) in Vienna - i.e. in your city - how we have to work today and how we hope to modify container and codec during the next data migration. Last but not least, since almost two years now it's impossible to work on the development of FFV1 v4. It's always the wrong time and/or the wrong place every time I am doing something for this cause. Why? This is extremely frustrating. Best regards, Reto
On Sat, Feb 03, 2018 at 01:58:10PM +0100, Reto Kromer wrote: > Michael Niedermayer wrote: > > >To clarify my suggestion, > >the algorithm should be tuned for high bit depth before using > >it for long term storage. This would be v4 (or later). > >Personally i would wait for v4 and not use v3 for high bit > >depth. Which is why i think its not smart to extend the v3 > >implementation with more high depth support. > > The issue is that in the real world we need to use the format > now. Otherwise the film archives must use MXF/DPX instead of > Матрёшка/FFV1. That's the point! Then using the v3 16bit is probably the most realistic option. And jeromes patch should probably be applied I hope this will not reduce interrest in working on a improved 9-16bit mode in v4. [...] > Last but not least, since almost two years now it's impossible > to work on the development of FFV1 v4. It's always the wrong > time and/or the wrong place every time I am doing something for > this cause. Why? This is extremely frustrating. I want to understand this too. IMO v4 development should be more important than or at least equally to the v3 draft polishing and neither should block the other.
Loooking to it from a pragmatic management point of view, there is a lot of open source unfriendly community from the EBU/SMPTE world which would have their opinion about a failed or even more delayed IETF spec finalisation. I think - given the efforts which have been put into it, the beauty with what is ALREADY existing, coexistency of v4 work and finalisation of v3 work towards IETF standardization is the strategy which we should concentrate on. 16bit in v3: Please go for it. And allow for v4 for other discussions. Not to forget about all video projects which would be happy to see that IETF and the community finalises v3 also for it´s 8/10bit formats which have an even higher representation in hundred thousands of archive hours already now. If there is something I can do please contact me. Christophe NOA -----Ursprüngliche Nachricht----- Von: Cellar [mailto:cellar-bounces@ietf.org] Im Auftrag von Michael Niedermayer Gesendet: Samstag, 03. Februar 2018 14:48 An: Codec Encoding for LossLess Archiving and Realtime transmission <cellar@ietf.org> Cc: ffmpeg-devel@ffmpeg.org Betreff: Re: [Cellar] [FFmpeg-devel] [PATCH] avcodec/ffv1: Support for RGBA64 and GBRAP16 On Sat, Feb 03, 2018 at 01:58:10PM +0100, Reto Kromer wrote: > Michael Niedermayer wrote: > > >To clarify my suggestion, > >the algorithm should be tuned for high bit depth before using it for > >long term storage. This would be v4 (or later). > >Personally i would wait for v4 and not use v3 for high bit depth. > >Which is why i think its not smart to extend the v3 implementation > >with more high depth support. > > The issue is that in the real world we need to use the format now. > Otherwise the film archives must use MXF/DPX instead of Матрёшка/FFV1. > That's the point! Then using the v3 16bit is probably the most realistic option. And jeromes patch should probably be applied I hope this will not reduce interrest in working on a improved 9-16bit mode in v4. [...] > Last but not least, since almost two years now it's impossible > to work on the development of FFV1 v4. It's always the wrong > time and/or the wrong place every time I am doing something for > this cause. Why? This is extremely frustrating. I want to understand this too. IMO v4 development should be more important than or at least equally to the v3 draft polishing and neither should block the other. -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Republics decline into democracies and democracies degenerate into despotisms. -- Aristotle
On Thu, Feb 01, 2018 at 01:43:00PM +0100, Jerome Martinez wrote: > Add support for 16-bit/component RGB with Alpha encoding and decoding in > FFV1, both RGBA64 and GBRAP16 for encoding, GBRAP16 for decoding. > > Resulting bitstream was tested about lossless encoding/decoding by the > compression from DPX to FFV1 then decompression from FFV1 to DPX, see > commands below (resulting framemd5 hashes are all same). > Resulting bitstream is decodable by another decoder (with same resulting > framemd5 hash). > Resulting bitstream passed through a conformance checker compared to current > FFV1 specification IETF draft. > > About the patch: > - some modified lines are not used (the ones not used when f->use32bit is > 1), but it makes the code more coherent (especially because decode_rgb_frame > signature is same for both 16-bit and 32-bit version) and prepares the > support of RGBA with 10/12/14 bits/component. > - GBRAP16 was chosen for decoding because GBRP16 is already used when no > alpha, and the code is more prepared for planar pix_fmt when bit depth is > >8. > - "s->transparency = desc->nb_components == 4 || desc->nb_components == 2;" > is a copy of a line a bit above about the detection of transparency, I > preferred to reuse it as is even if "YA" 16-bit/component is not (yet) > supported. > > FFmpeg commands used for tests: > ./ffmpeg -i in.dpx -c:v ffv1 out.mkv > ./ffmpeg -i in.dpx -pix_fmt gbrap16 -strict -2 -c:v ffv1 out2.mkv > ./ffmpeg -i out.mkv out.dpx > > ./ffmpeg -i in.dpx -f framemd5 in.dpx.framemd5 > ./ffmpeg -i out.mkv -pix_fmt rgba64be -f framemd5 out.mkv.framemd5 > ./ffmpeg -i out2.mkv -pix_fmt rgba64be -f framemd5 out2.mkv.framemd5 > ./ffmpeg -i out.dpx -f framemd5 out.dpx.framemd5 > > Test file used (renamed to in.dpx): > https://mediaarea.net/temp/uncropped_DPX_4K_16bit_Overscan15pros.dpx > > Jérôme > [...] > diff --git a/libavcodec/ffv1enc_template.c b/libavcodec/ffv1enc_template.c > index b7eea0dd70..2763082540 100644 > --- a/libavcodec/ffv1enc_template.c > +++ b/libavcodec/ffv1enc_template.c > @@ -122,8 +122,8 @@ static av_always_inline int RENAME(encode_line)(FFV1Context *s, int w, > return 0; > } > > -static int RENAME(encode_rgb_frame)(FFV1Context *s, const uint8_t *src[3], > - int w, int h, const int stride[3]) > +static int RENAME(encode_rgb_frame)(FFV1Context *s, const uint8_t *src[4], > + int w, int h, const int stride[4]) > { > int x, y, p, i; > const int ring_size = s->context_model ? 3 : 2; > @@ -152,14 +152,18 @@ static int RENAME(encode_rgb_frame)(FFV1Context *s, const uint8_t *src[3], > r = (v >> 16) & 0xFF; > a = v >> 24; > } else if (packed) { > - const uint16_t *p = ((const uint16_t*)(src[0] + x*6 + stride[0]*y)); > + const uint16_t *p = ((const uint16_t*)(src[0] + x*(3 + s->transparency)*2 + stride[0]*y)); > r = p[0]; > g = p[1]; > b = p[2]; > + if (s->transparency) transparency should possibly be moved into a local variable > + a = p[3]; > } else if (sizeof(TYPE) == 4) { > g = *((const uint16_t *)(src[0] + x*2 + stride[0]*y)); > b = *((const uint16_t *)(src[1] + x*2 + stride[1]*y)); > r = *((const uint16_t *)(src[2] + x*2 + stride[2]*y)); > + if (s->transparency) > + a = *((const uint16_t *)(src[3] + x*2 + stride[2]*y)); ^^^^^^^^^ this looks wrong also what speed effect do thes changes to the inner loop have ? [...]
On 03/02/2018 14:48, Michael Niedermayer wrote: > > I hope this will not reduce interrest in working on a improved > 9-16bit mode in v4. I don't like to put politics in technical stuff, but here this is politics, and I think that blocking an easy improvement of FFV1 v3 encoding/decoding in FFmpeg (actually just using the current FFV1 v3, and also v1 actually, specification without having artificial limitations about bit depths for a specific color space, i.e. 16-bit support was already there for YUV before I work on FFV1) just for forcing people to do a completely different work (e.g. working on FFV1 v4) is not fair and IMO is not in the idea behind open source. IMO if interest for 9-16bit (side note: I don't see why 8-bit should not be in the range, some ideas I have for v4 are useful also for 8-bit) mode in v4 is reduced, that would only mean that v3 is already so great and/or just that people have no better idea right now, it is not bad, and both works (using v3 at the maximum of its possibilities and working on v4) are separate works. FYI criticism I saw about FFV1 is not compression ratio but speed (playing a 4K stream is just impossible on a "normal" machine, you need a very expensive CPU for that) so my plan is to focus on speed improvement in priority. It could be v4 stuff (incompatible changes), or v3 (encoder/decoder optimization), depending of what I find. > > > [...] > >> Last but not least, since almost two years now it's impossible >> to work on the development of FFV1 v4. It's always the wrong >> time and/or the wrong place every time I am doing something for >> this cause. Why? This is extremely frustrating. > I want to understand this too. IMO v4 development should be more > important than or at least equally to the v3 draft polishing and neither > should block the other. Also politics, but I don't understand who is blocking v4 from your point of view. "impossible to work on v4"? Where? As far as I see 1 person (me) said his priority is having FFV1 v3 becoming a standard (so IETF related work) and widely used (so any bit depth for any supported color space in v3 because it is an easy demonstration that FFV1 is versatile without having to wait for v4 R&D, especially because v3 bitstream design is already pretty good and versatile) because this is what I need, and my personal case is not blocking anyone else for sending patches for either FFV1 specifications or FFmpeg code about v4. Eager to see patch proposals for v4 (and v3 if it does not break support of files in the wild), whoever feeling blocked with his patches should send patch proposals to either FFmpeg (code) or CELLAR mailing list (bitstream format). That said, I plan to add more pix_fmt support for FFV1 v3 (which is already a good format!) support in FFmpeg and some optimization ideas beside my work for IETF spec, with the hope we could speak about technical issues (e.g. more optimization hints or info about wrong code or warning about that it breaks the bitstream specs as currently written), leaving aside politics.
> On Feb 5, 2018, at 4:20 PM, Jerome Martinez <Jerome@MediaArea.net> wrote: > > On 03/02/2018 14:48, Michael Niedermayer wrote: >> >> I hope this will not reduce interrest in working on a improved >> 9-16bit mode in v4. > > I don't like to put politics in technical stuff, but here this is politics, and I think that blocking an easy improvement of FFV1 v3 encoding/decoding in FFmpeg (actually just using the current FFV1 v3, and also v1 actually, specification without having artificial limitations about bit depths for a specific color space, i.e. 16-bit support was already there for YUV before I work on FFV1) just for forcing people to do a completely different work (e.g. working on FFV1 v4) is not fair and IMO is not in the idea behind open source. > IMO if interest for 9-16bit (side note: I don't see why 8-bit should not be in the range, some ideas I have for v4 are useful also for 8-bit) mode in v4 is reduced, that would only mean that v3 is already so great and/or just that people have no better idea right now, it is not bad, and both works (using v3 at the maximum of its possibilities and working on v4) are separate works. FYI criticism I saw about FFV1 is not compression ratio but speed (playing a 4K stream is just impossible on a "normal" machine, you need a very expensive CPU for that) so my plan is to focus on speed improvement in priority. It could be v4 stuff (incompatible changes), or v3 (encoder/decoder optimization), depending of what I find. IMHO, improved higher bit depths in version 4 is very interesting. We already have a few noted exceptions where version 4 is intended to fix and optimize a few things of version 3 (signed 16 bit handling, RGB plane order for 9-15 bits), so a change in optimization of high bit depth handling seems already appropriate for consideration. >> [...] >> >>> Last but not least, since almost two years now it's impossible >>> to work on the development of FFV1 v4. It's always the wrong >>> time and/or the wrong place every time I am doing something for >>> this cause. Why? This is extremely frustrating. >> I want to understand this too. IMO v4 development should be more >> important than or at least equally to the v3 draft polishing and neither >> should block the other. > > Also politics, but I don't understand who is blocking v4 from your point of view. "impossible to work on v4"? Where? As far as I see 1 person (me) said his priority is having FFV1 v3 becoming a standard (so IETF related work) and widely used (so any bit depth for any supported color space in v3 because it is an easy demonstration that FFV1 is versatile without having to wait for v4 R&D, especially because v3 bitstream design is already pretty good and versatile) because this is what I need, and my personal case is not blocking anyone else for sending patches for either FFV1 specifications or FFmpeg code about v4. Eager to see patch proposals for v4 (and v3 if it does not break support of files in the wild), whoever feeling blocked with his patches should send patch proposals to either FFmpeg (code) or CELLAR mailing list (bitstream format). > > That said, I plan to add more pix_fmt support for FFV1 v3 (which is already a good format!) support in FFmpeg and some optimization ideas beside my work for IETF spec, with the hope we could speak about technical issues (e.g. more optimization hints or info about wrong code or warning about that it breaks the bitstream specs as currently written), leaving aside politics. In the cellar charter and timeline, the effort to produce a informational specification for FFV1 video codec versions 0, 1 and 3 precedes the effort to produce a specification for FFV1 video codec version 4. Initially I anticipated that the version 4 specification would be a fork from the completed version 0,1,3 draft; however, I think the current approach (one doc that ‘makes’ both outputs) works well to allow version 4 work to proceed without blocking any remaining version 0,1,3 work. Still I suggest that we resolve https://github.com/FFmpeg/FFV1/pull/87 and https://github.com/FFmpeg/FFV1/pull/71 soon as IMHO the version 0,1,3 informational specification is very close to a last call and submission to the IESG (chairs are welcome to offer other opinions ;) ). So while I encourage resolution to these pull requests, it seems we have a good system to manage concurrent work on both FFV1 goals of the charter. Kind Regards, Dave Rice
diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c index 5eadb6b158..923b79f3ab 100644 --- a/libavcodec/ffv1dec.c +++ b/libavcodec/ffv1dec.c @@ -336,14 +336,16 @@ static int decode_slice(AVCodecContext *c, void *arg) decode_plane(fs, p->data[0] + ps*x + y*p->linesize[0] , width, height, p->linesize[0], 0, 2); decode_plane(fs, p->data[0] + ps*x + y*p->linesize[0] + 1, width, height, p->linesize[0], 1, 2); } else if (f->use32bit) { - uint8_t *planes[3] = { p->data[0] + ps * x + y * p->linesize[0], + uint8_t *planes[4] = { p->data[0] + ps * x + y * p->linesize[0], p->data[1] + ps * x + y * p->linesize[1], - p->data[2] + ps * x + y * p->linesize[2] }; + p->data[2] + ps * x + y * p->linesize[2], + p->data[3] + ps * x + y * p->linesize[3] }; decode_rgb_frame32(fs, planes, width, height, p->linesize); } else { - uint8_t *planes[3] = { p->data[0] + ps * x + y * p->linesize[0], + uint8_t *planes[4] = { p->data[0] + ps * x + y * p->linesize[0], p->data[1] + ps * x + y * p->linesize[1], - p->data[2] + ps * x + y * p->linesize[2] }; + p->data[2] + ps * x + y * p->linesize[2], + p->data[3] + ps * x + y * p->linesize[3] }; decode_rgb_frame(fs, planes, width, height, p->linesize); } if (fs->ac != AC_GOLOMB_RICE && f->version > 2) { @@ -694,6 +696,10 @@ static int read_header(FFV1Context *f) f->avctx->pix_fmt = AV_PIX_FMT_GBRP16; f->use32bit = 1; } + else if (f->avctx->bits_per_raw_sample == 16 && f->transparency) { + f->avctx->pix_fmt = AV_PIX_FMT_GBRAP16; + f->use32bit = 1; + } } else { av_log(f->avctx, AV_LOG_ERROR, "colorspace not supported\n"); return AVERROR(ENOSYS); diff --git a/libavcodec/ffv1dec_template.c b/libavcodec/ffv1dec_template.c index 37df766773..2904a44112 100644 --- a/libavcodec/ffv1dec_template.c +++ b/libavcodec/ffv1dec_template.c @@ -107,7 +107,7 @@ static av_always_inline int RENAME(decode_line)(FFV1Context *s, int w, return 0; } -static void RENAME(decode_rgb_frame)(FFV1Context *s, uint8_t *src[3], int w, int h, int stride[3]) +static void RENAME(decode_rgb_frame)(FFV1Context *s, uint8_t *src[4], int w, int h, int stride[4]) { int x, y, p; TYPE *sample[4][2]; @@ -158,10 +158,14 @@ static void RENAME(decode_rgb_frame)(FFV1Context *s, uint8_t *src[3], int w, int *((uint16_t*)(src[0] + x*2 + stride[0]*y)) = g; *((uint16_t*)(src[1] + x*2 + stride[1]*y)) = b; *((uint16_t*)(src[2] + x*2 + stride[2]*y)) = r; + if (s->transparency) + *((uint16_t*)(src[3] + x*2 + stride[3]*y)) = a; } else { *((uint16_t*)(src[0] + x*2 + stride[0]*y)) = b; *((uint16_t*)(src[1] + x*2 + stride[1]*y)) = g; *((uint16_t*)(src[2] + x*2 + stride[2]*y)) = r; + if (s->transparency) + *((uint16_t*)(src[3] + x*2 + stride[3]*y)) = a; } } } diff --git a/libavcodec/ffv1enc.c b/libavcodec/ffv1enc.c index c0c1558ffe..9ba5c6fcfd 100644 --- a/libavcodec/ffv1enc.c +++ b/libavcodec/ffv1enc.c @@ -624,6 +624,14 @@ FF_ENABLE_DEPRECATION_WARNINGS s->chroma_planes = 1; s->bits_per_raw_sample = 8; break; + case AV_PIX_FMT_RGBA64: + s->colorspace = 1; + s->transparency = 1; + s->chroma_planes = 1; + s->bits_per_raw_sample = 16; + s->use32bit = 1; + s->version = FFMAX(s->version, 1); + break; case AV_PIX_FMT_RGB48: s->colorspace = 1; s->chroma_planes = 1; @@ -649,10 +657,12 @@ FF_ENABLE_DEPRECATION_WARNINGS if (!avctx->bits_per_raw_sample && !s->bits_per_raw_sample) s->bits_per_raw_sample = 14; case AV_PIX_FMT_GBRP16: + case AV_PIX_FMT_GBRAP16: if (!avctx->bits_per_raw_sample && !s->bits_per_raw_sample) s->bits_per_raw_sample = 16; else if (!s->bits_per_raw_sample) s->bits_per_raw_sample = avctx->bits_per_raw_sample; + s->transparency = desc->nb_components == 4 || desc->nb_components == 2; s->colorspace = 1; s->chroma_planes = 1; if (s->bits_per_raw_sample >= 16) { @@ -1028,9 +1038,10 @@ static int encode_slice(AVCodecContext *c, void *arg) const int ps = av_pix_fmt_desc_get(c->pix_fmt)->comp[0].step; int ret; RangeCoder c_bak = fs->c; - const uint8_t *planes[3] = {p->data[0] + ps*x + y*p->linesize[0], + const uint8_t *planes[4] = {p->data[0] + ps*x + y*p->linesize[0], p->data[1] ? p->data[1] + ps*x + y*p->linesize[1] : NULL, - p->data[2] ? p->data[2] + ps*x + y*p->linesize[2] : NULL}; + p->data[2] ? p->data[2] + ps*x + y*p->linesize[2] : NULL, + p->data[3] ? p->data[3] + ps*x + y*p->linesize[3] : NULL}; fs->slice_coding_mode = 0; if (f->version > 3) { @@ -1322,6 +1333,7 @@ AVCodec ff_ffv1_encoder = { AV_PIX_FMT_YA8, AV_PIX_FMT_GRAY10, AV_PIX_FMT_GRAY12, AV_PIX_FMT_GBRP16, AV_PIX_FMT_RGB48, + AV_PIX_FMT_GBRAP16, AV_PIX_FMT_RGBA64, AV_PIX_FMT_NONE }, diff --git a/libavcodec/ffv1enc_template.c b/libavcodec/ffv1enc_template.c index b7eea0dd70..2763082540 100644 --- a/libavcodec/ffv1enc_template.c +++ b/libavcodec/ffv1enc_template.c @@ -122,8 +122,8 @@ static av_always_inline int RENAME(encode_line)(FFV1Context *s, int w, return 0; } -static int RENAME(encode_rgb_frame)(FFV1Context *s, const uint8_t *src[3], - int w, int h, const int stride[3]) +static int RENAME(encode_rgb_frame)(FFV1Context *s, const uint8_t *src[4], + int w, int h, const int stride[4]) { int x, y, p, i; const int ring_size = s->context_model ? 3 : 2; @@ -152,14 +152,18 @@ static int RENAME(encode_rgb_frame)(FFV1Context *s, const uint8_t *src[3], r = (v >> 16) & 0xFF; a = v >> 24; } else if (packed) { - const uint16_t *p = ((const uint16_t*)(src[0] + x*6 + stride[0]*y)); + const uint16_t *p = ((const uint16_t*)(src[0] + x*(3 + s->transparency)*2 + stride[0]*y)); r = p[0]; g = p[1]; b = p[2]; + if (s->transparency) + a = p[3]; } else if (sizeof(TYPE) == 4) { g = *((const uint16_t *)(src[0] + x*2 + stride[0]*y)); b = *((const uint16_t *)(src[1] + x*2 + stride[1]*y)); r = *((const uint16_t *)(src[2] + x*2 + stride[2]*y)); + if (s->transparency) + a = *((const uint16_t *)(src[3] + x*2 + stride[2]*y)); } else { b = *((const uint16_t *)(src[0] + x*2 + stride[0]*y)); g = *((const uint16_t *)(src[1] + x*2 + stride[1]*y));
Add support for 16-bit/component RGB with Alpha encoding and decoding in FFV1, both RGBA64 and GBRAP16 for encoding, GBRAP16 for decoding. Resulting bitstream was tested about lossless encoding/decoding by the compression from DPX to FFV1 then decompression from FFV1 to DPX, see commands below (resulting framemd5 hashes are all same). Resulting bitstream is decodable by another decoder (with same resulting framemd5 hash). Resulting bitstream passed through a conformance checker compared to current FFV1 specification IETF draft. About the patch: - some modified lines are not used (the ones not used when f->use32bit is 1), but it makes the code more coherent (especially because decode_rgb_frame signature is same for both 16-bit and 32-bit version) and prepares the support of RGBA with 10/12/14 bits/component. - GBRAP16 was chosen for decoding because GBRP16 is already used when no alpha, and the code is more prepared for planar pix_fmt when bit depth is >8. - "s->transparency = desc->nb_components == 4 || desc->nb_components == 2;" is a copy of a line a bit above about the detection of transparency, I preferred to reuse it as is even if "YA" 16-bit/component is not (yet) supported. FFmpeg commands used for tests: ./ffmpeg -i in.dpx -c:v ffv1 out.mkv ./ffmpeg -i in.dpx -pix_fmt gbrap16 -strict -2 -c:v ffv1 out2.mkv ./ffmpeg -i out.mkv out.dpx ./ffmpeg -i in.dpx -f framemd5 in.dpx.framemd5 ./ffmpeg -i out.mkv -pix_fmt rgba64be -f framemd5 out.mkv.framemd5 ./ffmpeg -i out2.mkv -pix_fmt rgba64be -f framemd5 out2.mkv.framemd5 ./ffmpeg -i out.dpx -f framemd5 out.dpx.framemd5 Test file used (renamed to in.dpx): https://mediaarea.net/temp/uncropped_DPX_4K_16bit_Overscan15pros.dpx Jérôme From 0e149afd7eadb1ec05cc79fff78337b2c543ad8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Martinez?= <jerome@mediaarea.net> Date: Thu, 1 Feb 2018 13:11:53 +0100 Subject: [PATCH] avcodec/ffv1: Support for RGBA64 and GBRAP16 --- libavcodec/ffv1dec.c | 14 ++++++++++---- libavcodec/ffv1dec_template.c | 6 +++++- libavcodec/ffv1enc.c | 16 ++++++++++++++-- libavcodec/ffv1enc_template.c | 10 +++++++--- 4 files changed, 36 insertions(+), 10 deletions(-)