diff mbox series

[FFmpeg-devel] lavc/aomdec: Allow RGB decoding

Message ID CAB0OVGpWUwn=NZ27b0bqeDqwr5MErfaUhPVpV0GBp2bYRjZYUQ@mail.gmail.com
State New
Headers show
Series [FFmpeg-devel] lavc/aomdec: Allow RGB decoding | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make warning Make failed

Commit Message

Carl Eugen Hoyos Oct. 11, 2020, 5:27 p.m. UTC
Hi!

Attached patch fixes ticket #8929 for me.

Please comment, Carl Eugen
Subject: [PATCH] lavc/aomdec: Allow RGB decoding.

Fixes ticket #8929.
---
 libavcodec/libaomdec.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

Comments

James Almer Oct. 11, 2020, 5:51 p.m. UTC | #1
On 10/11/2020 2:27 PM, Carl Eugen Hoyos wrote:
> Hi!
> 
> Attached patch fixes ticket #8929 for me.
> 
> Please comment, Carl Eugen

> From 580477c4b0e07fa5436474ae732596bed5faf2d4 Mon Sep 17 00:00:00 2001
> From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
> Date: Sun, 11 Oct 2020 19:25:10 +0200
> Subject: [PATCH] lavc/aomdec: Allow RGB decoding.
> 
> Fixes ticket #8929.
> ---
>  libavcodec/libaomdec.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/libaomdec.c b/libavcodec/libaomdec.c
> index 1430a651fe..f83c11ddf8 100644
> --- a/libavcodec/libaomdec.c
> +++ b/libavcodec/libaomdec.c
> @@ -134,15 +134,27 @@ static int set_pix_fmt(AVCodecContext *avctx, struct aom_image *img)
>      case AOM_IMG_FMT_I444:
>      case AOM_IMG_FMT_I44416:
>          if (img->bit_depth == 8) {
> -            avctx->pix_fmt = AV_PIX_FMT_YUV444P;
> +            if (avctx->color_trc == AVCOL_TRC_IEC61966_2_1) {

AV1 signals RGB with the combination of AVCOL_SPC_RGB, AVCOL_PRI_BT709
and AVCOL_TRC_IEC61966_2_1, not just the latter alone.

Should be good with that change.

> +                avctx->pix_fmt = AV_PIX_FMT_GBRP;
> +            } else {
> +                avctx->pix_fmt = AV_PIX_FMT_YUV444P;
> +            }
>              avctx->profile = FF_PROFILE_AV1_HIGH;
>              return 0;
>          } else if (img->bit_depth == 10) {
> -            avctx->pix_fmt = AV_PIX_FMT_YUV444P10;
> +            if (avctx->color_trc == AVCOL_TRC_IEC61966_2_1) {
> +                avctx->pix_fmt = AV_PIX_FMT_GBRP10;
> +            } else {
> +                avctx->pix_fmt = AV_PIX_FMT_YUV444P10;
> +            }
>              avctx->profile = FF_PROFILE_AV1_HIGH;
>              return 0;
>          } else if (img->bit_depth == 12) {
> -            avctx->pix_fmt = AV_PIX_FMT_YUV444P12;
> +            if (avctx->color_trc == AVCOL_TRC_IEC61966_2_1) {
> +                avctx->pix_fmt = AV_PIX_FMT_GBRP12;
> +            } else {
> +                avctx->pix_fmt = AV_PIX_FMT_YUV444P12;
> +            }
>              avctx->profile = FF_PROFILE_AV1_PROFESSIONAL;
>              return 0;
>          } else {
> -- 
> 2.24.1
>
Carl Eugen Hoyos Oct. 11, 2020, 6:07 p.m. UTC | #2
Am So., 11. Okt. 2020 um 19:51 Uhr schrieb James Almer <jamrial@gmail.com>:
>
> On 10/11/2020 2:27 PM, Carl Eugen Hoyos wrote:
> > Hi!
> >
> > Attached patch fixes ticket #8929 for me.
> >
> > Please comment, Carl Eugen
>
> > From 580477c4b0e07fa5436474ae732596bed5faf2d4 Mon Sep 17 00:00:00 2001
> > From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
> > Date: Sun, 11 Oct 2020 19:25:10 +0200
> > Subject: [PATCH] lavc/aomdec: Allow RGB decoding.
> >
> > Fixes ticket #8929.
> > ---
> >  libavcodec/libaomdec.c | 18 +++++++++++++++---
> >  1 file changed, 15 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavcodec/libaomdec.c b/libavcodec/libaomdec.c
> > index 1430a651fe..f83c11ddf8 100644
> > --- a/libavcodec/libaomdec.c
> > +++ b/libavcodec/libaomdec.c
> > @@ -134,15 +134,27 @@ static int set_pix_fmt(AVCodecContext *avctx, struct aom_image *img)
> >      case AOM_IMG_FMT_I444:
> >      case AOM_IMG_FMT_I44416:
> >          if (img->bit_depth == 8) {
> > -            avctx->pix_fmt = AV_PIX_FMT_YUV444P;
> > +            if (avctx->color_trc == AVCOL_TRC_IEC61966_2_1) {
>
> AV1 signals RGB with the combination of AVCOL_SPC_RGB, AVCOL_PRI_BT709
> and AVCOL_TRC_IEC61966_2_1, not just the latter alone.

Testing for bt709 seems simply wrong to me.

Concerning the "colorspace":
Is it possible that this is badly specified?
Since we don't do this so far, it should not be part of this patch imo.

Carl Eugen
James Almer Oct. 12, 2020, 12:52 a.m. UTC | #3
On 10/11/2020 3:07 PM, Carl Eugen Hoyos wrote:
> Am So., 11. Okt. 2020 um 19:51 Uhr schrieb James Almer <jamrial@gmail.com>:
>>
>> On 10/11/2020 2:27 PM, Carl Eugen Hoyos wrote:
>>> Hi!
>>>
>>> Attached patch fixes ticket #8929 for me.
>>>
>>> Please comment, Carl Eugen
>>
>>> From 580477c4b0e07fa5436474ae732596bed5faf2d4 Mon Sep 17 00:00:00 2001
>>> From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
>>> Date: Sun, 11 Oct 2020 19:25:10 +0200
>>> Subject: [PATCH] lavc/aomdec: Allow RGB decoding.
>>>
>>> Fixes ticket #8929.
>>> ---
>>>  libavcodec/libaomdec.c | 18 +++++++++++++++---
>>>  1 file changed, 15 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/libavcodec/libaomdec.c b/libavcodec/libaomdec.c
>>> index 1430a651fe..f83c11ddf8 100644
>>> --- a/libavcodec/libaomdec.c
>>> +++ b/libavcodec/libaomdec.c
>>> @@ -134,15 +134,27 @@ static int set_pix_fmt(AVCodecContext *avctx, struct aom_image *img)
>>>      case AOM_IMG_FMT_I444:
>>>      case AOM_IMG_FMT_I44416:
>>>          if (img->bit_depth == 8) {
>>> -            avctx->pix_fmt = AV_PIX_FMT_YUV444P;
>>> +            if (avctx->color_trc == AVCOL_TRC_IEC61966_2_1) {
>>
>> AV1 signals RGB with the combination of AVCOL_SPC_RGB, AVCOL_PRI_BT709
>> and AVCOL_TRC_IEC61966_2_1, not just the latter alone.
> 
> Testing for bt709 seems simply wrong to me.
> 
> Concerning the "colorspace":
> Is it possible that this is badly specified?

I have no authority to say if it's wrong or not, but i would expect
something like this to caught earlier on during finalization if it was
wrong.

> Since we don't do this so far, it should not be part of this patch imo.

Both the libaom-av1 encoder and libdav1d decoder currently do it, given
it's what the spec states.

> 
> Carl Eugen
> _______________________________________________
> 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 series

Patch

diff --git a/libavcodec/libaomdec.c b/libavcodec/libaomdec.c
index 1430a651fe..f83c11ddf8 100644
--- a/libavcodec/libaomdec.c
+++ b/libavcodec/libaomdec.c
@@ -134,15 +134,27 @@  static int set_pix_fmt(AVCodecContext *avctx, struct aom_image *img)
     case AOM_IMG_FMT_I444:
     case AOM_IMG_FMT_I44416:
         if (img->bit_depth == 8) {
-            avctx->pix_fmt = AV_PIX_FMT_YUV444P;
+            if (avctx->color_trc == AVCOL_TRC_IEC61966_2_1) {
+                avctx->pix_fmt = AV_PIX_FMT_GBRP;
+            } else {
+                avctx->pix_fmt = AV_PIX_FMT_YUV444P;
+            }
             avctx->profile = FF_PROFILE_AV1_HIGH;
             return 0;
         } else if (img->bit_depth == 10) {
-            avctx->pix_fmt = AV_PIX_FMT_YUV444P10;
+            if (avctx->color_trc == AVCOL_TRC_IEC61966_2_1) {
+                avctx->pix_fmt = AV_PIX_FMT_GBRP10;
+            } else {
+                avctx->pix_fmt = AV_PIX_FMT_YUV444P10;
+            }
             avctx->profile = FF_PROFILE_AV1_HIGH;
             return 0;
         } else if (img->bit_depth == 12) {
-            avctx->pix_fmt = AV_PIX_FMT_YUV444P12;
+            if (avctx->color_trc == AVCOL_TRC_IEC61966_2_1) {
+                avctx->pix_fmt = AV_PIX_FMT_GBRP12;
+            } else {
+                avctx->pix_fmt = AV_PIX_FMT_YUV444P12;
+            }
             avctx->profile = FF_PROFILE_AV1_PROFESSIONAL;
             return 0;
         } else {