[FFmpeg-devel] [PATCH] lavf/async: Fix wrapped_url_read return value

[FFmpeg-devel] [PATCH] lavf/async: Fix wrapped_url_read return value

[Guangyu Sun]

This fixes a regression from commit 36117968ad.

wrapped_url_read() used to be able to return positive number from
ffurl_read(). It relies on the result to check if EOF is reached in
async_buffer_task().

Test case:
  ffmpeg -f lavfi -i testsrc -t 1 test.mp4
  ffmpeg -i async:test.mp4

Signed-off-by: Guangyu Sun <gsun@roblox.com>
---
 libavformat/async.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/async.c b/libavformat/async.c
index 547417aa1e..a929f4d16e 100644
--- a/libavformat/async.c
+++ b/libavformat/async.c
@@ -139,7 +139,7 @@ static int wrapped_url_read(void *src, void *dst, size_t *size)
     *size             = ret > 0 ? ret : 0;
     c->inner_io_error = ret < 0 ? ret : 0;
 
-    return c->inner_io_error;
+    return ret;
 }
 
 static int ring_write(RingBuffer *ring, URLContext *h, size_t size)
-- 
2.37.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] lavf/async: Fix wrapped_url_read return value

[Anton Khirnov]

Quoting Guangyu Sun (2022-09-19 07:22:39)
> This fixes a regression from commit 36117968ad.
> 
> wrapped_url_read() used to be able to return positive number from
> ffurl_read(). It relies on the result to check if EOF is reached in
> async_buffer_task().
> 
> Test case:
>   ffmpeg -f lavfi -i testsrc -t 1 test.mp4
>   ffmpeg -i async:test.mp4
> 
> Signed-off-by: Guangyu Sun <gsun@roblox.com>
> ---
>  libavformat/async.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/async.c b/libavformat/async.c
> index 547417aa1e..a929f4d16e 100644
> --- a/libavformat/async.c
> +++ b/libavformat/async.c
> @@ -139,7 +139,7 @@ static int wrapped_url_read(void *src, void *dst, size_t *size)
>      *size             = ret > 0 ? ret : 0;
>      c->inner_io_error = ret < 0 ? ret : 0;
>  
> -    return c->inner_io_error;
> +    return ret;

This is wrong, FIFO callbacks must return 0 on success. This should be
handled in ring_write() instead.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2] lavf/async: Fix ring_write return value

[Guangyu Sun]

his fixes a regression from commit 36117968ad.

wrapped_url_read() used to be able to return positive number from
ffurl_read(). It relies on the result to check if EOF is reached in
async_buffer_task().

But FIFO callbacks must return 0 on success. This should be handled
in ring_write() instead.

Test case:
  ffmpeg -f lavfi -i testsrc -t 1 test.mp4
  ffmpeg -i async:test.mp4

Signed-off-by: Guangyu Sun <gsun@roblox.com>
---
 libavformat/async.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/libavformat/async.c b/libavformat/async.c
index 547417aa1e..3c6f89cab9 100644
--- a/libavformat/async.c
+++ b/libavformat/async.c
@@ -144,8 +144,14 @@ static int wrapped_url_read(void *src, void *dst, size_t *size)
 
 static int ring_write(RingBuffer *ring, URLContext *h, size_t size)
 {
+    int ret;
+
     av_assert2(size <= ring_space(ring));
-    return av_fifo_write_from_cb(ring->fifo, wrapped_url_read, h, &size);
+    ret = av_fifo_write_from_cb(ring->fifo, wrapped_url_read, h, &size);
+    if (ret < 0)
+        return ret;
+
+    return size;
 }
 
 static int ring_size_of_read_back(RingBuffer *ring)
-- 
2.37.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2] lavf/async: Fix ring_write return value

[Anton Khirnov]

Quoting Guangyu Sun (2022-09-29 20:11:34)
> his fixes a regression from commit 36117968ad.
> 
> wrapped_url_read() used to be able to return positive number from
> ffurl_read(). It relies on the result to check if EOF is reached in
> async_buffer_task().
> 
> But FIFO callbacks must return 0 on success. This should be handled
> in ring_write() instead.
> 
> Test case:
>   ffmpeg -f lavfi -i testsrc -t 1 test.mp4
>   ffmpeg -i async:test.mp4
> 
> Signed-off-by: Guangyu Sun <gsun@roblox.com>
> ---
>  libavformat/async.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)

Thank you, will push.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] lavf/async: Fix wrapped_url_read return value

[Guangyu Sun]

On Wed, Sep 28, 2022 at 7:08 AM Anton Khirnov <anton@khirnov.net> wrote:
>
> Quoting Guangyu Sun (2022-09-19 07:22:39)
> > This fixes a regression from commit 36117968ad.
> >
> > wrapped_url_read() used to be able to return positive number from
> > ffurl_read(). It relies on the result to check if EOF is reached in
> > async_buffer_task().
> >
> > Test case:
> >   ffmpeg -f lavfi -i testsrc -t 1 test.mp4
> >   ffmpeg -i async:test.mp4
> >
> > Signed-off-by: Guangyu Sun <gsun@roblox.com>
> > ---
> >  libavformat/async.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavformat/async.c b/libavformat/async.c
> > index 547417aa1e..a929f4d16e 100644
> > --- a/libavformat/async.c
> > +++ b/libavformat/async.c
> > @@ -139,7 +139,7 @@ static int wrapped_url_read(void *src, void *dst, size_t *size)
> >      *size             = ret > 0 ? ret : 0;
> >      c->inner_io_error = ret < 0 ? ret : 0;
> >
> > -    return c->inner_io_error;
> > +    return ret;
>
> This is wrong, FIFO callbacks must return 0 on success. This should be
> handled in ring_write() instead.

Thanks. I just submitted a new version.

Guangyu

>
> --
> Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".