[FFmpeg-devel] [PATCH 4/4] movenc: Write QuickTime chapters

[David Conrad]

On Apr 19, 2010, at 5:27 PM, Baptiste Coudurier wrote:

> Hi David,
> 
> On 04/19/2010 10:24 AM, David Conrad wrote:
>> ---
>>  libavformat/movenc.c |   78 ++++++++++++++++++++++++++++++++++++++++++++++---
>>  1 files changed, 73 insertions(+), 5 deletions(-)
>> 
>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
>> index 4efed1b..3cec73c 100644
>> --- a/libavformat/movenc.c
>> +++ b/libavformat/movenc.c
>> @@ -77,12 +77,14 @@ typedef struct MOVIndex {
>>      MOVIentry   *cluster;
>>      int         audio_vbr;
>>      int         height; ///<  active picture (w/o VBI) height for D-10/IMX
>> +    int         qt_chapterid; ///<  trackID of the qt chapter track
> 
> Any reason why putting qt_ in front of the name ?

I thought it might be confusing since there's two different chapter methods, changed to use a more generic tref naming.

>>  } MOVTrack;
>> 
>>  typedef struct MOVMuxContext {
>>      int     mode;
>>      int64_t time;
>>      int     nb_streams;
>> +    int     qt_chapters; ///<  number of the qt chapter track
> 
> Ditto.
> 
> > [...]
>> 
>> +static int mov_write_packet(AVFormatContext *s, AVPacket *pkt);
> 
> Can the function be moved to avoid forward declaration ?
> If yes, please move it.


Done

>> +
>> +// QuickTime chapters involve an additional text track with the chapter names
>> +// as samples, and a tref pointing from the other tracks to the chapter one.
>> +static void mov_create_qt_chapter_track(AVFormatContext *s, int tracknum)
> 
> _qt_ seems useless to me. there is already mov_ prefix.
> 
>> +{
>> +    MOVMuxContext *mov = s->priv_data;
>> +    MOVTrack *track =&mov->tracks[tracknum];
>> +    AVPacket pkt = { .stream_index = tracknum, .flags = AV_PKT_FLAG_KEY };
>> +    int i, len;
>> +
>> +    track->mode = mov->mode;
>> +    track->tag = MKTAG('t','e','x','t');
>> +    track->timescale = MOV_TIMESCALE;
>> +    track->enc = avcodec_alloc_context();
> 
> Shouldn't enc be freed ?

It was after writing trak, but I guess it makes more sense to do so in write_trailer

> > [...]
> >
>> @@ -1832,7 +1894,11 @@ static int mov_write_header(AVFormatContext *s)
>>          }
>>      }
>> 
>> -    mov->tracks = av_mallocz(s->nb_streams*sizeof(*mov->tracks));
>> +    mov->nb_streams = s->nb_streams;
>> +    if (mov->mode&  (MODE_MOV|MODE_IPOD)&&  mov->nb_streams&&  s->nb_chapters)
>> +        mov->qt_chapters = mov->nb_streams++;
> 
> mov->nb_streams cannot be 0 since s->nb_streams cannot.

Fixed

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-movenc-Swap-positions-of-mov_write_header-and-mov_wr.patch
Type: application/octet-stream
Size: 10503 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100421/a984fa6f/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-movenc-Write-QuickTime-chapters.patch
Type: application/octet-stream
Size: 5952 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100421/a984fa6f/attachment-0001.obj>