On Tue, Aug 31, 2021 at 12:52 AM Jan Ekström <jee...@gmail.com> wrote:
>
> On Sun, Aug 29, 2021 at 7:43 PM Jan Ekström <jee...@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.
Jan
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
