[Ffmpeg-devel] [PATCH] - Error handling in MSRLE

[Michael Niedermayer]

Hi

On Wed, Feb 21, 2007 at 05:27:08AM +0200, emild at cs.technion.ac.il wrote:
> Hello,
> 
> ffmpeg complains when reading a legal .avi file encoded with MSRLE (such as
> "clock.avi" found in the Windows directory of every installation of the Windows
> OS), saying essentially that the frame is 2 bytes longer than how much it should
> be. The reason why this happens  is that the decoder does not read the "end of
> bitmap" marker 0x00, 0x01 because the decoding loop stops when all the lines
> have been decoded.
> 
> The attached patch fixes the bug. The error message will be displayed only if
> the last bytes are not a valid end-of bitmap marker. After applying this patch,
> ffmpeg should no longer complain as described below.
> 
>                                                          Regards,
>                                                                  Emil
> 
> 
> 
> 
> 
[...]
> diff -uwr ffmpeg-export-2007-02-20.orig/libavcodec/msrle.c ffmpeg-export-2007-02-20/libavcodec/msrle.c
> --- ffmpeg-export-2007-02-20.orig/libavcodec/msrle.c	2006-10-07 17:30:46.000000000 +0200
> +++ ffmpeg-export-2007-02-20/libavcodec/msrle.c	2007-02-21 02:17:37.000000000 +0200
> @@ -145,10 +145,13 @@
>      }
>  
>      /* one last sanity check on the way out */
> -    if (stream_ptr < s->size)
> +    if (stream_ptr < s->size &&
> +       !(s->size - stream_ptr == 2 && 
> +        s->buf[stream_ptr] == 0x00 && s->buf[stream_ptr + 1] == 0x01)) {
>          av_log(s->avctx, AV_LOG_ERROR, " MS RLE: ended frame decode with bytes left over (%d < %d)\n",
>              stream_ptr, s->size);
>  }
> +}

wrong indention

furthermore the code is a mess, is the end marker optional? if not the
following is correcter

if(stream_ptr+2 != s->size || AV_RB16(s->buf+stream_ptr) != 0x01)


>  
>  
>  
> @@ -232,7 +235,9 @@
>      }
>  
>      /* one last sanity check on the way out */
> -    if (stream_ptr < s->size)
> +    if (stream_ptr < s->size &&
> +       !(s->size - stream_ptr == 2 &&
> +        s->buf[stream_ptr] == 0x00 && s->buf[stream_ptr + 1] == 0x01))
>          av_log(s->avctx, AV_LOG_ERROR, " MS RLE: ended frame decode with bytes left over (%d < %d)\n",
>              stream_ptr, s->size);
>  }

code duplication
yes its already duplicated in svn but i wont accept any patch which
increases the amount of duplicated code, you will have to factor the common
code out of these 2 functions if you want the patch accepted

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070221/d7efd5c1/attachment.pgp>