diff mbox series

[FFmpeg-devel] avcodec/libdav1d: only return EAGAIN when there are no buffered packets

Message ID 20230520132113.1855-1-jamrial@gmail.com
State New
Headers show
Series [FFmpeg-devel] avcodec/libdav1d: only return EAGAIN when there are no buffered packets | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

James Almer May 20, 2023, 1:21 p.m. UTC
Fixes decoding packets containing split temporal units, as generated for example
by the av1_frame_split bsf.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/libdav1d.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Anton Khirnov May 20, 2023, 4:14 p.m. UTC | #1
Quoting James Almer (2023-05-20 15:21:13)
> Fixes decoding packets containing split temporal units, as generated for example
> by the av1_frame_split bsf.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/libdav1d.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
> index c15e98cbd1..55ea9166b6 100644
> --- a/libavcodec/libdav1d.c
> +++ b/libavcodec/libdav1d.c
> @@ -328,6 +328,7 @@ static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame)
>  #endif
>      int res;
>  
> +again:

yuck
James Almer May 20, 2023, 4:42 p.m. UTC | #2
On 5/20/2023 1:14 PM, Anton Khirnov wrote:
> Quoting James Almer (2023-05-20 15:21:13)
>> Fixes decoding packets containing split temporal units, as generated for example
>> by the av1_frame_split bsf.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavcodec/libdav1d.c | 9 ++++++---
>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
>> index c15e98cbd1..55ea9166b6 100644
>> --- a/libavcodec/libdav1d.c
>> +++ b/libavcodec/libdav1d.c
>> @@ -328,6 +328,7 @@ static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame)
>>   #endif
>>       int res;
>>   
>> +again:
> 
> yuck

I could make it a while loop, but then i need to reindent a lot of 
lines. Is a goto that bad? We use that everywhere...
Anton Khirnov May 20, 2023, 4:45 p.m. UTC | #3
Quoting James Almer (2023-05-20 18:42:11)
> On 5/20/2023 1:14 PM, Anton Khirnov wrote:
> > Quoting James Almer (2023-05-20 15:21:13)
> >> Fixes decoding packets containing split temporal units, as generated for example
> >> by the av1_frame_split bsf.
> >>
> >> Signed-off-by: James Almer <jamrial@gmail.com>
> >> ---
> >>   libavcodec/libdav1d.c | 9 ++++++---
> >>   1 file changed, 6 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
> >> index c15e98cbd1..55ea9166b6 100644
> >> --- a/libavcodec/libdav1d.c
> >> +++ b/libavcodec/libdav1d.c
> >> @@ -328,6 +328,7 @@ static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame)
> >>   #endif
> >>       int res;
> >>   
> >> +again:
> > 
> > yuck
> 
> I could make it a while loop, but then i need to reindent a lot of 
> lines. Is a goto that bad? We use that everywhere...

I know people use it, but I personally find it very evil. It's making
the code significantly less readable (especially when it's further
modified in the future) just to avoid restructuring it properly.
Future readers will thank you for restructuring it now.
diff mbox series

Patch

diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index c15e98cbd1..55ea9166b6 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -328,6 +328,7 @@  static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame)
 #endif
     int res;
 
+again:
     if (!data->sz) {
         pkt = av_packet_alloc();
 
@@ -398,10 +399,12 @@  FF_ENABLE_DEPRECATION_WARNINGS
 
     res = dav1d_get_picture(dav1d->c, p);
     if (res < 0) {
-        if (res == AVERROR(EINVAL))
-            res = AVERROR_INVALIDDATA;
-        else if (res == AVERROR(EAGAIN) && c->internal->draining)
+        if (res == AVERROR(EAGAIN)) {
+            if (!c->internal->draining)
+                goto again;
             res = AVERROR_EOF;
+        } else if (res == AVERROR(EINVAL))
+            res = AVERROR_INVALIDDATA;
 
         return res;
     }