[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 #339984|none                        |needs-work
             status|                            |

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

::: sys/decklink/gstdecklinkvanc.cpp
@@ +171,3 @@
+      guint32 w1 = ((guint32*) ptr) [1];
+      guint32 w2 = ((guint32*) ptr) [2];
+      guint32 w3 = ((guint32*) ptr) [3];

Might want to use a GstByteReader for going over the samples. For bounds
checking, keeping track of position, etc.

Also the above code is endianness dependent. It will behave different on little
endian and big endian systems. You probably want something like
gst_byte_reader_get_uint32_le() here, or GST_READ_UINT32_LE() or similar (I
assume your code is tested to work on little endian systems)

@@ +223,3 @@
+
+  for (gsize idx = 0; idx < data_size - min_adp_size; ++idx) {
+    // Find start of packet

Might want to use the GstByteReader API for reading over all the data, instead
of keeping track of position and doing bounds checking yourself

@@ +305,3 @@
+
+  BMDPixelFormat fmt = vanc_frame->GetPixelFormat();
+  GstByteWriter *writer = NULL;

Can be stack allocated, and you could initialize it with some sensible size
already

::: sys/decklink/gstdecklinkvanc.h
@@ +21,3 @@
+
+#ifndef _Decklink_VANC_H_
+#define _Decklink_VANC_H_

__GST_DECKLINK_VANC_H__

@@ +28,3 @@
+bool gst_processVANC(GstDecklinkVideoSrc *self,
+                     IDeckLinkVideoFrameAncillary *vanc_frame,
+                     GstBuffer * buffer);

gst_decklink_process_vanc(), gst_decklink_vanc_init() I guess. Also
G_GNUC_INTERNAL

::: sys/decklink/gstdecklinkvideosrc.cpp
@@ +198,3 @@
       "Sebastian Dröge <sebastian at centricular.com>");

+  gst_element_class_set_static_metadata (element_class, "Decklink VANC
Source",

Merge the names with the above call to set_static_metadata(). Only one per
element is allowed, a second call overwrites the old ones

@@ +201,3 @@
+      "Video/Subtitle", "Decklink VANC Source",
+      "Gavin Hurlbut <gavin at digital-nirvana.com>, "
+      "John Poet <jppoet at digital-nirvana.com");

Only add people here who actually wrote the thing :)

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