GNOME Bugzilla – Bug 791918
Add KLV meta and add support for it in matroska mux/demux
Last modified: 2018-11-03 12:02:32 UTC
Created attachment 365925 [details] [review] buffer: add gst_buffer_get_n_meta() convenience function (core) This adds a GstKLVMeta to libgsttag, which allows attaching arbitrary metadata in KLV format to buffers. Plus patches to read/write those to/from Matroska as BLOCKADDITIONALs. And an extra patch to v4l2src for testing which is not for merging.
Created attachment 365926 [details] [review] tag: add GstMeta for KLV metadata Can be used to attach arbitrary metadata to buffers, such as timestamps, GPS data, etc. See ITU Recommendation BT.1563-1 or SMPTE 336M for the KLV standard, and MISB EG 0902 (MISB Minimum Metadata Set) and other MISB standards and recommended practices for examples of KLV metadata local data sets for a variety of use cases.
Created attachment 365927 [details] [review] matroska-mux: write KLV metas attached to buffers as BlockAdditions
Created attachment 365928 [details] [review] matroska-demux: extract KLV metadata from BlockAdditionals
Created attachment 365929 [details] [review] TESTING: v4l2src: add KLV metadata with unix timestamp and GPS location to each frame
Review of attachment 365926 [details] [review]: 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?
Review of attachment 365927 [details] [review]: Is there a spec for this? Block additions AFAIU are codec specific, or not?
Review of attachment 365928 [details] [review]: ::: gst/matroska/matroska-demux.c @@ +3501,3 @@ + /* read all BlockMore sub-entries */ + while (ret == GST_FLOW_OK && gst_ebml_read_has_remaining (ebml, 1, TRUE)) { + Weird newline
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/issues/410.