[FFmpeg-devel] [PATCH] mov tkhd' width and height usage

[Maksym Veremeyenko]

Baptiste Coudurier ???????(??):
[...]
>>       /* Track width and height, for visual only */
>>       if(st&&  (track->enc->codec_type == AVMEDIA_TYPE_VIDEO ||
>>                 track->enc->codec_type == AVMEDIA_TYPE_SUBTITLE)) {
>> -        double sample_aspect_ratio = av_q2d(st->sample_aspect_ratio);
>> -        if(!sample_aspect_ratio || track->height != track->enc->height)
>> -            sample_aspect_ratio = 1;
>> -        put_be32(pb, sample_aspect_ratio * track->enc->width*0x10000);
>> -        put_be32(pb, track->height*0x10000);
>> +        put_be32(pb, track->enc->width<<  16);
>> +        put_be32(pb, track->height<<  16);
> 
> So, did you confirm that this works for mp4 ? Otherwise we need to keep 
> the old logic in that case.

i can't confirm that (i can't create an 16:9 MP4 file with Quicktime...) 
so i keep a login proposed in first patch, i.e. store codec's width and 
height for MODE_MOV tracks only


>> +    width = av_q2d(track->enc->sample_aspect_ratio) * track->enc->width;
> 
> Please avoid using doubles, use av_rescale.
replaced with
+    int32_t width = av_rescale(track->enc->sample_aspect_ratio.num, 
track->enc->width,
+                               track->enc->sample_aspect_ratio.den);


> 
>> +
>> +    put_be32(pb, 0x14);
> 
> Use sizes in decimal.
fixed

[...]
> Maybe inlining the enof and clef atoms in tapt is simpler, but I don't 
> care much about that, do as you prefer.
fixed


-- 
________________________________________
Maksym Veremeyenko