Fix reattaching audio source at dynamic audio codec change. (#530)#537
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. |
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
There was a problem hiding this comment.
Pull request overview
This PR addresses failures when reattaching the audio source during a dynamic audio codec change by introducing explicit “remove source” handling on the GStreamer player side and by gating audio data/underflow behavior while the audio source is considered removed.
Changes:
- Add
IGstGenericPlayer::removeSource()and a newtasks::generic::RemoveSourcetask to reset audio-stream state on removal. - Prevent audio data requests and audio underflow notifications while
audioSourceRemovedis set; ensure reattach re-enables data flow. - Extend unit tests/mocks and task-factory coverage to validate the new remove/reattach behavior.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 3 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 removeSource() mock for new player API. |
| tests/unittests/media/server/mocks/gstplayer/GenericPlayerTaskFactoryMock.h | Add createRemoveSource() factory mock method. |
| tests/unittests/media/server/main/mediaPipeline/SourceTest.cpp | Assert server-side removeSource() triggers Gst player removal. |
| tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/RemoveSourceTest.cpp | New task-level tests for RemoveSource behavior. |
| tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/NeedDataTest.cpp | Add test for NeedData behavior when audio source is removed. |
| tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/GenericPlayerTaskFactoryTest.cpp | Validate factory can create RemoveSource task. |
| tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/AttachSourceTest.cpp | Update reattach tests to require “removed” state + data request. |
| tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerTest.cpp | Add test for GstGenericPlayer::removeSource() scheduling. |
| tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerPrivateTest.cpp | Update/extend underflow scheduling tests with audioSourceRemoved. |
| tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.h | Add helpers for removed-source and RemoveSource task testing. |
| tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.cpp | Implement new test helpers + trigger RemoveSource task execution. |
| tests/unittests/media/server/gstplayer/CMakeLists.txt | Register new RemoveSource unit test. |
| media/server/main/source/MediaPipelineServerInternal.cpp | Invoke Gst player removeSource(type) during pipeline source removal. |
| media/server/gstplayer/source/tasks/generic/RemoveSource.cpp | New task to reset audio-stream state/buffers/EOS info on removal. |
| media/server/gstplayer/source/tasks/generic/NeedData.cpp | Skip requesting audio data from client when audio source is removed. |
| media/server/gstplayer/source/tasks/generic/GenericPlayerTaskFactory.cpp | Add factory creation of RemoveSource task. |
| media/server/gstplayer/source/tasks/generic/CheckAudioUnderflow.cpp | Disable audio underflow handling when audio source is removed. |
| media/server/gstplayer/source/tasks/generic/AttachSource.cpp | On audio reattach, re-enable data-needed and request more audio data. |
| media/server/gstplayer/source/GstGenericPlayer.cpp | Add removeSource() and gate underflow scheduling on removed-audio flag. |
| media/server/gstplayer/interface/IGstGenericPlayer.h | Add removeSource() API (doc requires alignment with implementation). |
| media/server/gstplayer/include/tasks/IGenericPlayerTaskFactory.h | Add createRemoveSource() to task factory interface. |
| media/server/gstplayer/include/tasks/generic/RemoveSource.h | New RemoveSource task header. |
| media/server/gstplayer/include/tasks/generic/GenericPlayerTaskFactory.h | Declare createRemoveSource() override. |
| media/server/gstplayer/include/GstGenericPlayer.h | Declare removeSource() on concrete player. |
| media/server/gstplayer/include/GenericPlayerContext.h | Add audioSourceRemoved flag to player context. |
| media/server/gstplayer/CMakeLists.txt | Build system: compile new RemoveSource task into gstplayer library. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * @brief Removes a source from gstreamer. | ||
| * | ||
| * @param[in] mediaSourceType : The media source type. | ||
| * | ||
| */ |
| 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)); | ||
| } |
| /** | ||
| * @brief Flag used to check, if audio source has been recently removed | ||
| * | ||
| * Flag can be used only in worker thread | ||
| */ |
|
Coverage statistics of your commit: |
Summary: Fix reattaching audio source at dynamic audio codec change.
Type: Fix
Test Plan: UT/CT, Fullstack
Jira: RDKEMW-17771