> -----Original Message----- > From: Li, Zhong > Sent: Monday, February 18, 2019 13:48 > To: FFmpeg development discussions and patches <ffmpeg- > de...@ffmpeg.org> > Cc: Fu, Linjie <linjie...@intel.com> > Subject: RE: [FFmpeg-devel] [PATCH, RFC] lavf/deinterlace_qsv: set TFF or > BFF together with progressive > > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On > Behalf > > Of Linjie Fu > > Sent: Saturday, February 16, 2019 3:50 AM > > To: ffmpeg-devel@ffmpeg.org > > Cc: Fu, Linjie <linjie...@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...@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)
Thanks, it seems more reasonable in the repeated_first_field cases in #7701. But I still have concerns: Progressive| TFF or Progressive | BFF frames will never be set after the modification. A discussion on " Is progresive TFF or BFF in mpeg2" : https://forum.doom9.org/showthread.php?t=165176 I think it is a more common way to suit all probable cases (not only in #7701): Allow FFmpeg qsv to set all attributes got from each frame, and pass down to MSDK/driver. It depends on MSDK/driver to decide whether the TFF and BFF flag is valid and useful in progressive mode. -- Linjie _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
