[FFmpeg-cvslog] r20552 - trunk/libavcodec/mjpegdec.c

Reimar Döffinger Reimar.Doeffinger
Sun Nov 22 13:10:36 CET 2009


On Sun, Nov 22, 2009 at 12:26:26PM +0100, Michael Niedermayer wrote:
> On Fri, Nov 20, 2009 at 03:57:59PM +0100, Reimar D?ffinger wrote:
> > On Fri, Nov 20, 2009 at 02:21:53PM +0100, Michael Niedermayer wrote:
> > > I dont think this is correct
> > > The test should be s->height % (s->v_max*8)
> > > 
> > > besides this the fliped code also looks suspect
> > 
> > Hmm. Does anyone actually have or can create a sample that should be flipped
> > and where the height is not divisible by 8 or so?
> 
> you can just set flipped = 1 with a normal jpeg to test it i guess 

I'm not sure about that, at least I do not know whether the padding is
on top or on bottom for the flipped image, and using a normal jpeg can
only tell me how that is for normal JPEGs, but not those created as
flipped by default I think?

> > Either way, I am quite confident that the check here should be exactly
> > for the same thing as whatever is used for the flipping code.
> 
> i doubt (height / 3) % 8 is correct
> example: height=25
> height/3 == 8
> actually written height= 48

And data[c] is incremented by
s->v_scount[i] * (8 * s->mb_height - 1)
lines, so all those 48 lines still fit in it even with EMU_EDGE.

> another example: (height /2) %8
> height=1 ...

Still no out-of-bounds write with EMU_EDGE I can see.
Now if this formula:
data[c] += (linesize[c] * (s->v_scount[i] * (8 * s->mb_height -((s->height/s->v_max)&7)) - 1 ));
ends up writing at the right place I don't know, however I still think
that my check ensures that no out-of-bounds writes happen even with
EMU_EDGE.

> > I suspect that your code is more correct, however I also think that
> > the 8*mb_height is completely bogus and should have particularly funny
> > effects if someone created a lossless JPEG (where mb_width is never
> > initialized) that somehow triggers flipped.
> 
> mjpeg_decode_scan() is under if(!lossless)

Ok, right, I didn't notice that. However how mb_height is calculated
from height depends on nb_components, too, so I wonder a bit why the
data[c] formula for flipping doesn't have to, too.



More information about the ffmpeg-cvslog mailing list