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

Mohamed Naufal naufal11
Sat Mar 27 18:27:30 CET 2010


On 27 March 2010 22:37, Reimar D?ffinger <Reimar.Doeffinger at gmx.de> wrote:
> 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.


But wouldn't that entail further checks? Please elaborate.

Thanks,
Naufal



More information about the ffmpeg-devel mailing list