[Bug 791918] Add KLV meta and add support for it in matroska mux/demux

[GStreamer (GNOME Bugzilla)]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #365926|none                        |reviewed
             status|                            |

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

Could also mention SMPTE ST380 (http://ieeexplore.ieee.org/document/7291569/)
as another example.

::: gst-libs/gst/tag/klv.c
@@ +146,3 @@
+  meta = (GstKLVMeta *) gst_buffer_add_meta (buffer, GST_KLV_META_INFO, NULL);
+
+  GST_TRACE ("Adding %u bytes of KLV data to buffer %p", (guint) size,
buffer);

Might want to also print the type here?

@@ +162,3 @@
+ *
+ * Attaches #GstKLVMeta metadata to @buffer with the given parameters,
+ * Does not take ownership of @data.

Should it contain exactly one KLV item, or one or more?

Should there be API to get the K/L/V directly from a buffer (or in case of "one
or more" an iterator over the data)?

::: gst-libs/gst/tag/klv.h
@@ +36,3 @@
+  /*< private >*/
+  GstMeta meta;
+} GstKLVMeta;

Why have the struct definition in the header if it's opaque anyway?

@@ +56,3 @@
+
+GST_EXPORT
+GstKLVMeta        * gst_buffer_add_klv_meta_take_data (GstBuffer * buffer,
guint8 * data, gsize size);

And maybe a take_data_full() that takes a user_data and GDestroyNotify?

@@ +67,3 @@
+
+GST_EXPORT
+GstKLVMeta        * gst_buffer_get_klv_meta (GstBuffer * buffer);

Can there only be one per buffer?

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