[FFmpeg-devel] [PATCH] add QT CC track mux support

[Carl Eugen Hoyos]

Jason Livingston <jason <at> cpcweb.com> writes:

> This patch enables FFmpeg to mux/copy QuickTime 
> Closed Caption tracks.

Your patch contains a lot of trailing whitespace 
that cannot be committed to the FFmpeg repository 
(it would reject it), consider using tools/patcheck 
to find all occurrences.

> +    av_log(avctx, AV_LOG_VERBOSE, "Not yet implemented.\n");

Then why is the file there at all?
(Sorry if I miss something obvious.)

> -    avio_wb32(pb, 0x2C);   /* size */
> -    ffio_wfourcc(pb, "text");
> -    avio_wb16(pb, 0x01);
> -    avio_wb32(pb, 0x00);

...

+    if (track->tag != MKTAG('c','6','0','8')) {
+        avio_wb32(pb, 0x2C);   /* size */
+        ffio_wfourcc(pb, "text");
+        avio_wb16(pb, 0x01);
+        avio_wb32(pb, 0x00);

Please do not re-indent this block, to make the 
patch easier to read simply add the "if(" and the 
"}" line, leave the rest as-is.
(You may send an additional patch to re-indent.)

> -    if (track->tag == MKTAG('t','e','x','t')) mov_write_gmhd_tag(pb, track);
> -    else                                      mov_write_nmhd_tag(pb);
> +    if (track->tag == MKTAG('t','e','x','t') || track->tag == 
> MKTAG('c','6','0','8')) 
> +        mov_write_gmhd_tag(pb, track);
> +    else
> +        mov_write_nmhd_tag(pb);

The following is preferred:

if (tag == MKTAG) {
    mog_write_gmhd_tag();
} else {
     mov_write_nmhd_tag();
}

(This makes future changes smaller.)

Carl Eugen