diff mbox series

[FFmpeg-devel,2/2] avcodec/libx265: improve full range flag setting logic

Message ID 20210817210032.17597-2-jeebjp@gmail.com
State Accepted
Commit dbe40478e293f78c5a5e4302eb3f38257106ee86
Headers show
Series [FFmpeg-devel,1/2] avcodec/libx264: leave full range flag unchanged if unknown | expand

Checks

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

Commit Message

Jan Ekström Aug. 17, 2021, 9 p.m. UTC
Unlike libx264, libx265 does not have a separate "unspecified"/"auto"
default for color range, so we do always have to specify it.
Thus, we are required to handle the RGB case on the libavcodec
side to enable the correct value to be written out in in case
of RGB content with unspecified color range being received.

In other words:
1. If the user has set color range specifically, follow that.
2. If the user has not set color range specifically, set full
   range by default in case of RGB and YUVJ pixel formats.
---
 libavcodec/libx265.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

Comments

mypopy@gmail.com Aug. 18, 2021, 1:35 a.m. UTC | #1
On Wed, Aug 18, 2021 at 5:01 AM Jan Ekström <jeebjp@gmail.com> wrote:
>
> Unlike libx264, libx265 does not have a separate "unspecified"/"auto"
> default for color range, so we do always have to specify it.
> Thus, we are required to handle the RGB case on the libavcodec
> side to enable the correct value to be written out in in case
> of RGB content with unspecified color range being received.
>
> In other words:
> 1. If the user has set color range specifically, follow that.
> 2. If the user has not set color range specifically, set full
>    range by default in case of RGB and YUVJ pixel formats.
> ---
>  libavcodec/libx265.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
> index 9395120471..b5c94b64a3 100644
> --- a/libavcodec/libx265.c
> +++ b/libavcodec/libx265.c
> @@ -181,10 +181,15 @@ static av_cold int libx265_encode_init(AVCodecContext *avctx)
>
>      ctx->params->vui.bEnableVideoSignalTypePresentFlag = 1;
>
> -    ctx->params->vui.bEnableVideoFullRangeFlag = avctx->pix_fmt == AV_PIX_FMT_YUVJ420P ||
> -                                                 avctx->pix_fmt == AV_PIX_FMT_YUVJ422P ||
> -                                                 avctx->pix_fmt == AV_PIX_FMT_YUVJ444P ||
> -                                                 avctx->color_range == AVCOL_RANGE_JPEG;
> +    if (avctx->color_range != AVCOL_RANGE_UNSPECIFIED)
> +        ctx->params->vui.bEnableVideoFullRangeFlag =
> +            avctx->color_range == AVCOL_RANGE_JPEG;
> +    else
> +        ctx->params->vui.bEnableVideoFullRangeFlag =
> +            (av_pix_fmt_desc_get(avctx->pix_fmt)->flags & AV_PIX_FMT_FLAG_RGB) ||
> +            avctx->pix_fmt == AV_PIX_FMT_YUVJ420P ||
> +            avctx->pix_fmt == AV_PIX_FMT_YUVJ422P ||
> +            avctx->pix_fmt == AV_PIX_FMT_YUVJ444P;
>
>      if ((avctx->color_primaries <= AVCOL_PRI_SMPTE432 &&
>           avctx->color_primaries != AVCOL_PRI_UNSPECIFIED) ||
> --
> 2.31.1
Patchset LGTM
Jan Ekström Aug. 18, 2021, 9:13 a.m. UTC | #2
On Wed, Aug 18, 2021 at 4:36 AM mypopy@gmail.com <mypopy@gmail.com> wrote:
>
> On Wed, Aug 18, 2021 at 5:01 AM Jan Ekström <jeebjp@gmail.com> wrote:
> >
> > Unlike libx264, libx265 does not have a separate "unspecified"/"auto"
> > default for color range, so we do always have to specify it.
> > Thus, we are required to handle the RGB case on the libavcodec
> > side to enable the correct value to be written out in in case
> > of RGB content with unspecified color range being received.
> >
> > In other words:
> > 1. If the user has set color range specifically, follow that.
> > 2. If the user has not set color range specifically, set full
> >    range by default in case of RGB and YUVJ pixel formats.
> > ---
> >  libavcodec/libx265.c | 13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
> > index 9395120471..b5c94b64a3 100644
> > --- a/libavcodec/libx265.c
> > +++ b/libavcodec/libx265.c
> > @@ -181,10 +181,15 @@ static av_cold int libx265_encode_init(AVCodecContext *avctx)
> >
> >      ctx->params->vui.bEnableVideoSignalTypePresentFlag = 1;
> >
> > -    ctx->params->vui.bEnableVideoFullRangeFlag = avctx->pix_fmt == AV_PIX_FMT_YUVJ420P ||
> > -                                                 avctx->pix_fmt == AV_PIX_FMT_YUVJ422P ||
> > -                                                 avctx->pix_fmt == AV_PIX_FMT_YUVJ444P ||
> > -                                                 avctx->color_range == AVCOL_RANGE_JPEG;
> > +    if (avctx->color_range != AVCOL_RANGE_UNSPECIFIED)
> > +        ctx->params->vui.bEnableVideoFullRangeFlag =
> > +            avctx->color_range == AVCOL_RANGE_JPEG;
> > +    else
> > +        ctx->params->vui.bEnableVideoFullRangeFlag =
> > +            (av_pix_fmt_desc_get(avctx->pix_fmt)->flags & AV_PIX_FMT_FLAG_RGB) ||
> > +            avctx->pix_fmt == AV_PIX_FMT_YUVJ420P ||
> > +            avctx->pix_fmt == AV_PIX_FMT_YUVJ422P ||
> > +            avctx->pix_fmt == AV_PIX_FMT_YUVJ444P;
> >
> >      if ((avctx->color_primaries <= AVCOL_PRI_SMPTE432 &&
> >           avctx->color_primaries != AVCOL_PRI_UNSPECIFIED) ||
> > --
> > 2.31.1
> Patchset LGTM

Thanks, applied as 7ca71b79f2b3256a0eef1a099b857ac9e4017e36 and
dbe40478e293f78c5a5e4302eb3f38257106ee86 .

Jan
diff mbox series

Patch

diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
index 9395120471..b5c94b64a3 100644
--- a/libavcodec/libx265.c
+++ b/libavcodec/libx265.c
@@ -181,10 +181,15 @@  static av_cold int libx265_encode_init(AVCodecContext *avctx)
 
     ctx->params->vui.bEnableVideoSignalTypePresentFlag = 1;
 
-    ctx->params->vui.bEnableVideoFullRangeFlag = avctx->pix_fmt == AV_PIX_FMT_YUVJ420P ||
-                                                 avctx->pix_fmt == AV_PIX_FMT_YUVJ422P ||
-                                                 avctx->pix_fmt == AV_PIX_FMT_YUVJ444P ||
-                                                 avctx->color_range == AVCOL_RANGE_JPEG;
+    if (avctx->color_range != AVCOL_RANGE_UNSPECIFIED)
+        ctx->params->vui.bEnableVideoFullRangeFlag =
+            avctx->color_range == AVCOL_RANGE_JPEG;
+    else
+        ctx->params->vui.bEnableVideoFullRangeFlag =
+            (av_pix_fmt_desc_get(avctx->pix_fmt)->flags & AV_PIX_FMT_FLAG_RGB) ||
+            avctx->pix_fmt == AV_PIX_FMT_YUVJ420P ||
+            avctx->pix_fmt == AV_PIX_FMT_YUVJ422P ||
+            avctx->pix_fmt == AV_PIX_FMT_YUVJ444P;
 
     if ((avctx->color_primaries <= AVCOL_PRI_SMPTE432 &&
          avctx->color_primaries != AVCOL_PRI_UNSPECIFIED) ||