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

Michael Niedermayer michaelni
Sat Jun 14 02:55:40 CEST 2008


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 ...

[...]
> Index: libavformat/movenc.c
> ===================================================================
> --- libavformat/movenc.c	(revision 13766)
> +++ libavformat/movenc.c	(working copy)
> @@ -1373,32 +1373,39 @@
>      return 0;
>  }
>  
> -/* TODO: This needs to be more general */
>  static void mov_write_ftyp_tag(ByteIOContext *pb, AVFormatContext *s)
>  {
>      MOVContext *mov = s->priv_data;
>      offset_t pos = url_ftell(pb);
> +    int has_h264 = 0, has_video = 0;
> +    int has_3gp_tags = 0;
>      int i;
>  
> +    for (i = 0; i < s->nb_streams; i++) {
> +        AVStream *st = s->streams[i];
> +        if (st->codec->codec_type == CODEC_TYPE_VIDEO)
> +            has_video = 1;
> +        if (st->codec->codec_id == CODEC_ID_H264)
> +            has_h264 = 1;
> +        /* XXX check 3g2 specific codecs later
> +         * h263 in mp4 matches but fails in find_codec_tag */

> +        if (codec_get_tag(codec_3gp_tags, st->codec->codec_id))
> +            has_3gp_tags |= 1<<i;

will fail with >32 streams


> +    }
> +
>      put_be32(pb, 0); /* size */
>      put_tag(pb, "ftyp");
>  
>      if (mov->mode == MODE_3GP)

> -        put_tag(pb, "3gp4");
> +        put_tag(pb, has_h264 ? "3gp6":"3gp4");
>      else if (mov->mode & MODE_3G2)
> -        put_tag(pb, "3g2a");
> +        put_tag(pb, has_h264 ? "3g2b":"3g2a");
>      else if (mov->mode == MODE_PSP)
>          put_tag(pb, "MSNV");
>      else if (mov->mode == MODE_MP4)
>          put_tag(pb, "isom");
>      else if (mov->mode == MODE_IPOD) {
> -        for (i = 0; i < s->nb_streams; i++)
> -            if (s->streams[i]->codec->codec_type == CODEC_TYPE_VIDEO) {
> -                put_tag(pb, "M4V ");
> -                break;
> -            }
> -        if (i == s->nb_streams)
> -            put_tag(pb, "M4A ");
> +        put_tag(pb, has_video ? "M4V ":"M4A ");
>      } else
>          put_tag(pb, "qt  ");
>  

> @@ -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)

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.

i also would prefer
if(mov->mode == MODE_PSP)
else

over 
mov->mode != MODE_PSP 
else
due to it being more readable

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 ...

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

I have often repented speaking, but never of holding my tongue.
-- Xenocrates
-------------- 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/20080614/b5503bd4/attachment.pgp>



More information about the ffmpeg-devel mailing list