[FFmpeg-devel] [PATCH] fix shifted picture in AMV video decoder when height is not dividable by 16

[Vladimir Voroshilov]

2007/10/12, Michael Niedermayer <michaelni at gmx.at>:
> On Wed, Oct 10, 2007 at 09:34:07PM +0700, Vladimir Voroshilov wrote:
> > Hi, All
> >
> > If picture height is not dividable by 16, AMV related flipping code in
> > mjpeg decoder does not work correctly.
> > You can easily see height%16 lines of garbage at the top of picture
> > while the same number of bottom lines are not visible.
> >
> > Attached patch adds code which moves three YUV planes in buffer up
> > after decoding picture.
>
> rejected
>
>
> >
> > Comments and (especialy) optimizations are welcome.
>
> a fix to the problem is welcome a memmove workaround is not

I'm afraid i'll never get solution :|

Well. I'll try to explain where the problem is.

0. Imagine you have picture with height 24

1. av_get_buffer will expand buffer to be enough large for picture
height 32 (16-lines alignment)

2a. Non-flipped picture will be decoded as: 1st row of macroblocks
will fill lines 1-16, 2nd (not complete) - lines 17-32 (where lines
25-32 will contain garbage, but since they are expanded and thus not
visible, picture is ok)

2b. Flipped picture will be decoded as: 1st row of macroblocks will
fill lines 32-17 (while should fill 24-9), and 2nd (not complete) will
fill lines 16-1 (where 8-1 will contain garbage and thus picture will
be wrong, i.e. shifted by 8 lines down)

I can see only two (non memmove) fixes:

1. shift data[] pointer up additionally to desired value
(corresponding to a number of non-aligned lines, 8 in above case) and
check ptr value on each for iteration to avoid pointing outside data[]
buffer.
Will work only if height dividable by 8.

2. create picture buffer inside sp5xdec.c height%16 lines larger then
required before calling to ff_mjpeg_decode_picture.
Shift data[] down to create space at beginning  of the buffer. shift
data[] up inside mjpeg_decode_scan as in previous case, but without
checking ptr (space before data[] already allocated).
Will work with any height values.

Both solutions looks even more hackish than memmove.

-- 
Regards,
Vladimir Voroshilov     mailto:voroshil at gmail.com
JID: voroshil at gmail.com, voroshil at jabber.ru
ICQ: 95587719