[FFmpeg-devel] [PATCH] avcodec/libdav1d: clear the buffered Dav1dData on decoding failure

[James Almer]

On 12/29/2024 11:11 AM, Ronald S. Bultje wrote:
> Hi,
> 
> On Fri, Dec 27, 2024 at 5:12 PM James Almer <jamrial at gmail.com> wrote:
> 
>> Should ensure avcodec_send_packet() doesn't return EAGAIN in scenarios
>> where it's not
>> meant to (e.g., ffmpeg_dec.c where avcodec_receive_frame() is called in a
>> loop to drain
>> all produced frames before trying to submit more packets).
>>
>> Fixes ticket #11377.
>>
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>>   libavcodec/libdav1d.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
>> index 546b42e9c6..ff67f44bd3 100644
>> --- a/libavcodec/libdav1d.c
>> +++ b/libavcodec/libdav1d.c
>> @@ -372,9 +372,10 @@ static int
>> libdav1d_receive_frame_internal(AVCodecContext *c, Dav1dPicture *p)
>>
>>       res = dav1d_get_picture(dav1d->c, p);
>>       if (res < 0) {
>> -        if (res == AVERROR(EINVAL))
>> +        if (res == AVERROR(EINVAL)) {
>> +            dav1d_data_unref(data);
>>               res = AVERROR_INVALIDDATA;
>> -        else if (res == AVERROR(EAGAIN))
>> +        } else if (res == AVERROR(EAGAIN))
>>               res = c->internal->draining ? AVERROR_EOF : 1;
>>       }
>>
>> --
>> 2.47.1
>>
> 
> This looks OK.
> 
> Do we need to refine the example code in the doxy of dav1d? I feel the
> error handling (including the special handling/meaning of some error codes)
> is not very explicitly documented there. See also
> https://code.videolan.org/videolan/dav1d/-/issues/436

The problem here was only on multithreaded scenarios, where libdav1d 
caches and returns errors that may not belong to the latest packet fed 
to it, which then was not properly handled in lavc with its own 
decoupled input/output API.

Not sure if the cached error stuff is something that should be documented.

> 
> And should we be using AVERROR() here? Isn't DAV1D_ERR() more appropriate?

Probably, but AVERROR and DAV1D_ERR are equal. Both negate the code if 
needed.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 495 bytes
Desc: OpenPGP digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20241229/1baeec39/attachment.sig>