[FFmpeg-devel] [PATCH] lavfi/qsvvpp: do not mix up FFmpeg and SDK error code

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Xiang, Haihao
> Sent: Thursday, 5 August 2021 07:24
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] lavfi/qsvvpp: do not mix up FFmpeg
> and SDK error code
> 
> On Wed, 2021-08-04 at 06:34 +0000, Soft Works wrote:
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> > > Haihao Xiang
> > > Sent: Friday, 30 July 2021 04:39
> > > To: ffmpeg-devel at ffmpeg.org
> > > Cc: Haihao Xiang <haihao.xiang at intel.com>
> > > Subject: [FFmpeg-devel] [PATCH] lavfi/qsvvpp: do not mix up FFmpeg
> > > and SDK error code
> > >
> > > The function ff_qsvvpp_filter_frame should return a FFmpeg error
> > > code if there is an error. However it might return a SDK error code
> > > without this patch.
> > > ---
> > >  libavfilter/qsvvpp.c | 15 +++++++++------
> > >  1 file changed, 9 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/libavfilter/qsvvpp.c b/libavfilter/qsvvpp.c index
> > > 4768f6208b..c7ef8a915f 100644
> > > --- a/libavfilter/qsvvpp.c
> > > +++ b/libavfilter/qsvvpp.c
> > > @@ -807,8 +807,7 @@ int ff_qsvvpp_filter_frame(QSVVPPContext *s,
> > > AVFilterLink *inlink, AVFrame *picr
> > >        if (MFXVideoCORE_SyncOperation(s->session, sync, 1000) < 0)
> > >            av_log(ctx, AV_LOG_WARNING, "Sync failed.\n");
> >
> > Why no looping and no checking for MFX_WRN_IN_EXECUTION like
> below?
> 
> Thanks for catching this, I think it should check for
> MFX_WRN_IN_EXECUTION, but it should be fixed in another patch.

OK. 

> > >
> > >          filter_ret = s->filter_frame(outlink, tmp->frame);
> > >          if (filter_ret < 0) {
> > >              av_frame_free(&tmp->frame);
> > > -            ret = filter_ret;
> > > -            break;
> > > +            return filter_ret;
> >
> > The title is about not to mix error codes, but this is a behavioral change.
> > After the patch, the input frame would no longer be processed in case
> > of a sync error.
> 
> The condition is 's->eof && qsv_fifo_size(s->async_fifo)'. When s->eof is
> true, the input frame is actually NULL. So without this patch, this function
> returns 0 for this case, the error code is ignored.

My bad, I missed the eof condition. 


All details clarified. Patch LGTM.

softworkz