[FFmpeg-devel] [PATCH] fix for rev11962 that broke yuv422 mpeg encoding
Michael Niedermayer
michaelni
Fri Feb 22 19:18:51 CET 2008
On Fri, Feb 22, 2008 at 06:57:09PM +0100, Baptiste Coudurier wrote:
> 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.
The last thing the author said was something like "iam leaving town in a minute
and wont have internet access for the next week" or so.
Also you have my approval to revert whatever is needed.
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080222/a596ab63/attachment.pgp>
More information about the ffmpeg-devel
mailing list