[Ffmpeg-devel] Maybe (?), robustness patch

[Michael Niedermayer]

Hi

On Mon, Jan 23, 2006 at 08:05:47AM +0000, Mean wrote:
> Michael Niedermayer wrote:
> >Hi
> >
> >On Sun, Jan 22, 2006 at 05:31:16PM +0000, Mean wrote:
> >  
> >>Hi,
> >>   While trying to create a correct avcC atom with lavformat/x264, i've 
> >>made some (interestingly ?) broken mp4 files.
> >>   One of them makes lavcodec crash hard, which is bad and uncommon.
> >>
> >>   The problem is that in that case bit_length ends up being equal to 
> >>-1, hence the crash a bit later
> >>   I'm not sure it is a fix of great interest since such broken files 
> >>should not exist at all, but anyway.
> >>
> >>   The trivial check that avoids the hard crash is below
> >>
> >>   One such funny file is available here
> >>          
> >>http://fixounet.free.fr/avidemux/win32/broken_h264_makes_lavcodec_crash.mp4
> >>   No image, but no crash either.
> >>
> >>Thanks.
> >>
> >>
> >>
> >>   Index: h264.c
> >>===================================================================
> >>--- h264.c      (revision 1724)
> >>+++ h264.c      (working copy)
> >>@@ -7458,7 +7458,13 @@
> >>        ptr= decode_nal(h, buf + buf_index, &dst_length, &consumed, 
> >>h->is_avc ? nalsize : buf_size - buf_index);
> >>        if(ptr[dst_length - 1] == 0) dst_length--;
> >>        bit_length= 8*dst_length - decode_rbsp_trailing(ptr + 
> >>dst_length - 1);
> >>-
> >>+       /* MEANX */
> >>+       if(bit_length<=0)
> >>+       {
> >>+                av_log(h->s.avctx, AV_LOG_ERROR, "h264: bit length 
> >>computing error, broken stream ? \n");
> >>+               return -1;
> >>+       }
> >>+       /* /MEANX */
> >>    
> >
> >can you submit patches without addng your name to every chunk? just imagine
> >how ffmpegs source would look like if everyone did this ...
> >
> >bit_length<0 also needs dst_length=0 and that already seems wrong ...
> >
> >[...]
> >  
> Hello,
>       Sorry to be thick, but does that mean that a check for dst_length 
> is also need or does that mean
>    that it is of no interest ?

IMHO a check for dst_length would be ideal yes

[...]

-- 
Michael