[MPlayer-dev-eng] [PATCH] yadif green line

[Ivan Kalvachev]

On 2/8/12, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Sun, Feb 05, 2012 at 08:50:27PM +0200, Ivan Kalvachev wrote:
>> A user provided me with a sample file that generated a random garbage
>> at the bottom line.
>>
>> It turned out that the secret for repeating this bug is in the parity.
>> It is always reproducible if the function filter(...)  is rigged so
>> the parity check calls filter_line() for the last image line.
>> In the filter_line_c() function the line "int e=cur[+ref];"
>> unconditionally reads the next line of the image. If we are at the
>> last line, it would read the line past the last one. ( mode<2 reads
>> and +2 lines)
>>
>> It seems that this quirk of the yadif have been known, so the filter
>> copies the image into buffer that have 3 lines pad at top and bottom.
>>
>> Unfortunately the store_ref() function that does that, doesn't pad the
>> additional lines, so they are left uninitialized. The solid green line
>> is obtained if malloced buffers are zeroed.
>>
>>
>> The patch that I attach pads 2 additional lines top and bottom.
>> It also fixed the allocation so the height is not aligned to 32 lines
>> (useless) but instead there are always 6 pad lines (3+3), even for
>> chroma planes.
>
> This issue has been fixed in ffmpegs yadif a year ago (see bad82d3d)
> the ffmpeg fix also doesnt need larger frames which makes integration
> with direct rendering easier.

I see. The filter functions uses separate 2 strides, one for the
previous and one for the next line so you can make them point to the
same line at edge cases.

I guess the change of mode in that case is done for a better
prediction not to avoid +-2 line access. Don't you think it should be
done for the first 2 lines, not just for the second one?

The height in ffmpeg is however still +2 lines and on top of that
aligned to 32 lines. You may want to remove the h+2 and align to 2
lines instead.

Well, I don't feel like porting big complicated changes, so I'll
commit the workaround for now.

Patch committed.