[FFmpeg-devel] [PATCH] Use tkhd matrix for proper display in mov

[Michael Niedermayer]

On Wed, May 28, 2008 at 02:48:33PM -0400, John Schmiederer wrote:
> > On Wed, May 28, 2008 at 11:14:31AM -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.
> > > > > >
> > > > > > > The video
> > > > > > >
> > > > http://samples.mplayerhq.hu/mov/tkhd_matrix/white_zombie_scrunch.m
> > > > > > > ov plays at 160x240 when it should really be scaled to
> > 320x240.
> > > > > >
> > > > > > > +    int i;
> > > > > > > +    int width;
> > > > > > > +    int height;
> > > > > > > +    float disp_transform[3];
> > > > > > > +    float display_matrix[3][3];
> > > > > >
> > > > > > please use integers, there is no need for floats.
> > > > > >
> > > > > Ok, I changed display_matrix to int, but left disp_transform as
> > > > > float
> > > > to avoid any possible overflow from multiplying large ints.
> > > >
> > > > use a*(int64_t)b
> > > >
> > > All right - no more nasty floats - I switched it to int64_t.
> > > The only caveat is a float cast when calculating the aspect ratio,
> > since multiplying an int_64 by an int_32 won't fit in 64bit precision.
> > >
> > > To Reimar's note "we do have function for multiplying two 16.16
> > fixed-point numbers correctly"; I couldn't find those functions.  If
> > anyone knows where that is off the top of their head, I would be
> > appreciated.  The closest thing I found was libavutil/integer, and that
> > didn't look used.
> > [...]
> >
> > > +    //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.

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope
-------------- 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/20080528/436f5577/attachment.pgp>