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] Support putFragmentMetadata from kvssink #1122

Merged
merged 27 commits into from
May 16, 2024

Conversation

niyatim23
Copy link
Contributor

@niyatim23 niyatim23 commented Jan 2, 2024

Issue #, if available:
#1031

What was changed?
Modify kvssink and sample application to support adding metadata per fragment

Why was it changed?
This feature was missing from kvssink and was requested by multiple customers

How was it changed?

  • This feature allows adding multiple metadata tags to fragments until a limit (10 metadata per fragment) is reached. To repeatedly put the metadata while streaming, a timer has been used which triggers a function put-metadata every 2 seconds. This application-level logic can be customized by the customers to be fired on their own set of events / frequency. put-metadata can be modified by the customers to generate metadata tags to be put. Currently, they consume values from a global instance of CustomData whose counter is updated within put-metadata itself. put-metadata also calls put-fragment-metadata
  • put-fragment-metadata generates the GstStructure expected by the GST_EVENT_CUSTOM_DOWNSTREAM and ultimately triggers the GST_EVENT_CUSTOM_DOWNSTREAM that handles the tags and calls the underlying KVS API putFragmentMetadata.
  • When this event is triggered, it checks whether the structure is in the expected format. If it has, it warns the customer about it through the logs without disrupting the streaming.

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

  • Tested that the metadata was persisted in the video by checking the video dump with mkvinfo
  • Tested that the metadata marked as persistent is indeed persisted for each fragment of the video
  • Tested that the metadata marked as non-persistent is not persisted for each fragment of the video
  • Tested that the metadata that is to be put, toggles between persistent and non-persistent and the both of them are marked correctly in the video
  • Tested that the streaming continues despite failures in the putFragmentMetadata
  • Tested that sending empty values for persistent metadata removes it
  • mkvinfo on a video dump from the tests (even metadata-keys are persistent, odd ones are non-persistent): mkvinfo-metadata.txt

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

@codecov-commenter
Copy link

codecov-commenter commented Jan 2, 2024

Codecov Report

Attention: Patch coverage is 0% with 58 lines in your changes are missing coverage. Please review.

Project coverage is 16.27%. Comparing base (eeb392f) to head (db6d18f).
Report is 9 commits behind head on develop.

❗ Current head db6d18f differs from pull request most recent head 47d702d. Consider uploading reports for the commit 47d702d to get more accurate results

Files Patch % Lines
samples/kvssink_gstreamer_sample.cpp 0.00% 50 Missing ⚠️
src/gstreamer/gstkvssink.cpp 0.00% 8 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1122      +/-   ##
===========================================
- Coverage    16.48%   16.27%   -0.22%     
===========================================
  Files           50       50              
  Lines         7019     7092      +73     
===========================================
- Hits          1157     1154       -3     
- Misses        5862     5938      +76     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@niyatim23 niyatim23 marked this pull request as ready for review January 4, 2024 12:43
src/gstreamer/gstkvssink.cpp Outdated Show resolved Hide resolved
src/gstreamer/gstkvssink.cpp Outdated Show resolved Hide resolved
@sirknightj sirknightj added the gstreamer Changes to kvssink or the kvssink samples label Feb 21, 2024
@disa6302 disa6302 mentioned this pull request Mar 1, 2024
@niyatim23 niyatim23 force-pushed the kvssink-fragment-metadata branch from 1a21a82 to 9a957ca Compare March 1, 2024 23:00
@sirknightj sirknightj added the Samples Related to the sample applications label Mar 4, 2024
samples/kvssink_gstreamer_sample.cpp Show resolved Hide resolved
samples/kvssink_gstreamer_sample.cpp Outdated Show resolved Hide resolved
samples/kvssink_gstreamer_sample.cpp Outdated Show resolved Hide resolved
samples/kvssink_gstreamer_sample.cpp Outdated Show resolved Hide resolved
samples/kvssink_gstreamer_sample.cpp Show resolved Hide resolved
samples/kvssink_gstreamer_sample.cpp Outdated Show resolved Hide resolved
src/gstreamer/gstkvssink.cpp Outdated Show resolved Hide resolved
src/gstreamer/gstkvssink.h Outdated Show resolved Hide resolved
@niyatim23 niyatim23 force-pushed the kvssink-fragment-metadata branch from fe817db to 9267a1a Compare March 5, 2024 23:56
@niyatim23 niyatim23 force-pushed the kvssink-fragment-metadata branch from 9267a1a to 998de7b Compare May 7, 2024 16:56
samples/kvssink_gstreamer_sample.cpp Show resolved Hide resolved
src/gstreamer/gstkvssink.h Outdated Show resolved Hide resolved
hassanctech
hassanctech previously approved these changes May 14, 2024
Copy link
Contributor

@hassanctech hassanctech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

sirknightj
sirknightj previously approved these changes May 16, 2024
@niyatim23 niyatim23 dismissed stale reviews from sirknightj and hassanctech via 577ce90 May 16, 2024 15:58
Copy link
Contributor

@hassanctech hassanctech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@niyatim23 niyatim23 merged commit 0b5c45b into develop May 16, 2024
16 checks passed
@niyatim23 niyatim23 deleted the kvssink-fragment-metadata branch May 16, 2024 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gstreamer Changes to kvssink or the kvssink samples Samples Related to the sample applications
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants