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

[D Richard Felker III]

On Tue, Nov 23, 2004 at 08:47:14AM +0100, Jindrich Makovicka wrote:
> 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.

perhaps you should read the libavcodec code in mplayer and see why it
doesn't need these hacks...? or, if it is using nasty hacks, how they
work..

rich