Fix reattaching audio source at dynamic audio codec change. (#530)#540
Merged
Conversation
Summary: Fix reattaching audio source at dynamic audio codec change. Type: Fix Test Plan: UT/CT, Fullstack Jira: RDKEMW-17771 --------- Co-authored-by: Marcin Wojciechowski <marcin.wojciechowski@sky.uk> Co-authored-by: Sasa Mudri <Sasa_Mudri@comcast.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
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 fixes audio-source reattachment behavior when the audio codec changes dynamically by introducing explicit audio-source removal handling in the GStreamer player and ensuring follow-on tasks (NeedData/Underflow) respect the “audio source removed” state.
Changes:
- Add
IGstGenericPlayer::removeSource()plus a newRemoveSourceworker-thread task and task-factory plumbing. - Track
audioSourceRemovedinGenericPlayerContextand use it to suppress audio NeedData requests and disable audio-underflow notifications while removed. - Extend unit tests to cover remove/reattach flows and the new task-factory behavior.
Reviewed changes
Copilot reviewed 27 out of 27 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 | Minor mock header cleanup. |
| tests/unittests/media/server/mocks/gstplayer/GstGenericPlayerMock.h | Add mock for removeSource. |
| tests/unittests/media/server/mocks/gstplayer/GenericPlayerTaskFactoryMock.h | Add mock for createRemoveSource. |
| tests/unittests/media/server/main/mediaPipeline/SourceTest.cpp | Update pipeline remove-source unit tests to expect gstPlayer->removeSource. |
| tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/RemoveSourceTest.cpp | New unit tests for RemoveSource task behavior. |
| tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/NeedDataTest.cpp | Add test ensuring NeedData doesn’t notify client when audio is removed. |
| tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/GenericPlayerTaskFactoryTest.cpp | Add factory test for creating RemoveSource task. |
| tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/AttachSourceTest.cpp | Adjust reattach tests to incorporate removed-state and new expectations. |
| tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerTest.cpp | Add unit test for GstGenericPlayer::removeSource(). |
| tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerPrivateTest.cpp | Update/extend underflow scheduling tests for removed-state. |
| tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.h | Add test helpers for removed-state and remove-source task. |
| tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.cpp | Implement new test helpers and task triggers. |
| tests/unittests/media/server/gstplayer/CMakeLists.txt | Register new RemoveSourceTest.cpp. |
| media/server/main/source/MediaPipelineServerInternal.cpp | Invoke m_gstPlayer->removeSource(type) and clean per-type maps on removal. |
| media/server/gstplayer/source/tasks/generic/RemoveSource.cpp | New task to clear audio buffers/state and mark audio as removed. |
| media/server/gstplayer/source/tasks/generic/NeedData.cpp | Skip audio client NeedData notifications when audio is removed. |
| media/server/gstplayer/source/tasks/generic/GenericPlayerTaskFactory.cpp | Add factory method to create RemoveSource task. |
| media/server/gstplayer/source/tasks/generic/CheckAudioUnderflow.cpp | Disable audio underflow when audio is removed. |
| media/server/gstplayer/source/tasks/generic/AttachSource.cpp | On audio reattach: request audio data and clear removed-state. |
| media/server/gstplayer/source/GstGenericPlayer.cpp | Add removeSource() method and gate audio-underflow scheduling on removed-state. |
| media/server/gstplayer/interface/IGstGenericPlayer.h | Add removeSource API to interface. |
| media/server/gstplayer/include/tasks/IGenericPlayerTaskFactory.h | Add createRemoveSource API. |
| media/server/gstplayer/include/tasks/generic/RemoveSource.h | New task header. |
| media/server/gstplayer/include/tasks/generic/GenericPlayerTaskFactory.h | Declare createRemoveSource override. |
| media/server/gstplayer/include/GstGenericPlayer.h | Declare removeSource override. |
| media/server/gstplayer/include/GenericPlayerContext.h | Add audioSourceRemoved flag to context. |
| media/server/gstplayer/CMakeLists.txt | Build new RemoveSource.cpp into gstplayer library. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+48
to
+52
| m_context.audioSourceRemoved = true; | ||
| if (m_gstPlayerClient) | ||
| { | ||
| m_gstPlayerClient->invalidateActiveRequests(m_type); | ||
| } |
Comment on lines
+53
to
+58
| auto sourceElem = m_context.streamInfo.find(m_type); | ||
| if (sourceElem == m_context.streamInfo.end()) | ||
| { | ||
| RIALTO_SERVER_LOG_WARN("Failed to remove source - streamInfo not found"); | ||
| return; | ||
| } |
Comment on lines
1627
to
1633
| void GstGenericPlayer::scheduleAudioUnderflow() | ||
| { | ||
| if (m_workerThread) | ||
| { | ||
| bool underflowEnabled = m_context.isPlaying; | ||
| bool underflowEnabled = m_context.isPlaying && !m_context.audioSourceRemoved; | ||
| m_workerThread->enqueueTask( | ||
| m_taskFactory->createUnderflow(m_context, *this, underflowEnabled, MediaSourceType::AUDIO)); |
Comment on lines
+209
to
+214
| /** | ||
| * @brief Flag used to check, if audio source has been recently removed | ||
| * | ||
| * Flag can be used only in worker thread | ||
| */ | ||
| bool audioSourceRemoved{false}; |
|
Coverage statistics of your commit: |
balasaraswathy-n
approved these changes
Jun 22, 2026
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Summary: Fix reattaching audio source at dynamic audio codec change.
Type: Fix
Test Plan: UT/CT, Fullstack
Jira: RDKEMW-17771