[FFmpeg-devel] lavc/vaapi_encode_h265: fix conf_win_xxx_offset for 4:2:2/4:4:4 encoding

Based on Table 6-1, set SubWidth and SubHeightC depending on chroma format.

Based on D-28 and D-29, set the correct cropped width/height.

croppedWidth  = pic_width_in_luma_samples −
                SubWidthC * ( conf_win_right_offset + conf_win_left_offset );

croppedHeight = pic_height_in_luma_samples −
                SubHeightC * ( conf_win_bottom_offset + conf_win_top_offset );

Signed-off-by: Linjie Fu <linjie.fu@intel.com>
---
 libavcodec/vaapi_encode_h265.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

On Thu, Mar 05, 2020 at 15:41:17 +0800, Linjie Fu wrote:
> +    int SubWidthC, SubHeightC;

Style nit:
ffmpeg's own variables don't use CamelCase.

Moritz

> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Moritz Barsnick
> Sent: Friday, March 6, 2020 18:24
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] lavc/vaapi_encode_h265: fix
> conf_win_xxx_offset for 4:2:2/4:4:4 encoding
> 
> On Thu, Mar 05, 2020 at 15:41:17 +0800, Linjie Fu wrote:
> > +    int SubWidthC, SubHeightC;
> 
> Style nit:
> ffmpeg's own variables don't use CamelCase.
> 
Will fix, thanks.

- Linjie

On 05/03/2020 07:41, Linjie Fu wrote:
> Based on Table 6-1, set SubWidth and SubHeightC depending on chroma format.
> 
> Based on D-28 and D-29, set the correct cropped width/height.
> 
> croppedWidth  = pic_width_in_luma_samples −
>                 SubWidthC * ( conf_win_right_offset + conf_win_left_offset );
> 
> croppedHeight = pic_height_in_luma_samples −
>                 SubHeightC * ( conf_win_bottom_offset + conf_win_top_offset );
> 
> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> ---
>  libavcodec/vaapi_encode_h265.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c
> index 12f0e6f..db1bf24 100644
> --- a/libavcodec/vaapi_encode_h265.c
> +++ b/libavcodec/vaapi_encode_h265.c
> @@ -268,6 +268,7 @@ static int vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)
>      VAEncPictureParameterBufferHEVC  *vpic = ctx->codec_picture_params;
>      const AVPixFmtDescriptor *desc;
>      int chroma_format, bit_depth;
> +    int SubWidthC, SubHeightC;
>      int i;
>  
>      memset(vps, 0, sizeof(*vps));
> @@ -405,15 +406,19 @@ static int vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)
>      sps->pic_width_in_luma_samples  = ctx->surface_width;
>      sps->pic_height_in_luma_samples = ctx->surface_height;
>  
> +    // Table 6-1
> +    SubWidthC  = chroma_format == 1 || chroma_format == 2 ? 2 : 1;
> +    SubHeightC = chroma_format == 1 ? 2 : 1;

You don't need to reverse chroma format into these value - desc->log2_chroma_* as used above to calculate it above is still available.

> +
>      if (avctx->width  != ctx->surface_width ||
>          avctx->height != ctx->surface_height) {
>          sps->conformance_window_flag = 1;
>          sps->conf_win_left_offset   = 0;
>          sps->conf_win_right_offset  =
> -            (ctx->surface_width - avctx->width) / 2;
> +            (ctx->surface_width - avctx->width) / SubWidthC;

(...width) >> desc->log2_chroma_w

>          sps->conf_win_top_offset    = 0;
>          sps->conf_win_bottom_offset =
> -            (ctx->surface_height - avctx->height) / 2;
> +            (ctx->surface_height - avctx->height) / SubHeightC;

(...height) >> desc->log2_chroma_h

>      } else {
>          sps->conformance_window_flag = 0;
>      }
> 

Thanks,

- Mark

> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Mark Thompson
> Sent: Sunday, March 8, 2020 00:35
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] lavc/vaapi_encode_h265: fix
> conf_win_xxx_offset for 4:2:2/4:4:4 encoding
> 
> On 05/03/2020 07:41, Linjie Fu wrote:
> > Based on Table 6-1, set SubWidth and SubHeightC depending on chroma
> format.
> >
> > Based on D-28 and D-29, set the correct cropped width/height.
> >
> > croppedWidth  = pic_width_in_luma_samples −
> >                 SubWidthC * ( conf_win_right_offset + conf_win_left_offset );
> >
> > croppedHeight = pic_height_in_luma_samples −
> >                 SubHeightC * ( conf_win_bottom_offset + conf_win_top_offset );
> >
> > Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> > ---
> >  libavcodec/vaapi_encode_h265.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/vaapi_encode_h265.c
> b/libavcodec/vaapi_encode_h265.c
> > index 12f0e6f..db1bf24 100644
> > --- a/libavcodec/vaapi_encode_h265.c
> > +++ b/libavcodec/vaapi_encode_h265.c
> > @@ -268,6 +268,7 @@ static int
> vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)
> >      VAEncPictureParameterBufferHEVC  *vpic = ctx->codec_picture_params;
> >      const AVPixFmtDescriptor *desc;
> >      int chroma_format, bit_depth;
> > +    int SubWidthC, SubHeightC;
> >      int i;
> >
> >      memset(vps, 0, sizeof(*vps));
> > @@ -405,15 +406,19 @@ static int
> vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)
> >      sps->pic_width_in_luma_samples  = ctx->surface_width;
> >      sps->pic_height_in_luma_samples = ctx->surface_height;
> >
> > +    // Table 6-1
> > +    SubWidthC  = chroma_format == 1 || chroma_format == 2 ? 2 : 1;
> > +    SubHeightC = chroma_format == 1 ? 2 : 1;
> 
> You don't need to reverse chroma format into these value - desc-
> >log2_chroma_* as used above to calculate it above is still available.

Updated, thanks for the review.

- Linjie