[FFmpeg-devel] [PATCH] Use tkhd matrix for proper display in mov
Michael Niedermayer
michaelni
Thu May 29 00:58:36 CEST 2008
On Wed, May 28, 2008 at 05:02:12PM -0400, John Schmiederer wrote:
> > > > > > On Tue, May 27, 2008 at 02:49:24PM -0400, John Schmiederer
> > wrote:
> > > > > > > > > Attached is a patch to account for the transformation
> > > > > > > > > matrix
> > > > > > contained in the tkhd atom for proper display width/height.
> > > > > > > >
> [...]
>
> > > > > + //transform the display width/height according to the matrix
> > > > > + if (width && height) {
> > > > > + for (i = 0; i < 2; i++)
> > > > > + disp_transform[i] =
> > > > > + (int64_t) width * display_matrix[0][i] +
> > > > > + (int64_t) height * display_matrix[1][i] +
> > > > > + display_matrix[2][i];
> > > >
> > > > This is not what the original patch did.
> > >
> > > It may look a little different, but the functionality has not
> > changed.
> >
> > You removed the scaling by 1<<16 and 1<<30 the code is no longer doing
> > the same. The relative scaling of w*matrix[0] and matrix[2] has
> > changed They are added together so the result is more than just wrong
> > by a constant scale factor.
>
> Argh! You're right, the math is off.
> Although I maintain that the functionality didn't change from the first patch - that was wrong, too. =)
> I forgot that width and height were 16.16 fixed, so instead of multiplying by [width height 1] it should have always been [width height 1<<16]
>
> The 1<<16 vs 1<<30 scaling is no longer an issue though, as all the 1<<30 scaled terms were in that last column of the display matrix that I no longer read in or use (display_matrix[*][2], not display_matrix[2][*]).
>
> So in this updated patch all the multiplied terms are in the same scale.
[...]
> + //transform the display width/height according to the matrix
> + // to keep the same scale, use [width height 1<<16]
> + if (width && height) {
> + for (i = 0; i < 2; i++)
> + disp_transform[i] =
> + (int64_t) width * display_matrix[0][i] +
> + (int64_t) height * display_matrix[1][i] +
> + (int64_t) (display_matrix[2][i] << 16);
with that order of operations display_matrix[2][i] << 16 can overflow
also things can be vertically aligned like:
(int64_t) width * display_matrix[0][i] +
(int64_t) height * display_matrix[1][i] +
looks more pretty ...
anyway iam mostly ok with the patch after this, maybe baptiste has some
further comments though ...
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin
-------------- 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/20080529/b375c799/attachment.pgp>
More information about the ffmpeg-devel
mailing list