[Ffmpeg-cvslog] r7889 - trunk/libavcodec/mpeg12.c

[Ivan Kalvachev]

2007/2/11, Michael Niedermayer <michaelni at gmx.at>:
> Hi
>
> On Sun, Feb 11, 2007 at 01:21:11AM +0200, Ivan Kalvachev wrote:
> > 2007/2/9, michael <subversion at mplayerhq.hu>:
> > >Author: michael
> > >Date: Fri Feb  9 03:21:34 2007
> > >New Revision: 7889
> > >
> > >Modified:
> > >   trunk/libavcodec/mpeg12.c
> > >
> > >Log:
> > >fix segfault with lol-ffplay2.mpg (dunno if this is exploitable, probably
> > >not easily)
> > >
> > >
> > >Modified: trunk/libavcodec/mpeg12.c
> > >==============================================================================
> > >--- trunk/libavcodec/mpeg12.c   (original)
> > >+++ trunk/libavcodec/mpeg12.c   Fri Feb  9 03:21:34 2007
> > >@@ -2593,6 +2593,10 @@
> > >             break;
> > >         }
> > >     }
> > >+    if(s->mb_x >= (unsigned)s->mb_width){
> > >+        av_log(s->avctx, AV_LOG_ERROR, "initial skip overflow\n");
> > >+        return -1;
> > >+    }
> > >
> > >     s->resync_mb_x= s->mb_x;
> > >     s->resync_mb_y= s->mb_y= mb_y;
> >
> > Are you sure this is error condition?
>
> yes
>
>
> >
> > I don't see anything in mpeg standards that indicates so. even contrary.
>
> ---
> slice_vertical_position --  This is given by the last eight bits of
> the slice_start_code. It is an unsigned integer giving the vertical
> position in macroblock units of the first macroblock in the slice.
> The slice_vertical_position of the first row of macroblocks is one.
>  Some slices may have the same slice_vertical_position, since
> slices may start and finish anywhere.  Note that the
> slice_vertical_position is constrained by Clause 2.4.1 to define
> non-overlapping slices with no gaps between them. The maximum value
> of slice_vertical_position is 175.

I got it, but, you skipped 2 steps in the explanation:
1.  "The first and last macroblock of a slice shall not be skipped."
2.  macroblock_address is always reset, so slice starts at the first
MB of the row. This means first  macroblock_address_increment in the
slice should point after the last MB of previous slices.

Obviously if slice goes over the end of the row, next one will start
on the next row.

Sorry for the fuss.