[FFmpeg-devel] [PATCH v1 4/4] avcodec/proresenc_anatoliy: support for more color matrix for proresenc

[Marton Balint]

On Sun, 3 May 2020, lance.lmwang at gmail.com wrote:

> On Sun, May 03, 2020 at 09:53:09AM +0200, Marton Balint wrote:
>> 
>> 
>> On Tue, 5 Nov 2019, lance.lmwang at gmail.com wrote:
>> 
>> > From: Limin Wang <lance.lmwang at gmail.com>
>> > 
>> > Please tested with below command:
>> > ./ffmpeg -i ../fate-suite/mpeg2/t.mpg  -c:v prores_aw -color_primaries bt2020 -colorspace bt2020_ncl -color_trc smpte2084 -an output.mov
>> > 
>> > mediainfo outout.mov
>> > ...
>> > Color primaries                          : BT.2020
>> > Transfer characteristics                 : PQ
>> > Matrix coefficients                      : BT.2020 non-constant
>> > 
>> > ./ffmpeg -i ../fate-suite/mpeg2/t.mpg  -c:v prores_aw -color_primaries bt2020 -colorspace bt2020_ncl -color_trc arib-std-b67 -an output.mov
>> > mediainfo outout.mov
>> > ...
>> > Color primaries                          : BT.2020
>> > Transfer characteristics                 : HLG
>> > Matrix coefficients                      : BT.2020 non-constant
>> > 
>> > Signed-off-by: Limin Wang <lance.lmwang at gmail.com>
>> > ---
>> > libavcodec/proresenc_anatoliy.c | 9 +++++----
>> > 1 file changed, 5 insertions(+), 4 deletions(-)
>> > 
>> > diff --git a/libavcodec/proresenc_anatoliy.c b/libavcodec/proresenc_anatoliy.c
>> > index 0fc79fc1de..81365c528c 100644
>> > --- a/libavcodec/proresenc_anatoliy.c
>> > +++ b/libavcodec/proresenc_anatoliy.c
>> > @@ -55,7 +55,8 @@ static const int bitrate_table[6]  = { 1000, 2100, 3500, 5400, 7000, 10000};
>> > 
>> > static const int valid_primaries[9]  = { AVCOL_PRI_RESERVED0, AVCOL_PRI_BT709, AVCOL_PRI_UNSPECIFIED, AVCOL_PRI_BT470BG,
>> >                                          AVCOL_PRI_SMPTE170M, AVCOL_PRI_BT2020, AVCOL_PRI_SMPTE431, AVCOL_PRI_SMPTE432,INT_MAX };
>> > -static const int valid_trc[4]        = { AVCOL_TRC_RESERVED0, AVCOL_TRC_BT709, AVCOL_TRC_UNSPECIFIED, INT_MAX };
>> > +static const int valid_trc[6]        = { AVCOL_TRC_RESERVED0, AVCOL_TRC_BT709, AVCOL_TRC_UNSPECIFIED, AVCOL_TRC_SMPTE2084,
>> > +                                         AVCOL_TRC_ARIB_STD_B67, INT_MAX };
>> > static const int valid_colorspace[5] = { AVCOL_SPC_BT709, AVCOL_SPC_UNSPECIFIED, AVCOL_SPC_SMPTE170M,
>> >                                          AVCOL_SPC_BT2020_NCL, INT_MAX };
>> > 
>> > @@ -757,9 +758,9 @@ static int prores_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
>> >     *buf++ = frame_flags;
>> >     *buf++ = 0; /* reserved */
>> >     /* only write color properties, if valid value. set to unspecified otherwise */
>> > -    *buf++ = ff_int_from_list_or_default(avctx, "frame color primaries", pict->color_primaries, valid_primaries, 0);
>> > -    *buf++ = ff_int_from_list_or_default(avctx, "frame color trc", pict->color_trc, valid_trc, 0);
>> > -    *buf++ = ff_int_from_list_or_default(avctx, "frame colorspace", pict->colorspace, valid_colorspace, 0);
>> > +    *buf++ = ff_int_from_list_or_default(avctx, "frame color primaries", avctx->color_primaries, valid_primaries, 0);
>> > +    *buf++ = ff_int_from_list_or_default(avctx, "frame color trc", avctx->color_trc, valid_trc, 0);
>> > +    *buf++ = ff_int_from_list_or_default(avctx, "frame colorspace", avctx->colorspace, valid_colorspace, 0);
>> >     if (avctx->profile >= FF_PROFILE_PRORES_4444) {
>> >         if (avctx->pix_fmt == AV_PIX_FMT_YUV444P10) {
>> >             *buf++ = 0xA0;/* src b64a and no alpha */
>> 
>> Please revert this, using codec context instead of frame does not seem
>> right. Also have you pinged this before applying?
>
> Before revert it, I'm glad to know why use frame instead of avctx, by my testing command, it'll failed to get expected result
> without change to use avctx? also, most of encoder like mepg2, nvenc, vaapi, videotoolboxenc are using avctx context, what's
> the rule for that?

Prores writes the settings for each frame. Other encoders don't do this, 
on a per-frame basis, so it is not possible to signal the possibly 
different colorspace per frame. Anyway, this is not something that you 
should silently change in a patch which only claims to add a new color 
trc. Especially, if this changes behaviour you are experiencing.

Regards,
Marton