[FFmpeg-devel] [PATCH 3/4] avutil/cuda_check: propagate AVERROR_UNRECOVERABLE when needed

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Timo Rothenpieler
> Sent: Tuesday, November 22, 2022 11:59 PM
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH 3/4] avutil/cuda_check: propagate
> AVERROR_UNRECOVERABLE when needed
> 
> On 22.11.2022 17:02, Soft Works wrote:
> >
> >
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> >> Timo Rothenpieler
> >> Sent: Tuesday, November 22, 2022 4:16 PM
> >> To: ffmpeg-devel at ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] [PATCH 3/4] avutil/cuda_check:
> propagate
> >> AVERROR_UNRECOVERABLE when needed
> >>
> >>
> >>
> >> On 22/11/2022 14:31, James Almer wrote:
> >>> On 11/22/2022 10:21 AM, Timo Rothenpieler wrote:
> >>>> On 22/11/2022 14:07, James Almer wrote:
> >>>>> Based on a patch by Soft Works.
> >>>>>
> >>>>> Signed-off-by: James Almer <jamrial at gmail.com>
> >>>>> ---
> >>>>>    libavutil/cuda_check.h | 4 ++++
> >>>>>    1 file changed, 4 insertions(+)
> >>>>>
> >>>>> diff --git a/libavutil/cuda_check.h b/libavutil/cuda_check.h
> >>>>> index f5a9234eaf..33aaf9c098 100644
> >>>>> --- a/libavutil/cuda_check.h
> >>>>> +++ b/libavutil/cuda_check.h
> >>>>> @@ -49,6 +49,10 @@ static inline int ff_cuda_check(void *avctx,
> >>>>>            av_log(avctx, AV_LOG_ERROR, " -> %s: %s", err_name,
> >>>>> err_string);
> >>>>>        av_log(avctx, AV_LOG_ERROR, "\n");
> >>>>> +    // Not recoverable
> >>>>> +    if (err == CUDA_ERROR_UNKNOWN)
> >>>>> +        return AVERROR_UNRECOVERABLE;
> >>>>
> >>>> Why does specifically CUDA_ERROR_UNKNOWN get mapped to
> >> unrecoverable?
> >>>
> >>> It's the code that Soft Works found out was returned repeatedly
> no
> >>> matter how many packets you fed to the encoder, which meant it
> was
> >> stuck
> >>> in an unrecoverable state. See
> >>> http://ffmpeg.org/pipermail/ffmpeg-devel/2021-October/287153.html
> >>>
> >>> If you know of cases where this error could be returned in valid
> >>> recoverable scenarios that are not already handled in some other
> >> way,
> >>> what do you suggest could be done?
> >>
> >> It's more that that very much depends on the context the function
> is
> >> used in.
> >> In the context of an active de/encoding loop, every non-success
> >> return
> >> would be unrecoverable. Or even during init.
> >>
> >> While in other places a failure just breaks a single frame and if
> it
> >> somehow recovery, it can continue on. Though that's usually the
> >> exception.
> >
> > Can you give an example for this in ffmpeg where CUDA returns an
> > error and ffmpeg continues?
> 
> Almost all the filters will continue when one filtering step fails.

When looking at vf_scale_cuda, all calls are checked and return
AVERROR_EXTERNAL back in case of error. Where is the "one error
is forgiven" handling?


> > At the time when I had submitted the patch, I had replied this to
> James
> > after he had argued that the behavior would be correct as is and
> > a user who would want ffmpeg to exit in such case, must specify
> > the -xerror parameter.
> >
> >
> > softworkz:
> >> When there's an error in a filter => ffmpeg exits
> >> When there's an error in an encoder => ffmpeg exits
> >> When there's an error initializing a decoder or an encoder
> >> => ffmpeg exits
> >>
> >> So why shouldn't ffmpeg exit when there's an unrecoverable error
> >> in a decoder?
> >
> >
> > AFAIR, in case of filtering and encoding, any "normal" CUDA
> > error is already sufficient to cause ffmpeg to exit.
> >
> >
> >> In any case, this function does not seem the right place to map an
> >> arbitrary return code to such a result.
> >
> > That depends on whether you consider CUDA_ERROR_UNKNOWN as
> > generally unrecoverable. When you do - like I did - then
> > it's the right place IMO.
> >
> > If you doubt it, then yes - it wouldn't be the right place.
> 
> Why is the function not unconditionally returning an unrecoverable
> error
> then?
> Like, 90% of current error returns, independent of CUDA, in
> filters/codecs can be considered UNRECOVERABLE then. 

Why "then"? I'm only considering CUDA_ERROR_UNKNOWN as unrecoverable
I have no knowledge about the other errors. That's why I 
would make the change only affect CUDA_ERROR_UNKNOWN.


> Which does make
> sense that you want them to about.
> But you in turn give up the informational character of error codes.

That's why I wouldn't to that.

> Makes me think this should be some kind of additional flag instead.

And update all the hundreds of usages?

I have no bias in this regard (I can easily say that because James
has submitted :-)

Best wishes,
softworkz