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

[Michael Niedermayer]

On Wed, Jul 16, 2008 at 11:48:20AM -0400, John Schmiederer wrote:
> >-----Original Message-----
> >From: ffmpeg-devel-bounces at mplayerhq.hu [mailto:ffmpeg-devel-
> >bounces at mplayerhq.hu] On Behalf Of Baptiste Coudurier
> >Sent: Tuesday, July 15, 2008 11:11 PM
> >To: FFmpeg development discussions and patches
> >Subject: Re: [FFmpeg-devel] [PATCH] Use tkhd matrix for proper display
> >in mov
> >
> >Hi,
> >
> >Michael Niedermayer wrote:
> >> On Thu, May 29, 2008 at 10:49:22AM -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
> >>> Oops - I misapplied those parentheses to fix a "parentheses around +
> >or - inside shift" warning.
> >>> Attached patch does it right this time.
> >>>
> >>>> also things can be vertically aligned like:
> >>>>                 (int64_t) width  * display_matrix[0][i] +
> >>>>                 (int64_t) height * display_matrix[1][i] +
> >>>>
> >>>> looks more pretty ...
> >>> Agreed.
> >>>
> >>>> anyway iam mostly ok with the patch after this, maybe baptiste has
> >some
> >>>> further comments though ...
> >>> Thanks again for all the help.
> >>
> >> patch looks ok if someone tested it with a few mov files
> >>
> >
> >Is it possible to have the exact quote from the specifications
> >describing the correct interpretation of these values ?
> >
> >Is this valid in mov or/and isom ?
> 
> The tkhd matrix is defined in both the QuickTime (mov) documentation and the ISO standard, as such I believe it is valid for both:
[...]
> "[...] Not all
> derived specifications use matrices; if they are not used, they shall be set to the identity matrix, If a matrix is used, the point (p,q) is transformed into (p', q') using the matrix as follows:

Or in other words the matrix may or may not contain valid values in a .mp4
I guess checking if the matrix is the identity matrix in .mp4 would be
required at minimum in addition to the patch.


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

Complexity theory is the science of finding the exact solution to an
approximation. Benchmarking OTOH is finding an approximation of the exact
-------------- 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/20080716/a44b4783/attachment.pgp>