[FFmpeg-devel] [RFC] write standard stsd tags and objecttype for mp4/3gp

[Baptiste Coudurier]

Michael Niedermayer wrote:
> On Mon, Jan 28, 2008 at 12:44:57PM +0100, Baptiste Coudurier wrote:
>> Hi
>>
>> I had that in my tree since some time now anyway, and it will avoid
>> other people wasting their time.
>>
>> This patch should do what's needed, except a few things:
>>
>> - stream copying mpeg-2 removes sequence header and puts it in extradata
>> -> is the mpegvideo split function actually used ? Can mpeg2 be stored
>> in any container without seq header ?
>> -> -vglobal 4 makes it working but it still puts extradata, and then
>> extradata is written in decodec specific tag, Im pretty sure this is
>> should not be done.
>>
>> - set correct profiles for mpeg-2.
>>
>> One patch to move prefered object types to be be chosen first.
>> One other patch to enable muxing.
>>
> 
>> This should be open for discussion, and I hope this time it won't turn
>> into flamewar.
> 
> i hope as well
> 
> 
> [...]
>> Index: libavformat/movenc.c
>> ===================================================================
>> --- libavformat/movenc.c	(revision 11646)
>> +++ libavformat/movenc.c	(working copy)
>> @@ -60,7 +60,7 @@
>>      int         hasBframes;
>>      int         language;
>>      int         trackID;
>> -    int         tag;
>> +    int         tag; ///< stsd fourcc
>>      AVCodecContext *enc;
>>  
>>      int         vosLen;
> 
> ok, commit anytime
> 
> 
> 
>> @@ -381,7 +381,7 @@
>>          track->enc->codec_id == CODEC_ID_PCM_S24LE ||
>>          track->enc->codec_id == CODEC_ID_PCM_S32LE))
>>          mov_write_wave_tag(pb, track);
>> -    else if(track->enc->codec_id == CODEC_ID_AAC)
>> +    else if(track->tag == MKTAG('m','p','4','a'))
>>          mov_write_esds_tag(pb, track);
>>      else if(track->enc->codec_id == CODEC_ID_AMR_NB)
>>          mov_write_amr_tag(pb, track);
> 
> ok, commit anytime



> [...]
>> +static int mov_find_codec_tag(AVFormatContext *s, MOVTrack *track)
>>  {
>>      int tag = track->enc->codec_tag;
>> -    if (!tag) {
>> +    if (track->mode == MODE_MP4 || track->mode == MODE_PSP) {
>> +        if (!codec_get_tag(ff_mp4_obj_type, track->enc->codec_id))
>> +            return 0;
> 
> this breaks h.263 muxing (assuming it was spec compliant before)
> as ff_mp4_obj_type doesnt have a h263 entry

Well AFAIK, h.263 is not supported in mpeg-4 yet. If I find specs
defining it,
I'll for sure support it.

>> +        if (track->enc->codec_id == CODEC_ID_H264)           tag = MKTAG('a','v','c','1');
>> +        else if (track->enc->codec_type == CODEC_TYPE_VIDEO) tag = MKTAG('m','p','4','v');
>> +        else if (track->enc->codec_type == CODEC_TYPE_AUDIO) tag = MKTAG('m','p','4','a');
> 
> this looks ok i think if you checked the spec, otherwise someone should
> check to make sure mp4v/mp4a are mandated by the latest spec instead of
> samr/sawb/...

Again AFAIK, amr is not supported in mpeg-4 yet. Only in 3gp like h.263

>> +    } else if (track->mode == MODE_3GP || track->mode == MODE_3G2) {
>> +        tag = codec_get_tag(codec_3gp_tags, track->enc->codec_id);
> 
> again, depends on what the spec says, if you checked it, iam ok with it

3gp is really clear and specific hopefully, It is by far IMHO the proper
derivative of iso media.

> [...]
>> +    track->tag = tag;
>>      return tag;
> 
> i think retunng the tag and vissibly setting it is more readable
> that is IMHO
>  track->tag = mov_find_video_codec_tag(s, track);
> is better than
> if (!mov_find_codec_tag(s, track)) {
>     av_log(s, AV_LOG_ERROR, "track %d: could not find tag for codec\n", i);
>     return -1;
> }
> 
> where mov_find_codec_tag() has a "hidden" sideeffect of setting track->tag
> 

Im fine with it.

Updated patch attached.

[...]

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
SMARTJOG S.A.                                    http://www.smartjog.com
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
Phone: +33 1 49966312
-------------- next part --------------
A non-text attachment was scrubbed...
Name: movenc_stsd_tag2.patch
Type: text/x-diff
Size: 4182 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080128/1f43109a/attachment.patch>