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

[Xiang, Haihao]

On Thu, 2021-08-05 at 16:05 +0000, Soft Works wrote:
> > -----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.

Could someone merge this patch if no more comments, please? 

Thanks
Haihao