Feature/rdkemw 17882#499
Open
balasaraswathy-n wants to merge 4 commits into
Open
Conversation
|
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 adds an end-to-end “first audio frame received” notification, detected in the GStreamer layer (signal when available, otherwise sink-pad probe), propagated through the server internals, IPC/protobuf, and up to the public client callback, with unit + component test coverage and accompanying OpenSpec artifacts.
Changes:
- Add
notifyFirstFrameReceivedcallback across public API, server/client IPC, and gstplayer client interfaces + protobuf eventFirstFrameReceivedEvent. - Detect first-audio-frame via GLib signal lookup (
first-audio-frame/first-audio-frame-callback) with fallback sink pad probe; schedule a worker task (FirstFrameReceived) with de-duplication. - Add unit/component tests and CMake integration for the new behavior.
Reviewed changes
Copilot reviewed 68 out of 68 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unittests/media/server/mocks/gstplayer/GstGenericPlayerPrivateMock.h | Adds mock for scheduleAudioFirstFrame() scheduling hook. |
| tests/unittests/media/server/mocks/gstplayer/GstGenericPlayerClientMock.h | Adds mock for notifyFirstFrameReceived(MediaSourceType). |
| tests/unittests/media/server/mocks/gstplayer/GenericPlayerTaskFactoryMock.h | Adds mock factory method for FirstFrameReceived task. |
| tests/unittests/media/server/main/mediaPipeline/CallbackTest.cpp | Adds unit tests for forwarding first-frame notification and failure when source mapping missing. |
| tests/unittests/media/server/ipc/mediaPipelineModuleService/MediaPipelineModuleServiceTestsFixture.h | Adds fixture helpers for first-frame event. |
| tests/unittests/media/server/ipc/mediaPipelineModuleService/MediaPipelineModuleServiceTestsFixture.cpp | Adds matcher + helper for first-frame event (but currently contains a duplicate definition). |
| tests/unittests/media/server/ipc/mediaPipelineModuleService/MediaPipelineModuleServiceTests.cpp | Adds test ensuring FirstFrameReceivedEvent is sent. |
| tests/unittests/media/server/gstplayer/UtilsTest.cpp | New tests for getFirstAudioFrameSignalName(). |
| tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/SetupElementTest.cpp | Adds tests for signal hookup and probe fallback paths. |
| tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/GenericPlayerTaskFactoryTest.cpp | Adds test for creating FirstFrameReceived task. |
| tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/FirstFrameReceivedTest.cpp | New tests for first-frame task behavior + de-duplication. |
| tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerPrivateTest.cpp | Adds test for scheduleAudioFirstFrame() enqueue behavior. |
| tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsContext.h | Extends test context to store first-frame callback/probe plumbing. |
| tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.h | Adds test helper declarations for first-frame signal/probe + task. |
| tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.cpp | Implements helpers for first-frame signal/probe tests + task trigger. |
| tests/unittests/media/server/gstplayer/CMakeLists.txt | Adds new gstplayer unit test sources. |
| tests/unittests/media/client/mocks/ipc/MediaPipelineIpcClientMock.h | Adds mock for client-side IPC first-frame callback. |
| tests/unittests/media/client/ipc/mediaPipelineIpc/CallbackTest.cpp | Adds tests for IPC callback dispatch + invalid session ignore. |
| tests/unittests/media/client/ipc/mediaPipelineIpc/base/MediaPipelineIpcTestBase.h | Adds event tag + callback storage for FirstFrameReceivedEvent. |
| tests/unittests/media/client/ipc/mediaPipelineIpc/base/MediaPipelineIpcTestBase.cpp | Subscribes/unsubscribes FirstFrameReceivedEvent in tests. |
| tests/componenttests/server/tests/mediaPipeline/FirstFrameReceivedTest.cpp | New server component test asserting FirstFrameReceivedEvent propagation. |
| tests/componenttests/server/tests/CMakeLists.txt | Adds server component test source. |
| tests/componenttests/server/stubs/ClientStub.cpp | Subscribes client stub to FirstFrameReceivedEvent. |
| tests/componenttests/client/tests/mse/FirstFrameReceivedNotificationTest.cpp | New client component test verifying public notification. |
| tests/componenttests/client/tests/base/MediaPipelineTestMethods.h | Adds client component test helpers for first-frame. |
| tests/componenttests/client/tests/base/MediaPipelineTestMethods.cpp | Implements first-frame expectations + stub event send. |
| tests/componenttests/client/stubs/MediaPipelineModuleStub.h | Adds stub method to emit FirstFrameReceivedEvent. |
| tests/componenttests/client/stubs/MediaPipelineModuleStub.cpp | Implements sending FirstFrameReceivedEvent from stub. |
| tests/componenttests/client/CMakeLists.txt | Adds client component test source. |
| tests/common/publicClientMocks/MediaPipelineClientMock.h | Adds public client mock method notifyFirstFrameReceived. |
| proto/mediapipelinemodule.proto | Adds FirstFrameReceivedEvent protobuf message. |
| openspec/project.md | Adds OpenSpec project constitution doc. |
| openspec/config.yaml | Adds OpenSpec configuration referencing project rules file. |
| openspec/changes/add-audio-first-frame-signal/tasks.md | Adds task breakdown for the feature implementation. |
| openspec/changes/add-audio-first-frame-signal/specs/audio-first-frame-signal/spec.md | Adds detailed spec requirements for detection/scheduling/propagation. |
| openspec/changes/add-audio-first-frame-signal/proposal.md | Adds proposal/overview for the change. |
| openspec/changes/add-audio-first-frame-signal/design.md | Adds design doc describing architecture + decisions. |
| openspec/changes/add-audio-first-frame-signal/.openspec.yaml | Adds change metadata for OpenSpec workflow. |
| media/server/main/source/MediaPipelineServerInternal.cpp | Implements server-internal mapping from MediaSourceType to sourceId + main-thread enqueue. |
| media/server/main/include/MediaPipelineServerInternal.h | Declares notifyFirstFrameReceived(MediaSourceType) on server internal. |
| media/server/ipc/source/MediaPipelineClient.cpp | Sends FirstFrameReceivedEvent over IPC. |
| media/server/ipc/include/MediaPipelineClient.h | Declares notifyFirstFrameReceived(int32_t). |
| media/server/gstplayer/source/Utils.cpp | Adds getFirstAudioFrameSignalName() signal discovery helper. |
| media/server/gstplayer/source/tasks/generic/SetupElement.cpp | Hooks first-frame signal or installs fallback pad probe on audio sink setup. |
| media/server/gstplayer/source/tasks/generic/GenericPlayerTaskFactory.cpp | Adds factory creation for FirstFrameReceived task. |
| media/server/gstplayer/source/tasks/generic/FirstFrameReceived.cpp | Implements de-duped worker task that notifies gstplayer client. |
| media/server/gstplayer/source/GstGenericPlayer.cpp | Enqueues FirstFrameReceived task via scheduleAudioFirstFrame(). |
| media/server/gstplayer/interface/IGstGenericPlayerClient.h | Extends interface with notifyFirstFrameReceived(MediaSourceType). |
| media/server/gstplayer/include/Utils.h | Declares getFirstAudioFrameSignalName(). |
| media/server/gstplayer/include/tasks/IGenericPlayerTaskFactory.h | Adds createFirstFrameReceived() to task factory interface. |
| media/server/gstplayer/include/tasks/generic/GenericPlayerTaskFactory.h | Declares createFirstFrameReceived() implementation. |
| media/server/gstplayer/include/tasks/generic/FirstFrameReceived.h | Declares new FirstFrameReceived task. |
| media/server/gstplayer/include/IGstSrc.h | Adds firstAudioFrameReceived de-dupe flag to StreamInfo. |
| media/server/gstplayer/include/IGstGenericPlayerPrivate.h | Adds scheduleAudioFirstFrame() scheduling API. |
| media/server/gstplayer/include/GstGenericPlayer.h | Declares scheduleAudioFirstFrame() implementation. |
| media/server/gstplayer/CMakeLists.txt | Adds FirstFrameReceived.cpp to gstplayer build target. |
| media/public/include/IMediaPipelineClient.h | Adds new public callback notifyFirstFrameReceived(int32_t). |
| media/client/ipc/source/MediaPipelineIpc.cpp | Subscribes and dispatches FirstFrameReceivedEvent to IPC client. |
| media/client/ipc/interface/IMediaPipelineIpcClient.h | Adds IPC client callback notifyFirstFrameReceived(int32_t). |
| media/client/ipc/include/MediaPipelineIpc.h | Declares onFirstFrameReceived() handler. |
| .github/skills/openspec-propose/SKILL.md | Adds OpenSpec “propose change” skill documentation. |
| .github/skills/openspec-explore/SKILL.md | Adds OpenSpec “explore mode” skill documentation. |
| .github/skills/openspec-archive-change/SKILL.md | Adds OpenSpec “archive change” skill documentation. |
| .github/skills/openspec-apply-change/SKILL.md | Adds OpenSpec “apply/implement change” skill documentation. |
| .github/prompts/opsx-propose.prompt.md | Adds prompt doc for /opsx:propose. |
| .github/prompts/opsx-explore.prompt.md | Adds prompt doc for /opsx:explore. |
| .github/prompts/opsx-archive.prompt.md | Adds prompt doc for /opsx:archive. |
| .github/prompts/opsx-apply.prompt.md | Adds prompt doc for /opsx:apply. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+898
to
+901
| void MediaPipelineModuleServiceTests::mediaClientWillSendFirstFrameReceivedEvent() | ||
| { | ||
| EXPECT_CALL(*m_clientMock, sendEvent(FirstFrameReceivedEventMatcher(kSourceId))); | ||
| } |
Comment on lines
+304
to
+312
| if (isAudioSink(*m_gstWrapper, m_element)) | ||
| { | ||
| std::optional<std::string> firstFrameSignalName = getFirstAudioFrameSignalName(*m_glibWrapper, m_element); | ||
| if (firstFrameSignalName) | ||
| { | ||
| 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(), | ||
| G_CALLBACK(firstAudioFrameCallback), &m_player); |
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.
Changes to include audio-first-frame based on Spec driven development