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

[Xiang, Haihao]

On Mon, 2021-08-09 at 22:58 -0300, James Almer wrote:
> On 8/9/2021 10:55 PM, Xiang, Haihao wrote:
> > 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
> 
> Pushed.

Thanks James.