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

[Jan Ekström]

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.

Jan