[FFmpeg-devel] [PATCH v2 2/2] avcodec/libx265: add support for setting chroma sample location

[Jan Ekström]

On Wed, Sep 1, 2021 at 12:11 AM Jan Ekström <jeebjp at gmail.com> wrote:
>
> On Tue, Aug 31, 2021 at 12:52 AM Jan Ekström <jeebjp at gmail.com> wrote:
> >
> > On Sun, Aug 29, 2021 at 7:43 PM Jan Ekström <jeebjp at gmail.com> wrote:
> > >
> > > Unlike libx264, libx265 does not handle the chroma format check
> > > on its own side, so in order to not write out values which are
> > > supposed to be ignored according to the specification, we limit
> > > the writing out of chroma sample location to 4:2:0 only.
> > > ---
> > >  libavcodec/libx265.c | 13 +++++++++++++
> > >  1 file changed, 13 insertions(+)
> > >
> > > diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
> > > index 71affbf61b..839b6ce9de 100644
> > > --- a/libavcodec/libx265.c
> > > +++ b/libavcodec/libx265.c
> > > @@ -211,6 +211,19 @@ static av_cold int libx265_encode_init(AVCodecContext *avctx)
> > >          ctx->params->vui.matrixCoeffs            = avctx->colorspace;
> > >      }
> > >
> > > +    // chroma sample location values are to be ignored in case of non-4:2:0
> > > +    // according to the specification, so we only write them out in case of
> > > +    // 4:2:0 (log2_chroma_{w,h} == 1).
> > > +    ctx->params->vui.bEnableChromaLocInfoPresentFlag =
> > > +        avctx->chroma_sample_location != AVCHROMA_LOC_UNSPECIFIED &&
> > > +        desc->log2_chroma_w == 1 && desc->log2_chroma_h == 1;
> >
> > Checked this another time, and this check seems correct. HWAccel
> > pix_fmts like VAAPI are also marked as 4:2:0, but thankfully they're
> > not on the supported list for this module.
> >
> > For the source of the check, it is based on the following in the specification:
> >
> > Otherwise  (chroma_format_idc  is  not  equal  to  1),  the  values
> > of  the  syntax  elements chroma_sample_loc_type_top_field  and
> > chroma_sample_loc_type_bottom_field  shall  be  ignored.  When
> > chroma_format_idc is equal to 2 (4:2:2 chroma format) or 3 (4:4:4
> > chroma format), the location of chroma samples is specified in clause
> > 6.2. When chroma_format_idc is equal to 0, there is no chroma sample
> > array.
> >
> > > +
> > > +    if (ctx->params->vui.bEnableChromaLocInfoPresentFlag) {
> > > +        ctx->params->vui.chromaSampleLocTypeTopField =
> > > +        ctx->params->vui.chromaSampleLocTypeBottomField =
> > > +            avctx->chroma_sample_location - 1;
> > > +    }
> >
> > For progressive content both values should be set to the same value.
> >
> > For interlaced content in theory the chroma location field value is
> > not necessarily the same, but:
> > 1. we have a single value in our APIs
> > 2. x264 does this exact same thing, utilizes a single value interface
> > and sets both fields to the same value.
>
> Unless there are objections, I will pull this in soon.

Applied as 9b2281a4a383677ded522e603514789cc22498fa .

Jan