[MPlayer-cvslog] CVS: main/libmpcodecs vd_libmpeg2.c,1.37,1.38

[Jindrich Makovicka]

Jindrich Makovicka wrote:
>> The above change does not make sense for me. Are you aware that the 
>> video system may actually
>> return buffer that is not aligned to 16? This will segfault, as mpeg 
>> always write full macro blocks.
>> I see that you are aware that sequence->height is already aligned to 
>> 16. While picture_height is
>> raw mpeg stream value (visible dimension)
> 
> 
> I must admit I am confused here, because the original with aligned 
> dimensions crashes exactly in the case when the dimensions are not 
> multiples of 16. For example the following 1920x1080 video playes using 
> -vo x11 crashes (xv doesn't work for me due to the video size).
> 
> http://mplayerhq.hu/~diego/libmpeg2bug.ts
> 
> The crash occurs at the end of playback, byt valgrind reports write 
> access error the YUV->RGB conversion right after line 1080. This occurs 
> with the original code with dims aligned to 16. The current code works 
> flawlessly.

well. i spent a beautiful evening looking at the vf & vo code and i 
found that actually both ways of calling get_image, either with aligned 
or unaligned dimensions, are wrong. they just move the point of 
potential crash from one place to another. the unaligned version can 
crash in the decoder writing the edge macroblock, while the aligned will 
crash in the VO trying to stuff the larger image into the output area. 
the correct fix is (imho) changing vf_get_image semantics to support the 
explicit specification of the horizontal vertical alignment, with 
original vf_get_image being a wrapper around it. this will ensure that 
enough space is allocated, while the correct image dimensions are 
propagated through VFs. the same problems are with -vf pp and other 
stuff and were reported in

http://mplayerhq.hu/pipermail/mplayer-dev-eng/2004-October/030502.html

i am doing to send a proposed patch to mplayer-dev.

so in fact, much like all the previous libmpeg2 patches in the past few 
days, even this one does not actually break things :) you can call it 
stupid or clumsy, but please shut up about breaking things as this is 
simply not the case.

-- 
Jindrich Makovicka