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

[FEATURE] Kvssink deliver-images with putEventMetadata #1176

Open
wants to merge 25 commits into
base: develop-pre-3.4.1
Choose a base branch
from

Conversation

niyatim23
Copy link
Contributor

@niyatim23 niyatim23 commented May 12, 2024

Issue #, if available:

Description of changes:

  • Introduced a new prop / attribute in kvssink which allows enabling / disabling image-generation
  • Surfaced a public API- putEventMetadata from the C layer to the CPP layer

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

Issue #, if available:

What was changed?

  • Introduced a new prop / attribute in kvssink which allows enabling / disabling image-generation

Why was it changed?

  • To maintain feature parity between producer c, cpp and kvssink

How was it changed?

  • Surfaced a public API- putEventMetadata from the C layer to the CPP layer.
  • It is invoked from gst_kvs_sink_handle_buffer for every key frame if the generate_images bool is set

What testing was done for the changes?
Tested the following locally:

  • Tested that the image tags were added to the video with mkvinfo when generate-images was set to TRUE
  • Tested that setting generate-images to FALSE doesn't generate image tags in the video

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

@niyatim23 niyatim23 added enhancement gstreamer Changes to kvssink or the kvssink samples labels May 12, 2024
@niyatim23 niyatim23 marked this pull request as ready for review May 13, 2024 16:48
@sirknightj
Copy link
Contributor

As discussed in the other PR, please adjust to every N keyframes to make this feature more flexible.

Also, please edit PR title, description, GStreamer parameter description, etc to clearly mark that this feature is explicitly for s3 image delivery. The s3 delivery mkv simple tag is not needed for image generation through GetImages, and thus, "image generation" is already supported in its current state.

For completeness, can you also share an outline of the procedure you used to verify the tag was added and a screenshot of some media with the tag.

@hassanctech
Copy link
Contributor

Discussed offline waiting for a change here to trigger a call to the put event metadata API based on receiving GST_EVENT_CUSTOM_DOWNSTREAM similar to the put fragment metadata API.

@sirknightj
Copy link
Contributor

I will push for keeping consistent with the docs and calling this image delivery to avoid confusion with the other feature getimages https://docs.aws.amazon.com/kinesisvideostreams/latest/dg/gs-getImages.html

Also, please update the readme as there is no information about how to use it.

@niyatim23 niyatim23 changed the title [FEATURE] Kvssink image-generation [FEATURE] Kvssink deliver images with putEventMetadata Jul 2, 2024
@niyatim23 niyatim23 changed the title [FEATURE] Kvssink deliver images with putEventMetadata [FEATURE] Kvssink deliver-images with putEventMetadata Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement gstreamer Changes to kvssink or the kvssink samples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants