After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 791533 - Improve support for KLV metadata in tsdemuxer and tsmuxer
Improve support for KLV metadata in tsdemuxer and tsmuxer
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
1.12.0
Other Linux
: Normal enhancement
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-12-12 17:53 UTC by Michael Fien
Modified: 2018-11-03 14:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Mpeg2 transport stream descriptor for KLV (11.92 KB, patch)
2017-12-12 17:53 UTC, Michael Fien
none Details | Review
Add asynchronous and asynchronous KLV improvements to Mpeg transport stream muxer (10.70 KB, patch)
2017-12-12 17:54 UTC, Michael Fien
none Details | Review
Modify Mpeg2 transport stream muxer caps to bring it in alignment with demuxe (1.25 KB, patch)
2017-12-12 17:55 UTC, Michael Fien
none Details | Review
Add asynchronous and asynchronous KLV improvements to Mpeg transport stream demuxer (9.53 KB, patch)
2017-12-12 17:56 UTC, Michael Fien
none Details | Review
Test video with asynchronous klv metadata (3.26 MB, application/octet-stream)
2017-12-12 18:06 UTC, Michael Fien
  Details
Test video with synchronous klv metadata (3.03 MB, application/octet-stream)
2017-12-12 18:07 UTC, Michael Fien
  Details
klvmeta export decorator patch (1.46 KB, patch)
2018-04-26 19:51 UTC, Lucas Johnson
none Details | Review

Description Michael Fien 2017-12-12 17:53:00 UTC
Created attachment 365454 [details] [review]
Mpeg2 transport stream descriptor for KLV

Add additions to tsdemuxer and tsmuxer that handle synchronous and asynchronous metadata streams per MISB-0604
Comment 1 Michael Fien 2017-12-12 17:54:31 UTC
Created attachment 365455 [details] [review]
Add asynchronous and asynchronous KLV improvements to Mpeg transport stream muxer
Comment 2 Michael Fien 2017-12-12 17:55:36 UTC
Created attachment 365456 [details] [review]
Modify Mpeg2 transport stream muxer caps to bring it in alignment with demuxe
Comment 3 Michael Fien 2017-12-12 17:56:17 UTC
Created attachment 365457 [details] [review]
Add asynchronous and asynchronous KLV improvements to Mpeg transport stream demuxer
Comment 4 Michael Fien 2017-12-12 18:06:54 UTC
Created attachment 365459 [details]
Test video with asynchronous klv metadata
Comment 5 Michael Fien 2017-12-12 18:07:24 UTC
Created attachment 365460 [details]
Test video with synchronous klv metadata
Comment 6 Michael Fien 2017-12-12 18:08:23 UTC
Test commandline

gst-launch-1.0 filesrc location=/tmp/klv_metadata_test_async.ts ! tsdemux name=demux ! queue ! h264parse ! mpegtsmux alignment=7 name=mux ! filesink location=/tmp/test.ts demux. ! 'meta/x-klv' ! queue ! mux.
Comment 7 Tim-Philipp Müller 2017-12-12 18:18:26 UTC
Nice, that's been on my list to finish up for ages!
Comment 8 Tim-Philipp Müller 2017-12-12 18:20:05 UTC
I also have a klv meta implementation I've been meaning to merge, will see how yours compares :)
Comment 9 Michael Fien 2017-12-12 18:20:50 UTC
Technical changes are referenced in this document:

http://www.gwg.nga.mil/misb/docs/standards/ST1402.2.pdf
Comment 10 Tim-Philipp Müller 2017-12-12 18:22:44 UTC
Comment on attachment 365456 [details] [review]
Modify Mpeg2 transport stream muxer caps to bring it in alignment with demuxe

What's the rationale for this change?

I think we want to keep the requirement for parsed and framed input?
Comment 11 Michael Fien 2017-12-12 18:32:34 UTC
I had caps negotiation problems between the demuxer and muxer if these parsed/framed booleans were set. I could ultimately determine what condition needed to be met for these values to be set true, but without the change I couldn't do a demux -> mux of these types.

Is there anything I could look at to understand when/why these should be set?
Comment 12 Tim-Philipp Müller 2017-12-12 18:36:43 UTC
You will need a parser (mpegaudioparse, aacparse, etc.) between demuxer and muxer. We should probably split this part of the discussion off into a separate bug if needed, or move it onto the mailing list.
Comment 13 Michael Fien 2017-12-12 18:38:19 UTC
I also just realized that the MISB document I reference in my code comment for patch 0001-Add-MPEG2-TS-descriptor-support-for-KLV-metadata.patch is incorrect. I references document MISB 0604 but it should be MISB ST1402. Sorry!
Comment 14 Michael Fien 2017-12-12 18:44:56 UTC
I created an enhancement to add typefinder support for meta/x-klv to bug.freeedesktop.org in late October. I have gotten any feedback on it at all.

Do you suggest I recreate that here instead?
Comment 15 Tim-Philipp Müller 2017-12-12 18:50:06 UTC
No, please post a follow-up comment there, and/or add it to "See also"
Comment 16 Michael Fien 2017-12-12 19:22:57 UTC
I also found that this change needed to be in place to prevent the pipeline from block on sparse data - bug 759807.
Comment 17 Tim-Philipp Müller 2017-12-24 11:48:46 UTC
Just for comparison, I have added my KLV meta patches to bug #791918. It's slightly different than what you're doing here though, so I don't think you need to worry about that at the moment, we just have to work through your patches and see how it should be done instead. Your KLVMeta thing is something a bit different, and I suspect it should be done differently.
Comment 18 Michael Fien 2018-01-04 20:08:36 UTC
It looks like our use cases are different. The metadata you are adding consists of binary KLV. In my case the buffer is KLV and the metadata is a header for each buffer as described in MISB Standard 0604, section 6.2 for synchronous muxing of KLV in an Mpeg2 transport stream.

I didn't think about it initially but I see now, at the very least, that my meta structure and function names are too generic for this specific use case.

Do you see potentially combining these use cases into a single KLV file?

It also looks like I may have put my implementation in the wrong place. I added mine to codecparsers and yours is under tag.
Comment 19 Tim-Philipp Müller 2018-01-04 20:21:46 UTC
Yes, different use cases indeed. We can and will support both scenarios though, so there's no problem with that. At first glance your klv meta looked like it might be specific to the mpeg-ts mapping, in which case it should probably be done differently. I need to read up on the specs again first to make a more useful suggestion here though.
Comment 20 Michael Fien 2018-01-05 00:03:12 UTC
Yes, it is specific to ts mapping. The MISB 0604 standard describes synchronous KLV muxing but the access unit header specific details are actually contained in MISP 1402.2, section 9.4.1. These header values are what my meta is for.
Comment 21 Lucas Johnson 2018-03-09 20:57:19 UTC
We maintain a change that is different, but also parses synchronous metadata.  Can I help with this change.  I would love not to maintain this on our end.
Comment 22 Lucas Johnson 2018-04-17 02:06:49 UTC
These patches don't work on 1.14.0.  I am getting undefined symbols for gst_buffer_add_klv_meta on tsdemux plugin and another one for tsmux.
Comment 23 Lucas Johnson 2018-04-26 19:51:09 UTC
Created attachment 371446 [details] [review]
klvmeta export decorator patch

Turns out there are some required export decorators needed to work with 1.14.0/master.  There are some other minor changes required to get the already attached patches working as well, which I can upload if anyone wants it.
Comment 24 GStreamer system administrator 2018-11-03 14:16:48 UTC
-- 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-bad/issues/642.