Add support for 'first-audio-frame-callback' signal#552
Open
Koky2701 wants to merge 1 commit into
Open
Conversation
Koky2701
commented
Jun 30, 2026
Koky2701
commented
Contributor
|
Pull request must be merged with a description containing the required fields, Summary: If there is no jira releated to this change, please put 'Jira: NO-JIRA'. Description can be changed by editing the top comment on your pull request and making a new commit. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends Rialto’s existing “first video frame received” notification pipeline to also support the first audio frame, including support for the first-audio-frame-callback signal and a fallback sink-pad probe when the callback signal is unavailable, while reusing the existing FirstFrameReceivedEvent / notifyFirstFrameReceived(...) contract end-to-end.
Changes:
- Add audio first-frame detection during GStreamer element setup (signal hookup + sink fallback probe) and schedule the existing first-frame task path using
MediaSourceType::AUDIO. - Add lifecycle cleanup/reset for audio first-frame state (attach/reattach/flush/stop/term).
- Add/update unit + component tests and add spec-driven documentation (openspec change).
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unittests/media/server/mocks/gstplayer/GstGenericPlayerPrivateMock.h | Extends private-player mock with audio first-frame scheduling/probe APIs. |
| tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/SetupElementTest.cpp | Adds UT for first-audio-frame signal hookup behavior. |
| tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/FirstFrameReceivedTest.cpp | Adds UT coverage for notifying first frame received for audio. |
| tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerPrivateTest.cpp | Adds UT for scheduling first audio frame received task. |
| tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsContext.h | Adds audio first-frame callback storage for UT harness. |
| tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.h | Adds UT helpers for audio first-frame signal/trigger paths. |
| tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.cpp | Implements UT helper expectations and triggers for audio first-frame. |
| tests/componenttests/server/tests/mediaPipeline/FirstFrameNotificationTest.cpp | Adds server CT covering end-to-end audio first-frame notification. |
| tests/componenttests/client/tests/mse/FirstFrameNotificationTest.cpp | Extends client CT to validate both video and audio first-frame notifications. |
| tests/componenttests/client/tests/base/MediaPipelineTestMethods.h | Adds client CT helper declarations for audio first-frame notification. |
| tests/componenttests/client/tests/base/MediaPipelineTestMethods.cpp | Implements client CT helpers to expect/send audio first-frame events. |
| openspec/changes/audio-first-frame/tasks.md | Adds implementation/validation task checklist for the change. |
| openspec/changes/audio-first-frame/specs/audio-first-frame/spec.md | Adds formal requirements for audio first-frame detection + one-shot guard. |
| openspec/changes/audio-first-frame/proposal.md | Documents motivation and scope of the audio first-frame change. |
| openspec/changes/audio-first-frame/design.md | Documents detection strategy, fallback probing, and one-shot emission design. |
| openspec/changes/audio-first-frame/.openspec.yaml | Adds openspec metadata for the new change package. |
| media/server/gstplayer/source/Utils.cpp | Extends first-frame signal name detection set to include audio signals. |
| media/server/gstplayer/source/tasks/generic/Stop.cpp | Resets audio first-frame state and clears fallback probe on stop. |
| media/server/gstplayer/source/tasks/generic/SetupElement.cpp | Wires first-audio-frame signal callback + installs sink fallback probe when needed. |
| media/server/gstplayer/source/tasks/generic/AttachSource.cpp | Resets audio first-frame state on (re)attach. |
| media/server/gstplayer/source/GstGenericPlayer.cpp | Implements scheduling guard + probe state management/cleanup for audio first-frame. |
| media/server/gstplayer/include/IGstGenericPlayerPrivate.h | Extends private interface for audio first-frame scheduling and probe lifecycle control. |
| media/server/gstplayer/include/GstGenericPlayer.h | Declares new private-interface methods on the concrete player. |
| media/server/gstplayer/include/GenericPlayerContext.h | Adds audio first-frame one-shot flag + fallback probe state to context. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+69
to
+72
| /** | ||
| * @brief Schedules first audio frame received task. Called by the worker thread. | ||
| */ | ||
| virtual void scheduleFirstAudioFrameReceived() = 0; |
Comment on lines
100
to
105
| GCallback m_audioUnderflowCallback; | ||
| GCallback m_videoUnderflowCallback; | ||
| GCallback m_firstVideoFrameCallback; | ||
| GCallback m_firstAudioFrameCallback; | ||
| GCallback m_childAddedCallback; | ||
| GCallback m_childRemovedCallback; |
Comment on lines
205
to
+208
| GCallback m_firstVideoFrameCallback; | ||
| gpointer m_firstVideoFrameData{nullptr}; | ||
| GCallback m_firstFrameCallback; | ||
| gpointer m_firstFrameData{nullptr}; |
Comment on lines
+331
to
+335
| else if (isAudio(*m_gstWrapper, m_element)) | ||
| { | ||
| RIALTO_SERVER_LOG_INFO("Connecting first audio frame callback for signal: %s", | ||
| firstFrameSignalName.value().c_str()); | ||
| m_glibWrapper->gSignalConnect(m_element, firstFrameSignalName.value().c_str(), |
Koky2701
force-pushed
the
feature/RDKEMW-17882_v2
branch
from
fe8574e to
11dc49b
Compare
Koky2701
force-pushed
the
feature/RDKEMW-17882_v2
branch
from
11dc49b to
a4be17b
Compare
Comment on lines
+147
to
+157
| EXPECT_CALL(*m_gstWrapperMock, gstElementFactoryListIsType(m_elementFactory, GST_ELEMENT_FACTORY_TYPE_DECODER)) | ||
| .WillOnce(Return(TRUE)); | ||
| EXPECT_CALL(*m_gstWrapperMock, | ||
| gstElementFactoryListIsType(m_elementFactory, GST_ELEMENT_FACTORY_TYPE_MEDIA_VIDEO)) | ||
| .WillOnce(Return(FALSE)); | ||
| EXPECT_CALL(*m_gstWrapperMock, | ||
| gstElementFactoryListIsType(m_elementFactory, | ||
| GST_ELEMENT_FACTORY_TYPE_DECODER | | ||
| GST_ELEMENT_FACTORY_TYPE_MEDIA_AUDIO)) | ||
| .WillOnce(Return(TRUE)) | ||
| .RetiresOnSaturation(); |
Comment on lines
+69
to
+72
| /** | ||
| * @brief Schedules first audio frame received task. Called by the worker thread. | ||
| */ | ||
| virtual void scheduleFirstAudioFrameReceived() = 0; |
Koky2701
force-pushed
the
feature/RDKEMW-17882_v2
branch
from
a4be17b to
f276929
Compare
|
tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.cpp:456:0: style: The function 'expectFirstAudioFrameSignalConnection' is never used. [unusedFunction] |
Koky2701
force-pushed
the
feature/RDKEMW-17882_v2
branch
from
f276929 to
3dc70f5
Compare
Comment on lines
31
to
33
| const char *underflowSignals[]{"buffer-underflow-callback", "vidsink-underflow-callback", "underrun-callback"}; | ||
| const char *firstFrameSignals[]{"first-video-frame-callback"}; | ||
| const char *firstFrameSignals[]{"first-video-frame-callback", "first-audio-frame", "first-audio-frame-callback"}; | ||
|
|
Comment on lines
+1735
to
+1747
| void GstGenericPlayer::scheduleFirstAudioFrameReceived() | ||
| { | ||
| if (m_context.firstAudioFrameReceived) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| m_context.firstAudioFrameReceived = true; | ||
|
|
||
| if (m_workerThread) | ||
| { | ||
| m_workerThread->enqueueTask(m_taskFactory->createFirstFrameReceived(m_context, *this, MediaSourceType::AUDIO)); | ||
| } |
Koky2701
force-pushed
the
feature/RDKEMW-17882_v2
branch
from
3dc70f5 to
5bef55c
Compare
|
tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.cpp:795:0: style: The function 'shouldSetupAudioSinkElementWithPendingLowLatency' is never used. [unusedFunction] |
Koky2701
force-pushed
the
feature/RDKEMW-17882_v2
branch
from
5bef55c to
e015fde
Compare
Contributor
Comment on lines
+1750
to
+1756
| void GstGenericPlayer::setAudioFirstFrameFallbackProbe(GstPad *pad, gulong id) | ||
| { | ||
| clearAudioFirstFrameFallbackProbe(); | ||
|
|
||
| m_context.audioFirstFrameProbePad = pad; | ||
| m_context.audioFirstFrameProbeId = id; | ||
| } |
Comment on lines
+1758
to
+1777
| void GstGenericPlayer::clearAudioFirstFrameFallbackProbe() | ||
| { | ||
| if (m_context.audioFirstFrameProbePad && m_context.audioFirstFrameProbeId != 0) | ||
| { | ||
| m_gstWrapper->gstPadRemoveProbe(m_context.audioFirstFrameProbePad, m_context.audioFirstFrameProbeId); | ||
| } | ||
|
|
||
| clearAudioFirstFrameFallbackProbeState(); | ||
| } | ||
|
|
||
| void GstGenericPlayer::clearAudioFirstFrameFallbackProbeState() | ||
| { | ||
| if (m_context.audioFirstFrameProbePad) | ||
| { | ||
| m_gstWrapper->gstObjectUnref(m_context.audioFirstFrameProbePad); | ||
| m_context.audioFirstFrameProbePad = nullptr; | ||
| } | ||
|
|
||
| m_context.audioFirstFrameProbeId = 0; | ||
| } |
Comment on lines
+1735
to
+1743
| void GstGenericPlayer::scheduleFirstAudioFrameReceived() | ||
| { | ||
| if (m_context.firstAudioFrameReceived) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| m_context.firstAudioFrameReceived = true; | ||
|
|
| } | ||
|
|
||
| void GenericTasksTestsBase::expectAudioUnderflowSignalConnection() | ||
| void expectAudioUnderflowAndFirstFrameSignalConnection(const char *signalName) |
Koky2701
force-pushed
the
feature/RDKEMW-17882_v2
branch
from
e015fde to
ba877b1
Compare
Summary: Add support for 'first-audio-frame-callback' signal to Rialto sink
Type: Fix
Test Plan: UT/CT, Fullstack
Jira: RDKEMW-17882
Koky2701
force-pushed
the
feature/RDKEMW-17882_v2
branch
from
ba877b1 to
a9c8627
Compare
Comment on lines
+1735
to
+1739
| void GstGenericPlayer::scheduleFirstAudioFrameReceived() | ||
| { | ||
| if (m_context.firstAudioFrameReceived) | ||
| { | ||
| return; |
Comment on lines
+1750
to
+1756
| void GstGenericPlayer::setAudioFirstFrameFallbackProbe(GstPad *pad, gulong id) | ||
| { | ||
| clearAudioFirstFrameFallbackProbe(); | ||
|
|
||
| m_context.audioFirstFrameProbePad = pad; | ||
| m_context.audioFirstFrameProbeId = id; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.