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

[Richard Sotke]

Am Mittwoch, 16. Juli 2008 05:10:31 schrieb Baptiste Coudurier:
> 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 ?

I would use the transformation matrix only for the QuickTime File Format MOV. 
and maybe? for Apples iTunes Format M4V.
I wouldn't use the  transformation matrix, to correct the presentation of 
anamorphic video, at players which don't support par/sar. 
If the transformation matrix is used for this, I would remove the anamorphic 
par/sar information from the video stream, to avoid trouble with future 
MOV/M4V hardware/software players.
Here someone already mentioned problems with a player:
http://forum.doom9.org/showthread.php?p=1146980#post1146980

The MP4 File Format doesn't use the transformation matrix, default value are 
set. In the MP4 File Format you can get the same result via BIFS.

Regards
Richard