GNOME Bugzilla – Bug 794901
Closed Caption and Ancillary data support
Last modified: 2018-04-09 13:18:01 UTC
These patches add some utilities for: * Ancillary data (according to SMPTE S334M) * VBI parsing (for HD formats) * Closed Caption metadata support * Closed Caption caps support
Created attachment 370440 [details] [review] pbutils: Add Closed Caption caps description
Created attachment 370441 [details] [review] video: Add support for VANC and Closed Caption This commits add common elements for Ancillary Data and Closed Caption support in GStreamer: * A VBI (Video Blanking Interval) parser that supports detection and extraction of Ancillary data according to the SMPTE S291M specification. Currently supports the v210 and UYVY video formats. * A new GstMeta for Closed Caption : GstVideoCaptionMeta. This supports the two types of CC : CEA-608 and CEA-708, along with the 4 different ways they can be transported (other systems are super-set of those).
Created attachment 370442 [details] [review] playback: Add support for CEA 608/708 CC overlay elements
Do you have the actual element that does the cc parsing & rendering somewhere? There were some actual code in bug #704881 Does this replace bug #678146 ?
(In reply to Olivier Crête from comment #4) > Do you have the actual element that does the cc parsing & rendering > somewhere? There were some actual code in bug #704881 I have updated patches for the rendering/overly which I will put there > > Does this replace bug #678146 ? No, this is the -base part for support in GStreamer.
Review of attachment 370441 [details] [review]: ::: gst-libs/gst/video/video-anc.c @@ +205,3 @@ + * gst_video_vbi_parser_get_ancillary: + * @parser: a #GstVideoVBIParser + * @anc: (out): a #GstVideoAncillary to start the eventual ancillary data This seems more like a case of (out caller-allocates) Also this is not very bindings-friendly (or rather GI-friendly). There would be no automatic memory management of the data in the VANC struct (another reason to make it a fixed 255 byte array). @@ +411,3 @@ + break; + default: + GST_ERROR ("UNSUPPORTED FORMAT !"); g_assert_not_reached() or not? @@ +449,3 @@ + return FALSE; + } else { + /* return FALSE, if transform type is not supported */ AFAIU this meta could be copied over always. It's like a separate stream, it does not depend on anything inside the buffer @@ +461,3 @@ + GstVideoCaptionMeta *emeta = (GstVideoCaptionMeta *) meta; + + emeta->caption_type = GST_VIDEO_CAPTION_TYPE_UNKNOWN; You need to set all things to 0 if needed, metas are not initialized by default @@ +471,3 @@ + GstVideoCaptionMeta *emeta = (GstVideoCaptionMeta *) meta; + + g_free (emeta->data); ... otherwise you might free a random pointer here ::: gst-libs/gst/video/video-anc.h @@ +49,3 @@ + guint8 SDID_block_number; + guint8 data_count; + guint8 *data; Maybe this should just be a fixed 255 byte array?
Review of attachment 370441 [details] [review]: ::: gst-libs/gst/video/video-anc.c @@ +205,3 @@ + * gst_video_vbi_parser_get_ancillary: + * @parser: a #GstVideoVBIParser + * @anc: (out): a #GstVideoAncillary to start the eventual ancillary data Did you have anything in mind ? @@ +411,3 @@ + break; + default: + GST_ERROR ("UNSUPPORTED FORMAT !"); Technically this shouldn't happen (since we check the format when creating the parser), but I'll add one. @@ +461,3 @@ + GstVideoCaptionMeta *emeta = (GstVideoCaptionMeta *) meta; + + emeta->caption_type = GST_VIDEO_CAPTION_TYPE_UNKNOWN; ack @@ +471,3 @@ + GstVideoCaptionMeta *emeta = (GstVideoCaptionMeta *) meta; + + g_free (emeta->data); ack ::: gst-libs/gst/video/video-anc.h @@ +49,3 @@ + guint8 SDID_block_number; + guint8 data_count; + guint8 *data; Ack, and avoid the potentially confusing alloc/copy.
Created attachment 370481 [details] [review] pbutils: Add Closed Caption caps description
Created attachment 370482 [details] [review] video: Add support for VANC and Closed Caption This commits add common elements for Ancillary Data and Closed Caption support in GStreamer: * A VBI (Video Blanking Interval) parser that supports detection and extraction of Ancillary data according to the SMPTE S291M specification. Currently supports the v210 and UYVY video formats. * A new GstMeta for Closed Caption : GstVideoCaptionMeta. This supports the two types of CC : CEA-608 and CEA-708, along with the 4 different ways they can be transported (other systems are super-set of those).
Created attachment 370483 [details] [review] playback: Add support for CEA 608/708 CC overlay elements
Review of attachment 370482 [details] [review]: ::: gst-libs/gst/video/video-anc.c @@ +205,3 @@ + * gst_video_vbi_parser_get_ancillary: + * @parser: a #GstVideoVBIParser + * @anc: (out): a #GstVideoAncillary to start the eventual ancillary data (out caller-allocates) @@ +450,3 @@ + return FALSE; + } else { + /* return FALSE, if transform type is not supported */ Same comment as before here, I think we can always copy the meta because it's like a separate stream
Fixed the issues and pushed Attachment 370481 [details] pushed as 43254a2 - pbutils: Add Closed Caption caps description Attachment 370482 [details] pushed as 9dceb6c - video: Add support for VANC and Closed Caption Attachment 370483 [details] pushed as 1d2a311 - playback: Add support for CEA 608/708 CC overlay elements