GNOME Bugzilla – Bug 606643
isomp4: closed captions support (muxing and demuxing)
Last modified: 2018-04-23 14:12:50 UTC
That file has closed captions: http://trac.webkit.org/export/53075/trunk/LayoutTests/media/content/counting-captioned.mov Some pointers that could maybe (or not!) help to implement this: - the quicktime overview mentions the CEA 608 format: http://developer.apple.com/quicktime/overview.html - http://developer.apple.com/mac/library/samplecode/ClosedCaptionImporter/ - http://www.ce.org/Standards/browseByCommittee_2523.asp - a patch for mp4v2: http://forum.handbrake.fr/viewtopic.php?f=4&t=5144#p58091
There is also CEA 708 (for HD content): Samples and info : https://www.cpcweb.com/hdtv/aja.htm
(In reply to Philippe Normand from comment #0) > That file has closed captions: > > http://trac.webkit.org/export/53075/trunk/LayoutTests/media/content/counting- > captioned.mov About this video, if someone is working with it: I played with quicktime (OSX), and it showed just the closed captions of pair numbers. With VLC (ArchLinux) the closed captions wasn't detect. And I confirmed the closed captions showed by parsing the file. Can I work on it? But I need some guidance on plugin architecture.
Created attachment 367704 [details] [review] qtdemux: Detect and expose CEA 608/708 Closed Caption tracks
Created attachment 367985 [details] [review] qtdemux: Detect and expose CEA 608/708 Closed Caption tracks
Created attachment 367986 [details] [review] qtmux: Refactor pad re-negotiation code It was similar for all pads
Created attachment 367987 [details] [review] isomp4: Make 'gmhd' atom usage more generic Only the 'gmin' atom is required. Any other entry within it are optional.
Created attachment 367988 [details] [review] isomp4: qtmux: Add Closed Caption support Supports CEA 608 and CEA 708 CC streams (as byte pairs)
Review of attachment 367985 [details] [review]: Looks good, have a question/optimisation, but I doubt it really relevant. ::: gst/isomp4/qtdemux.c @@ +5427,3 @@ + gst_buffer_unref (buf); + if (cc) { + buf = _gst_buffer_new_wrapped (cc, cclen, g_free); Couldn't we sub-buffer this instead of copying ? (i.e. buffer_copy() buffer_resize())
Review of attachment 367986 [details] [review]: Good idea.
Review of attachment 367987 [details] [review]: Just a question. ::: gst/isomp4/atoms.c @@ +4019,3 @@ + gmhd->tmcd = atom_tmcd_new (); + gmhd->tmcd->tcmi.text_size = 12; + gmhd->tmcd->tcmi.font_name = g_strdup ("Chicago"); /* Pascal string */ Could you pass these two as parameter to _new() ?
Review of attachment 367988 [details] [review]: ::: gst/isomp4/atoms.c @@ +4061,3 @@ + + /* FIXME : Set the timescale to the same timescale as the video trak ! */ + trak->mdia.mdhd.time_info.timescale = 30000; Does this have any known side effects ? What would it take to carry the track timescale ? ::: gst/isomp4/gstqtmux.c @@ +884,3 @@ + + gst_buffer_map (newbuf, &map, GST_MAP_WRITE); + gst_buffer_map (buf, &inmap, GST_MAP_READ); Shouldn't you check your return value ? @@ +888,3 @@ + GST_WRITE_UINT32_BE (map.data, map.size); + GST_WRITE_UINT32_LE (map.data + 4, FOURCC_cdat); + memcpy (map.data + 8, inmap.data, inmap.size); Why not use gst_buffer_prepend_memory() ? (note I have no idea what size of buffers we are talking about here)
After more thinking, we need to handle CC diferently in these cases (via GstMeta). will figure out better patches once I have a full end-to-end chain working.
Ok, no worries.
Attachment 367985 [details] pushed as 2869ede - qtdemux: Detect and expose CEA 608/708 Closed Caption tracks Attachment 367986 [details] pushed as cc0c278 - qtmux: Refactor pad re-negotiation code
Created attachment 370690 [details] [review] isomp4: Make 'gmhd' atom usage more generic Only the 'gmin' atom is required. Any other entry within it are optional.
Created attachment 370691 [details] [review] isomp4: qtmux: Add Closed Caption support Supports CEA 608 and CEA 708 CC streams Also supports usage in "Robust Prefill" mode if the incoming caption stream is constant (i.e. there is one incoming CC buffer for each video frame).