VPLAY-12012: Deprecate auxiliary audio support in AAMP #740
Conversation
There was a problem hiding this comment.
Pull request overview
This PR deprecates auxiliary audio support in AAMP by removing all auxiliary audio-related functionality from the codebase. The changes systematically eliminate auxiliary audio handling from the streaming abstraction layer, player instances, middleware components, and test infrastructure.
Key Changes:
- Removed auxiliary audio track support from stream abstraction interfaces and implementations
- Updated method signatures to remove auxiliary audio format parameters
- Cleaned up auxiliary audio-related enumerations, constants, and configuration options
Reviewed changes
Copilot reviewed 82 out of 85 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| AampMediaType.h | Removed eMEDIATYPE_AUX_AUDIO and related enum values |
| StreamAbstractionAAMP.h | Removed eTRACK_AUX_AUDIO track type and auxiliary audio methods |
| priv_aamp.h/cpp | Removed auxiliary audio language settings and format handling |
| StreamSink.h | Updated Configure signature to remove auxFormat parameter |
| fragmentcollector_hls.h/cpp | Removed auxiliary audio track handling and playlist logic |
| middleware/InterfacePlayerRDK.h/cpp | Removed auxiliary audio pipeline forwarding |
| test files | Updated all test cases to remove auxiliary audio parameters and assertions |
| AampDefine.h | Updated AAMP_TRACK_COUNT from 4 to 3 tracks |
Comments suppressed due to low confidence (2)
test/utests/tests/AampGstPlayer/FunctionalTests.cpp:1
- Corrected typo: removed extraneous 's' character at end of line.
middleware/test/utests/tests/GstPlayer/FunctionalTests.cpp:1 - [nitpick] Comment was updated to be more generic but 'specific SOC builds' is vague. Consider specifying which SOC platforms this applies to, or remove the caveat if it's no longer relevant after auxiliary audio removal.
| } | ||
| static void callback_element_added (GstElement * element, GstElement * source, gpointer data) | ||
| { | ||
| // TODO: Check if this is needed |
There was a problem hiding this comment.
TODO comment lacks context about what needs to be checked. Consider clarifying what functionality or side effects need verification before potential removal of the callback_element_added function.
| // TODO: Check if this is needed | |
| // TODO: Before removing this callback, verify if it is required for logging, debugging, or as a necessary hook for element addition in the pipeline. Ensure that its removal does not affect playback, element initialization, or any dependent functionality. |
fragmentcollector_mpd.cpp
Outdated
| } | ||
|
|
||
| //TODO - check whether the ugly hack above is in operation | ||
| // This is again a dirty hack, the check for PTS restamp enabled. TODO: We need to remove this in future |
There was a problem hiding this comment.
[nitpick] Existing TODO mentions removing a 'dirty hack' related to PTS restamp checks. With auxiliary audio removal, consider whether this hack is still necessary or can be addressed now.
Coverity Issue - Out-of-bounds readOverrunning array "this->mMediaStreamContext" of 3 8-byte elements at element index 3 (byte offset 31) using index "iTrack" (which evaluates to 3). High Impact, CWE-125 Issue locationThis issue was discovered outside the diff for this Pull Request. You can find it at: |
Coverity Issue - Out-of-bounds accessOverrunning callee's array of size 3 by passing argument "(AampMediaType)iTrack" (which evaluates to 3) in call to "StopTrackInjection". High Impact, CWE-119 Issue locationThis issue was discovered outside the diff for this Pull Request. You can find it at: |
Reason for change: Deprecate auxiliary audio support Test Procedure: Ensure there are no regression Risks: None Signed-off-by: Vinish100 <[email protected]>
Vinish100
force-pushed
the
feature/VPLAY-12012
branch
from
b50a52f to
de7dab1
Compare
Reason for change: Deprecate auxiliary audio support
Test Procedure: Ensure there are no regression
Risks: None