diff mbox

[FFmpeg-devel,v2,1/2] lavc: consider an error during decoder draining as EOF

Message ID 20170302094524.10628-1-nfxjfg@googlemail.com
State Accepted
Commit a755b725ec1d657609c8bd726ce37e7cf193d03f
Headers show

Commit Message

wm4 March 2, 2017, 9:45 a.m. UTC 
There is no reason that draining couldn't return an error or two. But
some decoders don't handle this very well, and might always return an
error. This can lead to API users getting into an infinite loop and
burning CPU, because no progress is made and EOF is never returned.

In fact, ffmpeg.c contains a hack against such a case. It is removed
with this patch. This particular error case seems to have been fixed
since the hack was added, though.

This might lose frames if decoding returns errors during draining.
---
Minor lavc bump missing.
---
 libavcodec/utils.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

wm4 March 6, 2017, 10:09 a.m. UTC | #1 
On Thu,  2 Mar 2017 10:45:23 +0100
wm4 <nfxjfg@googlemail.com> wrote:

> There is no reason that draining couldn't return an error or two. But
> some decoders don't handle this very well, and might always return an
> error. This can lead to API users getting into an infinite loop and
> burning CPU, because no progress is made and EOF is never returned.
> 
> In fact, ffmpeg.c contains a hack against such a case. It is removed
> with this patch. This particular error case seems to have been fixed
> since the hack was added, though.
> 
> This might lose frames if decoding returns errors during draining.
> ---
> Minor lavc bump missing.
> ---
>  libavcodec/utils.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index 1156e43079..db3adb18d4 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -2807,12 +2807,12 @@ static int do_decode(AVCodecContext *avctx, AVPacket *pkt)
>      if (ret == AVERROR(EAGAIN))
>          ret = pkt->size;
>  
> -    if (ret < 0)
> -        return ret;
> -
>      if (avctx->internal->draining && !got_frame)
>          avctx->internal->draining_done = 1;
>  
> +    if (ret < 0)
> +        return ret;
> +
>      if (ret >= pkt->size) {
>          av_packet_unref(avctx->internal->buffer_pkt);
>      } else {

Both patches pushed, with some changes to the commit message of the
first one. Also I just noticed that I forgot the minor lavc bump, but I
guess it's not too important anyway.
diff mbox

Patch

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 1156e43079..db3adb18d4 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -2807,12 +2807,12 @@  static int do_decode(AVCodecContext *avctx, AVPacket *pkt)
     if (ret == AVERROR(EAGAIN))
         ret = pkt->size;
 
-    if (ret < 0)
-        return ret;
-
     if (avctx->internal->draining && !got_frame)
         avctx->internal->draining_done = 1;
 
+    if (ret < 0)
+        return ret;
+
     if (ret >= pkt->size) {
         av_packet_unref(avctx->internal->buffer_pkt);
     } else {