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

[James Almer]

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.