On Tue, 20 Aug 2013, John Stebbins wrote:

On 08/20/2013 07:17 AM, Martin Storsjö wrote:
On Mon, 19 Aug 2013, John Stebbins wrote:

QuickTime requires that the stsd.text box be completely filled in.
---
libavformat/movenc.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 0fc173a..8f3cd72 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -2975,18 +2975,25 @@ static int mov_write_packet(AVFormatContext *s, 
AVPacket *pkt)

// 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_chapter_track(AVFormatContext *s, int tracknum)
+static int mov_create_chapter_track(AVFormatContext *s, int tracknum)
{
    MOVMuxContext *mov = s->priv_data;
    MOVTrack *track = &mov->tracks[tracknum];
    AVPacket pkt = { .stream_index = tracknum, .flags = AV_PKT_FLAG_KEY };
    int i, len;
+    // These properties are required to make QT recognize the chapter track
+    uint8_t chapter_properties[43] = { 0, 0, 0, 0, 0, 0, 0, 1, };

    track->mode = mov->mode;
    track->tag = MKTAG('t','e','x','t');
    track->timescale = MOV_TIMESCALE;
    track->enc = avcodec_alloc_context3(NULL);
Unrelated to this patch, but needs fixing: This alloc needs to be checked.

    track->enc->codec_type = AVMEDIA_TYPE_SUBTITLE;
+    track->enc->extradata = av_malloc(sizeof(chapter_properties));
+    if (track->enc->extradata == NULL)
+        return AVERROR(ENOMEM);
+    track->enc->extradata_size = sizeof(chapter_properties);
+    memcpy(track->enc->extradata, chapter_properties, 
sizeof(chapter_properties));

I didn't read through all of movenc regarding to chapters yet, but I'd
like to be sure - did you test to make sure that things work out fine (or
error out cleanly) if this function returns an error?



I did not test what happens if I force an error here, but I inspected all calls 
to this function.  Nothing following
each call relies on the results of this function. The resulting file will have 
an entry in the moov for a chapter track
that isn't there assuming that writing the moov doesn't error out as well.

Right. I've got a suspicion that some of this might fail and/or leak on error, but that issue is also present before your patch and thus unrelated, so I'd say this patch is good then, I'll push tomorrow unless there's more issues.

Also, the task of checking that errors in this function is handled properly (add a check for the avcodec_alloc_context3 call, test to make sure there's no crash, no leaks) is up for grabs.

// Martin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to