[FFmpeg-devel] [PATCH] mp4 and ipod metadata
Baptiste Coudurier
baptiste.coudurier
Sun Jun 15 02:38:23 CEST 2008
Michael Niedermayer wrote:
> On Sat, Jun 14, 2008 at 03:12:49PM -0700, Baptiste Coudurier wrote:
>> Michael Niedermayer wrote:
>>> On Fri, Jun 13, 2008 at 09:14:13PM -0700, Baptiste Coudurier wrote:
>>>> 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.
>> Changed the logic.
>>
>>> [...]
>>>> 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.
>>> hmm, i would vote for leaving isom in there being able to listen to
>>> the audio of a music video is nice by itself.
>>>
>> Yeah I agree.
>>
>> I reversed the condition for psp and for mov since I thought it was
>> similar. This is the patch I'd like to have applied at the end.
>>
>> Tell me if you want another pretty printing code.
>>
>> And, again, I found something with the minor version in 3gp/3g2:
>>
>> "MinorVersion: This identifies the minor version of the brand. Files
>> with brand '3g2x', where x is an alphabetic character, shall have a
>> corresponding release X.Y.Z such that X = 1 when x = 'a'; X = 2 when x =
>> 'b'; and so on. A conforming minor version value for releaseX.y.z uses
>> the byte aligned and right adjusted value of release X*2562 + y*256+z."
>>
>> While for 3gp is:
>> "MinorVersion: This identifies the minor version of the brand. For
>> files with brand '3gLZ', where L is a letter and Z a digit, and
>> conforming to version Z.x.y of this specification, this field takes the
>> value x*256 + y."
>>
>> First 3gp standard defining the use of brand and defining specific 3gp
>> file format was 4.2.0 (TS 26.234 4.2.0), this explains 0x200.
>>
>> 3gp6 we can use 6.0.0, therefore 0 as minor.
> [...]
>> @@ -1414,16 +1418,16 @@
>> put_tag(pb, "iso2");
>> if(has_h264)
>> put_tag(pb, "avc1");
>> + if(mov->mode == MODE_MP4)
>> + put_tag(pb, "mp41");
>> + if(mov->mode == MODE_PSP)
>> + put_tag(pb, "MSNV");
>
>> + else if(has_3gp_tags == s->nb_streams){
>> + put_tag(pb, has_h264 ? "3g2b":"3g2a");
>> + put_tag(pb, has_h264 ? "3gp6":"3gp4");
>> + }
>
> This is wrong in several cases
I believe it is not:
> 1. i belive only 3gpp2 requires all streams to be 3gpp compatible
Yes, we have our mp4 not mp42 compatible, 3gp compatible with 3g2, and
3g2 compatible with 3gp in any case (no 3g2 specific codec), and
find_codec_tag which won't allow any non defined codec in 3gp/3g2, thus
has_3gp_tags == s->nb_streams in any case of -f 3g/2.
This case if for mp4 when they contain mp4a/mp4v which is compatible
with 3g/2.
Furthermore, ensuring all codecs are 3gp ones before putting 3g brands
is better IMHO for the phones.
> 2. 3GPP2 C.S0050-B v1.0:
> The brand identifier for this specification is '3g2c'. This brand identifier shall occur in
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> the compatible brands list, and may also be the primary brand. Readers should check
> ^^^^^^^^^^^^^^^^^^^^^^^^^^
> the compatible brands list for the identifiers they recognize, and not rely on the file
> having a particular primary brand, for maximum compatibility. Files may be compatible
> with more than one brand, and have a 'best use' other than this specification, yet still
> be compatible with this specification.
>
> The issue i see here is that -f 3g2 has it in the primary but not the
> compatible list. Thus not really generating 3g2 (incompatible track aside)
> IMHO we either should try to generate a file as close to 3gp2 or not generate
> one at all if its not possible to follow the spec down to every dot.
>
I repeat I don't see how this could arise, code only allows in -f 3gp/2
codecs specified in standard, thus it will always have has_3gp_tags ==
s->nb_streams for 3gp/2.
Are you ok with the minor version patch otherwise ?
--
Baptiste COUDURIER GnuPG Key Id: 0x5C1ABAAA
Smartjog USA Inc. http://www.smartjog.com
Key fingerprint 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
More information about the ffmpeg-devel
mailing list