diff mbox

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

Message ID 20191105023151.28113-1-lance.lmwang@gmail.com
State Accepted
Commit e0eed1fd523ec5d0cc390a08c468dbc57316378a
Headers show

Commit Message

Lance Wang Nov. 5, 2019, 2:31 a.m. UTC 
From: Limin Wang <lance.lmwang@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@gmail.com>
---
 libavcodec/proresenc_anatoliy.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Marton Balint May 3, 2020, 7:53 a.m. UTC | #1 
On Tue, 5 Nov 2019, lance.lmwang@gmail.com wrote:

> From: Limin Wang <lance.lmwang@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@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?

Thanks,
Marton
Lance Wang May 3, 2020, 12:53 p.m. UTC | #2 
On Sun, May 03, 2020 at 09:53:09AM +0200, Marton Balint wrote:
> 
> 
> On Tue, 5 Nov 2019, lance.lmwang@gmail.com wrote:
> 
> > From: Limin Wang <lance.lmwang@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@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?

> 
> Thanks,
> Marton
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Marton Balint May 3, 2020, 5:01 p.m. UTC | #3 
On Sun, 3 May 2020, lance.lmwang@gmail.com wrote:

> On Sun, May 03, 2020 at 09:53:09AM +0200, Marton Balint wrote:
>> 
>> 
>> On Tue, 5 Nov 2019, lance.lmwang@gmail.com wrote:
>> 
>> > From: Limin Wang <lance.lmwang@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@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
Lance Wang May 3, 2020, 9:45 p.m. UTC | #4 
On Sun, May 03, 2020 at 07:01:36PM +0200, Marton Balint wrote:
> 
> 
> On Sun, 3 May 2020, lance.lmwang@gmail.com wrote:
> 
> > On Sun, May 03, 2020 at 09:53:09AM +0200, Marton Balint wrote:
> > > 
> > > 
> > > On Tue, 5 Nov 2019, lance.lmwang@gmail.com wrote:
> > > 
> > > > From: Limin Wang <lance.lmwang@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@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.

thanks, revert it anyway.

> 
> Regards,
> Marton
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox

Patch

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 */