[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
> 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?

>
>          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.

Also, shouldn't you call 

tmp->queued--;

before exiting?

>          }
>          tmp->queued--;
>          s->got_frame = 1;
> @@ -842,7 +841,7 @@ int ff_qsvvpp_filter_frame(QSVVPPContext *s,
> AVFilterLink *inlink, AVFrame *picr
>          if (ret < 0 && ret != MFX_ERR_MORE_SURFACE) {
>              /* Ignore more_data error */
>              if (ret == MFX_ERR_MORE_DATA)
> -                ret = AVERROR(EAGAIN);
> +                return AVERROR(EAGAIN);
>              break;
>          }

I guess that makes sense. In case of overlay, we need to return immediately
to let the caller submit the overlay surface.

>          out_frame->frame->pts = av_rescale_q(out_frame-
> >surface.Data.TimeStamp,
> @@ -864,8 +863,7 @@ int ff_qsvvpp_filter_frame(QSVVPPContext *s,
> AVFilterLink *inlink, AVFrame *picr
>              filter_ret = s->filter_frame(outlink, tmp->frame);
>              if (filter_ret < 0) {
>                  av_frame_free(&tmp->frame);
> -                ret = filter_ret;
> -                break;
> +                return filter_ret;
>              }
> 
>              tmp->queued--;

Same as above: Shouldn't this be called before returning?


softworkz