[FFmpeg-devel] [PATCH] mp4 and ipod metadata

[Baptiste Coudurier]

Michael Niedermayer wrote:
> On Fri, Jun 13, 2008 at 02:26:24PM -0700, Baptiste Coudurier wrote:
>> Baptiste Coudurier wrote:
>>> Michael Niedermayer wrote:
>>>>>> [...]
>>>>>>
>>>>>> And above all even if no spec defined it, that would not affect that
>>>>>> its default value of 2 would be valid for all specs then (as none
>>>>>> defined it otherwise).
>>>>>> Its kinda simple either one defines it or none defines it. Either way
>>>>>> there the incompatibility you claimed does not exist.
>>>>>> Whats even more ridiculous is that you insist to only support a ancient
>>>>>> revission of the 3gp spec because the later REQUIRE all 3gp files to
>>>>>> claim to be iso media compatible. And you seem to prefer if they are
>>>>>> not compatible.
>>>>> This is false. 
>>>> what is false?
>>> That I insist to only support an ancient revision. I dont prefer them
>>> not being compatible, I'm definitely ok to put 'isom' in compatible
>>> brands, and as much compatible brands that it is possible.
>>>
>> Here is an attempt, I may have overlooked things.
> 
> This patch changes quite a lot of things at the same time ...
> 

Yes. I'll split the commits.

> 
>> +        if (codec_get_tag(codec_3gp_tags, st->codec->codec_id))
>> +            has_3gp_tags |= 1<<i;
> 
> will fail with >32 streams

Damn. Ok.

>> [...]
>>  
> 
>> @@ -1407,18 +1414,18 @@
>>      if(mov->mode != MODE_MOV){
>>          put_tag(pb, "isom");
>>          put_tag(pb, "iso2");
>> -    }
>> +        if(has_h264)
>> +            put_tag(pb, "avc1");
>> +        if(mov->mode != MODE_PSP){
>> +            if(has_3gp_tags == (1<<s->nb_streams)-1){
>> +                put_tag(pb, has_h264 ? "3g2b":"3g2a");
>> +                put_tag(pb, has_h264 ? "3gp6":"3gp4");
>> +            }
> 
> This is incorrect, as an example why, lets consider a video stream and
> 2 audio streams. One which is allowed in 3gp6 and one which is allowed in
> mp41. The file is clearly compatible with both not with none.
> (ignoring the missing tracks issue for the moment)

What I found:
In 3gp Release 5:

"- the maximum number of tracks shall be one for video, one for audio
and one for text;"

3GPP TS 26.234 V5.7.0 (2005-03), yes, dates are really wierd.

this is not mentioned for 3gp Release 4 however, but,

in latest 3GPP TS 26.244 V7.3.0 (2007-12):

"NOTE 1:
The Basic profile of 3GP in Release 6 and 7 corresponds to 3GP files of
earlier releases, which did not define profiles. Files with brands
?3gp4? and ?3gp5? in Release 4 and 5, respectively, correspond to files
with brand ?3gp6? in Release 6 and ?3gp7? in Release 7."

Also in 3GPP2 C.S0050-B:

"Conditions for using 3GPP branding are that the media types contained
in the ".3g2" file are restricted to those identified for use in the
".3gp" file format [5]."

So 3GPP2 disagrees with you it seems.

Now, yes, 3GPP does not mandates this it seems, and anyway we are not
using any 3G2 specific codecs, but QCELP is definitely on the way :>

> Besides
> 3GPP TS 26.244 V2.0.0 (2004-03)
> "The brand identifier (of one of the profiles) must occur in the compatible-brands list"
> 
> So i think that if we claim 3g* in the major brand, we also should list
> that in the compatible brands.

Yes, and this is the case in the code, because code only allows codec
supported by 3gp in MODE_3GP/3G2. So code will put 3gp4 as major and put
it as compatible, if h264 it will put 3gp6 as major and 3gp6 in compatible.

Also:

"5.5	File-branding guidelines

[...]

-	that a reader implementing that specification (possibly only that
specification) is given permission to read and interpret the file."

If we put codecs not supported, the reader will not be given permission
to read the file IMHO.

"All 3GP files of Release 5 or later shall contain the compatible brand
?isom? indicating that they conform to the ISO base media file format,
unless the reader is required to interpret extensions specific to the
AVC file format [20], for which case the compatible brand ?avc1? shall
be used instead (see note 2)"

Seems if we want to force the player to be able read H.264 track it
seems with must not put "isom". Do we want ? He might still read the
audio track.

> i also would prefer
> if(mov->mode == MODE_PSP)
> else
> 
> over 
> mov->mode != MODE_PSP 
> else
> due to it being more readable

Ok.

> and last, "mp41" disapears, yes i know we are not compatible to it but it was
> there and its removial will likely break some players.
> besides if the user asks for .mp4 it would only be logic to do our best to
> generate one. Generating pure isom instead seems a little odd ...
> 

Ok.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Smartjog USA Inc.                                http://www.smartjog.com
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA