[FFmpeg-devel] [RFC] comment mpegvideo_split

[Baptiste Coudurier]

Hi,

Michael Niedermayer wrote:
> On Wed, Mar 19, 2008 at 04:11:37PM +0100, Baptiste Coudurier wrote:
>> Baptiste Coudurier wrote:
>>> Michael Niedermayer wrote:
>>>> On Tue, Mar 18, 2008 at 01:16:34AM +0100, Baptiste Coudurier wrote:
>>>>> Hi,
>>>>>
>>>>> Currently, mpegvideo split function causes problems with containers
>>>>> setting AVFMT_GLOBAL_HEADER, mov and mp4 in particular, because
>>>>> MPEG-1/2 in those containers are to be muxed normally.
>>>>>
>>>>> I don't know any particular usage of the mpegvideo split function, so I
>>>>> tend to want to comment it, to finally fix this issue.
>>>>>
>>>>> Anyone see another better alternative ?
>>>> After a few more seconds :)
>>>> The muxers could just dump extradata for the case of mpeg1/2 before keyframes,
>>>> that should fix it i think. Its of course a little odd to first extract and
>>>> then put it back in the muxer.
>>>>
>>> Would the attached patch be ok ?
>>>
>> This one is more correct, messy though.
>>
>> -- 
>> Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
>> SMARTJOG SAS                                     http://www.smartjog.com
>> Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
>> Phone: +33 1 49966312
> 
>> Index: libavformat/movenc.c
>> ===================================================================
>> --- libavformat/movenc.c	(revision 12503)
>> +++ libavformat/movenc.c	(working copy)
>> @@ -386,7 +386,8 @@
>>          mov_write_esds_tag(pb, track);
>>      else if(track->enc->codec_id == CODEC_ID_AMR_NB)
>>          mov_write_amr_tag(pb, track);
>> -    else if(track->vosLen > 0)
>> +    else if(track->enc->codec_id != CODEC_ID_MPEG2VIDEO &&
>> +            track->vosLen > 0)
>>          mov_write_glbl_tag(pb, track);
>>  
>>      return updateSize (pb, pos);
> 
> I think this is unneeded ?

Well, for known wrapping methods, I prefer avoiding to write unneeded atom,
which have the tendency to confuse FCP or Quicktime.

>> @@ -1537,6 +1538,7 @@
>>      AVCodecContext *enc = trk->enc;
>>      unsigned int samplesInChunk = 0;
>>      int size= pkt->size;
>> +    int dump_extradata = 0;
>>  
>>      if (url_is_streamed(s->pb)) return 0; /* Can't handle that */
>>      if (!size) return 0; /* Discard 0 sized packets */
>> @@ -1582,6 +1584,10 @@
>>          trk->vosLen = 640;
>>          trk->vosData = av_malloc(trk->vosLen);
>>          memcpy(trk->vosData, pkt->data, 640);
>> +    } else if (enc->codec_id == CODEC_ID_MPEG2VIDEO && trk->vosLen > 4 &&
>> +               AV_RB32(trk->vosData) == 0x000001b3 && pkt->flags & PKT_FLAG_KEY) {
>> +        size += trk->vosLen;
>> +        dump_extradata = 1;
>>      }
>>  
>>      if (!(trk->entry % MOV_INDEX_CLUSTER_SIZE)) {
>> @@ -1609,6 +1615,10 @@
>>      trk->sampleCount += samplesInChunk;
>>      mov->mdat_size += size;
>>  
>> +    if (dump_extradata) {
>> +        put_buffer(pb, trk->vosData, trk->vosLen);
>> +        size -= trk->vosLen;
>> +    }
>>      put_buffer(pb, pkt->data, size);
>>  
>>      put_flush_packet(pb);
> 
> ok, its not beautifull but i dont know a better solution. Maybe some
> auto insertion of a bitstream filter, that could also be usefull to
> simplify other cases ...
> Anyway i think its better to commit this, we can always revert it if
> someone does find a better solution.

Sure.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
SMARTJOG SAS                                     http://www.smartjog.com
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
Phone: +33 1 49966312