[FFmpeg-devel] Fwd: [PATCH] Psygnosis YOP demuxer

[Reimar Döffinger]

On Sat, Mar 27, 2010 at 09:52:48PM +0530, Mohamed Naufal wrote:
> On 27 March 2010 19:03, Reimar D?ffinger <Reimar.Doeffinger at gmx.de> wrote:
> > On Sat, Mar 27, 2010 at 01:15:46PM +0100, Michael Niedermayer wrote:
> >> anyway it would be great if we could avoid the linesize overriding but i
> >> dont think this is possible with this codec, i think it depends on having
> >> line stored with linesize==width (or it would need some messy special case
> >> handling close to the image borders, which does not seem like a good
> >> idea)
> >
> > Btw. I suspect it is only significantly more messy than the current
> > solution because the current patch cheats and lacks necessary checks,
> > happily crashing with invalid motion vectors near the start and end
> > of frame.
> 
> [...]
> But the motion vector doesn't cause the crossing of the upper frame boundary.

I don't quite understand that part, but I think I now see the issue, I guess
it could be that bufptr[0] is at the end of one and bufptr[1] and the start of
the next row.
Then I agree it's probably better to leave the linesize hack in.

> +    // Calculate position for the copy source
> +    bufptr = s->dstptr + motion_vector[copy_tag][0] +
> +             s->frame.linesize[0] * motion_vector[copy_tag][1];
> +    if (bufptr < s->dstbuf) {
> +        av_log(s->avctx, AV_LOG_ERROR,
> +               "YOP: cannot decode, file probably corrupt");
> +        return AVERROR_INVALIDDATA;
> +    }

I guess this is mostly ok here, but you should be aware that this is not
really valid C, and in the real world can have issues with overflows (in
particular it assumes there is a NULL-page larger than 4*linesize[0]).
Btw. (I'm not asking you to implement it) you could probably gain quite
some speed by using a "tighter" loop for the middle part of the frame that
does not do this check and the dstptr - dstbuf check, and thanks to padding
the srcptr check could be done much more rarely there as well.