Allow to switch audio codec, when audio is re-attached#448
Allow to switch audio codec, when audio is re-attached#448skywojciechowskim merged 1 commit intomasterfrom
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. |
There was a problem hiding this comment.
Pull request overview
This PR enables audio codec switching when an audio source is re-attached by adding a new code path in the AttachSource task. Previously, attempting to attach an already-attached audio source would fail with a "cannot update caps" error. Now, when an audio source is re-attached, the code will call the existing reattachSource method to perform the codec switch.
Changes:
- Added logic to handle audio source reattachment in AttachSource::execute()
- Renamed and expanded test coverage for the audio source reattachment scenario
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| media/server/gstplayer/source/tasks/generic/AttachSource.cpp | Added reattachAudioSource() method and logic to handle audio source reattachment when source already exists in streamInfo |
| media/server/gstplayer/include/tasks/generic/AttachSource.h | Added declaration for the new reattachAudioSource() private method |
| tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/AttachSourceTest.cpp | Renamed test from shouldSkipSwitchAudioSourceWhenSourceIsNotRemoved to shouldSwitchAudioSourceWhenSourceIsReattached and added a failure test case |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| RIALTO_SERVER_LOG_ERROR("Reattaching source failed!"); | ||
| return; | ||
| } | ||
| RIALTO_SERVER_LOG_MIL("Audio source reattached"); |
There was a problem hiding this comment.
The success log message is inconsistent with the codebase convention. Throughout the codebase, log messages use common::convertMediaSourceType to make messages generic for different source types. For example, SwitchSource uses "%s source switched" with convertMediaSourceType. Consider using a similar pattern like "RIALTO_SERVER_LOG_MIL("%s source reattached", common::convertMediaSourceType(m_attachedSource->getType()))" to match the established convention and allow the code to work generically for any source type that might be supported in the future.
| RIALTO_SERVER_LOG_MIL("Audio source reattached"); | |
| RIALTO_SERVER_LOG_MIL("%s source reattached", | |
| common::convertMediaSourceType(m_attachedSource->getType())); |
| { | ||
| if (!m_player.reattachSource(m_attachedSource)) | ||
| { | ||
| RIALTO_SERVER_LOG_ERROR("Reattaching source failed!"); |
There was a problem hiding this comment.
The error logging level is inconsistent with SwitchSource task. The SwitchSource task uses RIALTO_SERVER_LOG_WARN for the same reattachSource failure scenario (see SwitchSource.cpp line 43). Consider using RIALTO_SERVER_LOG_WARN instead of RIALTO_SERVER_LOG_ERROR for consistency.
| RIALTO_SERVER_LOG_ERROR("Reattaching source failed!"); | |
| RIALTO_SERVER_LOG_WARN("Reattaching source failed!"); |
|
Coverage statistics of your commit: |
2 participants
Summary: Allow to switch audio codec, when audio is re-attached
Type: Fix
Test Plan: UT/CT, Fullstack
Jira: LLAMA-18057