Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Copy changes #1015

Conversation

richardgavel-ordinaryexperts

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@hassanctech
Copy link
Contributor

@richardgavel-ordinaryexperts Thank you for your contribution!

I have some concerns regarding the design. The metadata tagging is a fragment level feature. If it is exposed as a GStreamer element property now this is something that is set at the start and we make the call after calling start() on the stream so it happens only one time.

The PIC level API though it is designed to be called at any time during active streaming. Since each fragment can have a unique tag, I think a better way to express this in GStreamer is as a custom event that the application puts on the GStreamer bus which kvssink then knows how to handle and it makes the call to the put event metadata API.

@richardgavel-ordinaryexperts
Copy link
Author

The point of this change is to support adding a set of persistent metadata on the stream when making use of the the gst-launch-1.0 app, not necessarily when using the gstreamer libraries directly. This seemed to make sense. The only other option I could see is writing a whole new gstreamer pipeline plugin that can drop custom events based on pipeline params.

@hassanctech
Copy link
Contributor

You have a valid point there it would be useful to customers to support something like this from the command line. I'm thinking this can live in conjunction with a custom event (that the client application places on the pipeline bus) which kvssink then reacts to and calls this API on the "current" fragment. This would allow finer grained control to clients who wish to have such control. Let me consider your implementation a bit more carefully and get back.

@disa6302
Copy link
Contributor

disa6302 commented Mar 1, 2024

Closing this in favor of #1122

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants