[FFmpeg-devel] [PATCH] fix for rev11962 that broke yuv422 mpeg encoding

[Baptiste Coudurier]

Hi

Michael Niedermayer wrote:
> On Wed, Feb 20, 2008 at 12:57:13AM +0100, christophelorenz wrote:
>> The following command
>> ffmpeg -i source -pix_fmt yuv422p -vcodec mpeg2video -intra -qscale 1 
>> output422.m2v
>> outputs corrupted chroma since rev 11962.
>> (source is 720x576, -intra and -qscale make the chroma alignment problem 
>> easier to see)
>>
>> Rev 11962 has changed the linesize of the yuv planes using ALIGN macro to 
>> align Y on U dimension.
>> However macro is only designed to align to pow2 values and not various 
>> image width.
>> Also after alignment to picture.linesize[1] there's no garantee that the 
>> linesize[0] is still aligned to a STRIDE_ALIGN multiple anymore.
>>
>> Refs in libavcodec/utils.c:
>> L162:
>> #define ALIGN(x, a) (((x)+(a)-1)&~((a)-1))
>> L298:
>> picture.linesize[0] = ALIGN(picture.linesize[0], picture.linesize[1]);
>>
>> I changed the code so that the linesize[1,2,3] are adjusted to upper value 
>> that is a round fraction of linesize[0],
>> while preserving alignment to STRIDE_ALIGN for all planes.
>>
>> Chris.
>>
> 
>> Index: libavcodec/utils.c
>> ===================================================================
>> --- libavcodec/utils.c	(revision 12154)
>> +++ libavcodec/utils.c	(working copy)
>> @@ -293,10 +293,11 @@
>>  
>>          /* next ensures that linesize= 2^x uvlinesize, that is needed because
>>           * some MC code assumes it */
>> +        picture.linesize[0] = ALIGN(picture.linesize[0], STRIDE_ALIGN);
>>          if (picture.linesize[1])
>> -            picture.linesize[0] = ALIGN(picture.linesize[0], picture.linesize[1]);
>> -        else
>> -            picture.linesize[0] = ALIGN(picture.linesize[0], STRIDE_ALIGN);
>> +            picture.linesize[1] = picture.linesize[0]/(picture.linesize[0]/picture.linesize[1]);
>> +        if (picture.linesize[2])
>> +            picture.linesize[2] = picture.linesize[0]/(picture.linesize[0]/picture.linesize[2]);
>> +        if (picture.linesize[3])
>> +            picture.linesize[3] = picture.linesize[0]/(picture.linesize[0]/picture.linesize[3]);
> 
> This is completely wrong
> Some MC code assumes 2*linesize[1] == linesize[0] for YV12 or it did assume,
> i dont remember, but if it still exists and assumes that, it wil definitly not
> work with any other factor than 2.
> So first find out if that code still exists and then either remove the align
> stuff or fix it properly.
> Also what you wrote above is code duplication due to:
> for (i=1; i<4; i++)
>     picture.linesize[i] = ALIGN(picture.linesize[i], STRIDE_ALIGN);
> 
> being straight before your code for picture.linesize[0]
> 

Shit, this bug is critical.

Can someone (preferably the author of the concerned commit) fix this
issue asap ? If this issue stays too long (it's been one week already),
I'd like to have the commit reverted, and properly implemented.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
SMARTJOG S.A.                                    http://www.smartjog.com
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
Phone: +33 1 49966312