[FFmpeg-devel] [PATCH, RFC] lavf/deinterlace_qsv: set TFF or BFF together with progressive

[Li, Zhong]

> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
> Of Linjie Fu
> Sent: Saturday, February 16, 2019 3:50 AM
> To: ffmpeg-devel at ffmpeg.org
> Cc: Fu, Linjie <linjie.fu at intel.com>
> Subject: [FFmpeg-devel] [PATCH, RFC] lavf/deinterlace_qsv: set TFF or BFF
> together with progressive
> 
> Currently, BFF or TFF will not be set if the frame is progressive.
> This will break the Input PicStruct check in MSDK because of absence of the
> specific PicStruct check for:
> MFX_PICSTRUCT_PROGRESSIVE | MFX_PICSTRUCT_FIELD_REPEATED
> 
> Set PicStruct to MFX_PICSTRUCT_FIELD_TFF or MFX_PICSTRUCT_FIELD_BFF
> to match the CheckInputPicStruct in MSDK.
> 
> Fix #7701.

After checking this ticket, I believe this is not a MSDK issue, and current fix in this patch is not correct. 
If I understand correctly, this issue only happens when H264 pic_struct = 5 or 6.
In this case, MFX_PICSTRUCT_FIELD_TFF or MFX_PICSTRUCT_FIELD_BFF should be set, else we don't know which filed should be repeated.

> Signed-off-by: Linjie Fu <linjie.fu at intel.com>
> ---
> Is it acceptable to add the TFF or BFF to PicStruct according to the attribute
> of the input frames, even if it's not interlaced?
> Or it should be fixed in MSDK level to support more PicStruct?
> 
>  libavfilter/vf_deinterlace_qsv.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/libavfilter/vf_deinterlace_qsv.c b/libavfilter/vf_deinterlace_qsv.c
> index d6b02e98c5..f03d65f029 100644
> --- a/libavfilter/vf_deinterlace_qsv.c
> +++ b/libavfilter/vf_deinterlace_qsv.c
> @@ -417,8 +417,9 @@ static int submit_frame(AVFilterContext *ctx,
> AVFrame *frame,
>      qf->surface.Info.CropH  = qf->frame->height;
> 
>      qf->surface.Info.PicStruct = !qf->frame->interlaced_frame ?
> MFX_PICSTRUCT_PROGRESSIVE :
> -                                 (qf->frame->top_field_first ?
> MFX_PICSTRUCT_FIELD_TFF :
> -
> MFX_PICSTRUCT_FIELD_BFF);
> +
> MFX_PICSTRUCT_UNKNOWN;
> +    qf->surface.Info.PicStruct |= qf->frame->top_field_first ?
> MFX_PICSTRUCT_FIELD_TFF :
> +
> + MFX_PICSTRUCT_FIELD_BFF;
>      if (qf->frame->repeat_pict == 1)
>          qf->surface.Info.PicStruct |= MFX_PICSTRUCT_FIELD_REPEATED;

I believe the correct fix should be here (previous code change is not needed and should be removed):
if (qf->frame->repeat_pict == 1) {
  qf->surface.Info.PicStruct |= MFX_PICSTRUCT_FIELD_REPEATED;
  qf->surface.Info.PicStruct |= qf->frame->top_field_first ? MFX_PICSTRUCT_FIELD_TFF : MFX_PICSTRUCT_FIELD_BFF;
}

>      else if (qf->frame->repeat_pict == 2)