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

[Michael Niedermayer]

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
1. i belive only 3gpp2 requires all streams to be 3gpp compatible
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.

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

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus
-------------- 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/20080615/1091ff82/attachment.pgp>