[Bug 773863] decklinkvideosrc: add VANC parsing

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=773863

Sebastian Dröge (slomo) <slomo at coaxion.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #339845|none                        |needs-work
             status|                            |

--- Comment #21 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Review of attachment 339845:
 --> (https://bugzilla.gnome.org/review?bug=773863&attachment=339845)

Now only cosmetical bits left :) And documentation

::: gst-libs/gst/vanc/Makefile.am
@@ +21,3 @@
+    $(GST_PLUGINS_BASE_CFLAGS) \
+    -DGST_USE_UNSTABLE_API \
+    $(GST_CXXFLAGS)

No CXXFLAGS, this is plain C

::: gst-libs/gst/vanc/gstvancmeta.c
@@ +77,3 @@
+
+  } else {
+    GST_WARNING ("gst_vanc_meta_transform type not supported");

I think you should do the same for all transforms, there is no possible
transform that would make the vanc metadata invalid. It's like a separate
stream, right?

@@ +96,3 @@
+gst_vanc_meta_get_info (void)
+{
+  static const GstMetaInfo *meta_info = NULL;

Not const

@@ +112,3 @@
+GstVANCMeta *
+gst_buffer_add_vanc_meta (GstBuffer * buffer,
+    gint did, gint dbn_sdid, guint16 * data, gsize size, guint16 checksum)

data_id vs did

@@ +127,3 @@
+  vmeta->vanc_data = g_array_sized_new (FALSE, FALSE, sizeof (guint16), size);
+
+  g_array_append_vals (vmeta->vanc_data, data, size);

GBytes, or storage as (guint16 *, gsize) pair. GArray is too much overhead for
data that is not going to be modified in 99.999% of the cases

::: gst-libs/gst/vanc/gstvancmeta.h
@@ +45,3 @@
+  gint             dbn_sdid;  // Secondary Data ID (type 2)
+  guint16          checksum;  // Checksum of raw ancillary data
+  GArray          *vanc_data; // Raw ancillary data

No C99/C++ comments (// vs /* */)

A GBytes instead of a GArray still seems nicer for the data

@@ +55,3 @@
+
+GstVANCMeta * gst_buffer_add_vanc_meta (GstBuffer * buffer,
+                                        gint did, gint dbn_sdid,

It's called data_id above, and did only here

::: pkgconfig/gstreamer-plugins-bad-uninstalled.pc.in
@@ +11,3 @@
 Version: @VERSION@
 Requires: gstreamer- at GST_API_VERSION@
+Libs: -L at abs_top_builddir@/gst-libs/gst/audio/.libs
-L at abs_top_builddir@/gst-libs/gst/basecamerabinsrc/.libs
-L at abs_top_builddir@/gst-libs/gst/codecparsers/.libs
-L at abs_top_builddir@/gst-libs/gst/gl/.libs
-L at abs_top_builddir@/gst-libs/gst/insertbin/.libs
-L at abs_top_builddir@/gst-libs/gst/interfaces/.libs
-L at abs_top_builddir@/gst-libs/gst/mpegts/.libs
-L at abs_top_builddir@/gst-libs/gst/player/.libs
-L at abs_top_builddir@/gst-libs/gst/signalprocessor/.libs
-L at abs_top_builddir@/gst-libs/gst/video/.libs
-L at abs_top_builddir@/gst-libs/gst/vanc/.libs
-L at abs_top_builddir@/gst-libs/gst/wayland/.libs

Should also add a (two) pkg-config files for the vanc library alone

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.