From 5846d046159fafe5be4a01d56cf8b63d3ada90f2 Mon Sep 17 00:00:00 2001 From: DomSyna <192202460+DomSyna@users.noreply.github.com> Date: Tue, 2 Dec 2025 17:35:50 +0000 Subject: [PATCH 1/4] VPLAY-11969 GetTextTrackInfo returns stale information If the text track is changed without a re-tune then priv_aamps mPreferredTextTrack, and stream abstractions mTextTrackIndex are not updated to reflect the current track The manifest url can be set to null due to an issue with passing the same string into cache handler methods as multiple params by reference. --- AampCacheHandler.cpp | 22 +++++++++++++++------- StreamAbstractionAAMP.h | 8 ++++++++ priv_aamp.cpp | 2 ++ 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/AampCacheHandler.cpp b/AampCacheHandler.cpp index 6166d1b03..7ff840ea8 100644 --- a/AampCacheHandler.cpp +++ b/AampCacheHandler.cpp @@ -66,16 +66,20 @@ bool AampCacheHandler::RetrieveFromPlaylistCache( const std::string &url, AampGr AampCachedData *cachedData = mPlaylistCache.Find(url); if( cachedData ) { + // Create a local copy to prevent aliasing bug when url and effectiveUrl reference the same string + // This can happen when callers pass GetManifestUrl() for both parameters + std::string urlCopy = url; + effectiveUrl = cachedData->effectiveUrl; - if( effectiveUrl.empty() ) + if (effectiveUrl.empty()) { - effectiveUrl = url; + effectiveUrl = urlCopy; } buffer->Clear(); - buffer->AppendBytes( cachedData->buffer->GetPtr(), cachedData->buffer->GetLen() ); + buffer->AppendBytes(cachedData->buffer->GetPtr(), cachedData->buffer->GetLen()); // below fails when playing an HLS playlist directly, then seeking or retuning // assert( mediaType == cachedData->mediaType ); - AAMPLOG_TRACE( "%s %s found", GetMediaTypeName(cachedData->mediaType), url.c_str() ); + AAMPLOG_TRACE("%s %s found", GetMediaTypeName(cachedData->mediaType), url.c_str()); ret = true; } else @@ -124,19 +128,23 @@ bool AampCacheHandler::RetrieveFromInitFragmentCache(const std::string &url, Aam bool ret = false; std::lock_guard lock(mCacheAccessMutex); AampCachedData *cachedData = mInitFragmentCache.Find(url); - if( cachedData ) + if (cachedData) { std::shared_ptr buf = cachedData->buffer; + + // Create a local copy to prevent aliasing bug when url and effectiveUrl reference the same string + std::string urlCopy = url; + if (cachedData->effectiveUrl.empty()) { - effectiveUrl = url; + effectiveUrl = urlCopy; } else { effectiveUrl = cachedData->effectiveUrl; } buffer->Clear(); - buffer->AppendBytes( buf->GetPtr(), buf->GetLen() ); + buffer->AppendBytes(buf->GetPtr(), buf->GetLen()); AAMPLOG_INFO("%s %s found", GetMediaTypeName(cachedData->mediaType), url.c_str()); ret = true; } diff --git a/StreamAbstractionAAMP.h b/StreamAbstractionAAMP.h index 7c95ef9b4..da97bc872 100644 --- a/StreamAbstractionAAMP.h +++ b/StreamAbstractionAAMP.h @@ -1902,6 +1902,14 @@ class StreamAbstractionAAMP : public AampLicenseFetcher */ void SetCurrentAudioTrackIndex(std::string& index) { mAudioTrackIndex = index; } + /** + * @brief Set current text track index + * + * @param[in] string index + * @return void + */ + void SetCurrentTextTrackIndex(std::string& index) { mTextTrackIndex = index; } + /** * @brief Change muxed audio track index * diff --git a/priv_aamp.cpp b/priv_aamp.cpp index 5dc8539c5..5946788bd 100644 --- a/priv_aamp.cpp +++ b/priv_aamp.cpp @@ -12610,6 +12610,8 @@ void PrivateInstanceAAMP::SetPreferredTextLanguages(const char *param) if (closedCaptionTrackId >= 0) { TextTrackInfo track = trackInfo[closedCaptionTrackId]; + SetPreferredTextTrack(track); // If we found the track via CheckPreferredTextLanguages() it may not be the current preferred text track + mpStreamAbstractionAAMP->SetCurrentTextTrackIndex(track.index); // Normally set as part of parsing the manifest during tune, but if we don't tune we should keep it consistent SetClosedCaptionsFromTextTrack(track); } } From 15fef7f227ace5b4c9a088b68f76fecb1f9cc336 Mon Sep 17 00:00:00 2001 From: DomSyna <192202460+DomSyna@users.noreply.github.com> Date: Wed, 3 Dec 2025 15:11:49 +0000 Subject: [PATCH 2/4] VPLAY-11969 GetTextTrackInfo returns stale information If the text track is changed without a re-tune then priv_aamps mPreferredTextTrack, and stream abstractions mTextTrackIndex are not updated to reflect the current track The manifest url can be set to null due to an issue with passing the same string into cache handler methods as multiple params by reference. The specified language via SetTextTrack() is ignored in perference to the prefferedTextLanguage list. --- fragmentcollector_hls.cpp | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/fragmentcollector_hls.cpp b/fragmentcollector_hls.cpp index 1031d4671..aaecf217c 100644 --- a/fragmentcollector_hls.cpp +++ b/fragmentcollector_hls.cpp @@ -3450,19 +3450,26 @@ AAMPStatusType StreamAbstractionAAMP_HLS::Init(TuneType tuneType) // Generate audio and text track structures PopulateAudioAndTextTracks(); - // Select preferred text track based on user language preferences - TextTrackInfo selectedTextTrack; - if (SelectPreferredTextTrack(selectedTextTrack)) + if (aamp->GetPreferredTextTrack().index.empty()) { - AAMPLOG_INFO("Selected text track - lang:%s, name:%s, rendition:%s", - selectedTextTrack.language.c_str(), - selectedTextTrack.name.c_str(), - selectedTextTrack.rendition.c_str()); - aamp->SetPreferredTextTrack(std::move(selectedTextTrack)); + // Select preferred text track based on user language preferences + TextTrackInfo selectedTextTrack; + if (SelectPreferredTextTrack(selectedTextTrack)) + { + AAMPLOG_INFO("Selected text track - lang:%s, name:%s, rendition:%s", + selectedTextTrack.language.c_str(), + selectedTextTrack.name.c_str(), + selectedTextTrack.rendition.c_str()); + aamp->SetPreferredTextTrack(std::move(selectedTextTrack)); + } + else + { + AAMPLOG_WARN("No text track matched user preferences, will use default selection"); + } } else { - AAMPLOG_WARN("No text track matched user preferences, will use default selection"); + AAMPLOG_INFO("Preferred text track set, so not looking at the list of preferred languages"); } // Configure Subtitle track for the playback @@ -7593,6 +7600,7 @@ void StreamAbstractionAAMP_HLS::SelectSubtitleTrack() bool StreamAbstractionAAMP_HLS::SelectPreferredTextTrack(TextTrackInfo &selectedTextTrack) { std::vector availableTracks = GetAvailableTextTracks(); + if (availableTracks.empty()) { AAMPLOG_WARN("No text tracks available"); From 6c409881d34fe7d57613ddf2d70f374c763cc890 Mon Sep 17 00:00:00 2001 From: DomSyna <192202460+DomSyna@users.noreply.github.com> Date: Thu, 4 Dec 2025 09:13:44 +0000 Subject: [PATCH 3/4] VPLAY-11969 GetTextTrackInfo returns stale information If the text track is changed without a re-tune then priv_aamps mPreferredTextTrack, and stream abstractions mTextTrackIndex are not updated to reflect the current track The manifest url can be set to null due to an issue with passing the same string into cache handler methods as multiple params by reference. The specified language set via SetTextTrack() is ignored in perference to the prefferedTextLanguage list. --- AampCacheHandler.cpp | 29 ++++------------------ AampCacheHandler.h | 4 +-- test/utests/fakes/FakeAampCacheHandler.cpp | 4 +-- 3 files changed, 9 insertions(+), 28 deletions(-) diff --git a/AampCacheHandler.cpp b/AampCacheHandler.cpp index 7ff840ea8..48e4eca81 100644 --- a/AampCacheHandler.cpp +++ b/AampCacheHandler.cpp @@ -59,22 +59,14 @@ void AampCacheHandler::StopPlaylistCache( void ) mCondVar.notify_one(); } -bool AampCacheHandler::RetrieveFromPlaylistCache( const std::string &url, AampGrowableBuffer* buffer, std::string& effectiveUrl, AampMediaType mediaType ) +bool AampCacheHandler::RetrieveFromPlaylistCache(std::string url, AampGrowableBuffer* buffer, std::string& effectiveUrl, AampMediaType mediaType) { bool ret = false; std::lock_guard lock(mCacheAccessMutex); AampCachedData *cachedData = mPlaylistCache.Find(url); - if( cachedData ) + if (cachedData) { - // Create a local copy to prevent aliasing bug when url and effectiveUrl reference the same string - // This can happen when callers pass GetManifestUrl() for both parameters - std::string urlCopy = url; - - effectiveUrl = cachedData->effectiveUrl; - if (effectiveUrl.empty()) - { - effectiveUrl = urlCopy; - } + effectiveUrl = cachedData->effectiveUrl.empty() ? url : cachedData->effectiveUrl; buffer->Clear(); buffer->AppendBytes(cachedData->buffer->GetPtr(), cachedData->buffer->GetLen()); // below fails when playing an HLS playlist directly, then seeking or retuning @@ -123,7 +115,7 @@ void AampCacheHandler::InsertToInitFragCache( const std::string &url, const Aamp mInitFragmentCache.Insert( url, buffer, effectiveUrl, mediaType ); } -bool AampCacheHandler::RetrieveFromInitFragmentCache(const std::string &url, AampGrowableBuffer* buffer, std::string& effectiveUrl) +bool AampCacheHandler::RetrieveFromInitFragmentCache(std::string url, AampGrowableBuffer* buffer, std::string& effectiveUrl) { bool ret = false; std::lock_guard lock(mCacheAccessMutex); @@ -131,18 +123,7 @@ bool AampCacheHandler::RetrieveFromInitFragmentCache(const std::string &url, Aam if (cachedData) { std::shared_ptr buf = cachedData->buffer; - - // Create a local copy to prevent aliasing bug when url and effectiveUrl reference the same string - std::string urlCopy = url; - - if (cachedData->effectiveUrl.empty()) - { - effectiveUrl = urlCopy; - } - else - { - effectiveUrl = cachedData->effectiveUrl; - } + effectiveUrl = cachedData->effectiveUrl.empty() ? url : cachedData->effectiveUrl; buffer->Clear(); buffer->AppendBytes(buf->GetPtr(), buf->GetLen()); AAMPLOG_INFO("%s %s found", GetMediaTypeName(cachedData->mediaType), url.c_str()); diff --git a/AampCacheHandler.h b/AampCacheHandler.h index c08ab9b99..a98317eb8 100755 --- a/AampCacheHandler.h +++ b/AampCacheHandler.h @@ -447,7 +447,7 @@ class AampCacheHandler * @param[out] effectiveUrl - Final URL * @return true: found, false: not found */ - bool RetrieveFromPlaylistCache( const std::string &url, AampGrowableBuffer* buffer, std::string& effectiveUrl, AampMediaType mediaType ); + bool RetrieveFromPlaylistCache(std::string url, AampGrowableBuffer* buffer, std::string& effectiveUrl, AampMediaType mediaType); /** * @brief Remove playlist from cache @@ -493,7 +493,7 @@ class AampCacheHandler * * @return true: found, false: not found */ - bool RetrieveFromInitFragmentCache(const std::string &url, AampGrowableBuffer* buffer, std::string& effectiveUrl); + bool RetrieveFromInitFragmentCache(std::string url, AampGrowableBuffer* buffer, std::string& effectiveUrl); /** * @brief set max initialization fragments allowed in cache (per track) diff --git a/test/utests/fakes/FakeAampCacheHandler.cpp b/test/utests/fakes/FakeAampCacheHandler.cpp index e8c969548..54316a56e 100644 --- a/test/utests/fakes/FakeAampCacheHandler.cpp +++ b/test/utests/fakes/FakeAampCacheHandler.cpp @@ -39,7 +39,7 @@ void AampCacheHandler::StopPlaylistCache() { } -bool AampCacheHandler::RetrieveFromPlaylistCache(const std::string &url, AampGrowableBuffer* buffer, std::string& effectiveUrl, AampMediaType mediaType) +bool AampCacheHandler::RetrieveFromPlaylistCache(std::string url, AampGrowableBuffer* buffer, std::string& effectiveUrl, AampMediaType mediaType) { return false; } @@ -65,7 +65,7 @@ void AampCacheHandler::InsertToInitFragCache( const std::string &url, const Aamp { } -bool AampCacheHandler::RetrieveFromInitFragmentCache(const std::string &url, AampGrowableBuffer* buffer, std::string& effectiveUrl) +bool AampCacheHandler::RetrieveFromInitFragmentCache(std::string url, AampGrowableBuffer* buffer, std::string& effectiveUrl) { return false; } From cdfdbd984400642a04c47b1e2346fc1ee4ec18bb Mon Sep 17 00:00:00 2001 From: DomSyna <192202460+DomSyna@users.noreply.github.com> Date: Fri, 5 Dec 2025 17:20:29 +0000 Subject: [PATCH 4/4] VPLAY-11969 GetTextTrackInfo returns stale information If the text track is changed without a re-tune then priv_aamps mPreferredTextTrack, and stream abstractions mTextTrackIndex are not updated to reflect the current track The manifest url can be set to null due to an issue with passing the same string into cache handler methods as multiple params by reference. The specified language set via SetTextTrack() is ignored in perference to the prefferedTextLanguage list. --- StreamAbstractionAAMP.h | 2 +- streamabstraction.cpp | 9 +++++++ .../fakes/FakeStreamAbstractionAamp.cpp | 8 ++++++ test/utests/mocks/MockStreamAbstractionAAMP.h | 2 ++ .../SetPreferredTextLanguagesTests.cpp | 25 +++++++++++++++++-- 5 files changed, 43 insertions(+), 3 deletions(-) diff --git a/StreamAbstractionAAMP.h b/StreamAbstractionAAMP.h index 2b9e47557..a13a618de 100644 --- a/StreamAbstractionAAMP.h +++ b/StreamAbstractionAAMP.h @@ -1915,7 +1915,7 @@ class StreamAbstractionAAMP : public AampLicenseFetcher * @param[in] string index * @return void */ - void SetCurrentTextTrackIndex(std::string& index) { mTextTrackIndex = index; } + void SetCurrentTextTrackIndex(std::string& index); /** * @brief Change muxed audio track index diff --git a/streamabstraction.cpp b/streamabstraction.cpp index 906f440f1..e55b55c8b 100644 --- a/streamabstraction.cpp +++ b/streamabstraction.cpp @@ -3824,6 +3824,15 @@ bool StreamAbstractionAAMP::GetCurrentTextTrack(TextTrackInfo &textTrack) } return bFound; } + +/** + * @brief Set current text track index + */ +void StreamAbstractionAAMP::SetCurrentTextTrackIndex(std::string& index) +{ + mTextTrackIndex = index; +} + /** * @brief verify in-band CC availability for a stream. */ diff --git a/test/utests/fakes/FakeStreamAbstractionAamp.cpp b/test/utests/fakes/FakeStreamAbstractionAamp.cpp index 8045397da..32bd8aef7 100644 --- a/test/utests/fakes/FakeStreamAbstractionAamp.cpp +++ b/test/utests/fakes/FakeStreamAbstractionAamp.cpp @@ -111,6 +111,14 @@ bool StreamAbstractionAAMP::GetCurrentTextTrack(TextTrackInfo &textTrack) return 0; } +void StreamAbstractionAAMP::SetCurrentTextTrackIndex(std::string& index) +{ + if (g_mockStreamAbstractionAAMP != nullptr) + { + g_mockStreamAbstractionAAMP->SetCurrentTextTrackIndex(index); + } +} + bool StreamAbstractionAAMP::isInBandCcAvailable() { return 0; diff --git a/test/utests/mocks/MockStreamAbstractionAAMP.h b/test/utests/mocks/MockStreamAbstractionAAMP.h index b75770ff8..75ce5af0d 100644 --- a/test/utests/mocks/MockStreamAbstractionAAMP.h +++ b/test/utests/mocks/MockStreamAbstractionAAMP.h @@ -98,6 +98,8 @@ class MockStreamAbstractionAAMP : public StreamAbstractionAAMP MOCK_METHOD(bool, DoEarlyStreamSinkFlush, (bool newTune, float rate), (override)); MOCK_METHOD(void, ReinitializeInjection, (double rate)); + + MOCK_METHOD(void, SetCurrentTextTrackIndex, (std::string& index)); }; extern MockStreamAbstractionAAMP *g_mockStreamAbstractionAAMP; diff --git a/test/utests/tests/PreferredLanguages/SetPreferredTextLanguagesTests.cpp b/test/utests/tests/PreferredLanguages/SetPreferredTextLanguagesTests.cpp index 247db23d1..5f5201187 100644 --- a/test/utests/tests/PreferredLanguages/SetPreferredTextLanguagesTests.cpp +++ b/test/utests/tests/PreferredLanguages/SetPreferredTextLanguagesTests.cpp @@ -255,6 +255,11 @@ TEST_F(SetPreferredTextLanguagesTests, LanguageListTest2) EXPECT_STREQ(mPrivateInstanceAAMP->preferredTextLanguagesString.c_str(), "lang1"); EXPECT_EQ(mPrivateInstanceAAMP->preferredTextLanguagesList.size(), 1); EXPECT_STREQ(mPrivateInstanceAAMP->preferredTextLanguagesList.at(0).c_str(), "lang1"); + + /* Verify SetPreferredTextTrack() was called and stored the track */ + const TextTrackInfo& preferredTrack = mPrivateInstanceAAMP->GetPreferredTextTrack(); + EXPECT_STREQ(preferredTrack.language.c_str(), "lang0"); + EXPECT_STREQ(preferredTrack.index.c_str(), "idx0"); } /** @@ -452,14 +457,13 @@ TEST_F(SetPreferredTextLanguagesTests, RenditionTest1) EXPECT_CALL(*g_mockStreamAbstractionAAMP, SelectPreferredTextTrack(_)) .WillOnce(::testing::DoAll(::testing::SetArgReferee<0>(tracks[0]),Return(true))); EXPECT_CALL(*g_mockStreamAbstractionAAMP, Stop(_)) - .Times(1); + .WillOnce(Invoke(this, &SetPreferredTextLanguagesTests::Stop)); EXPECT_CALL(*g_mockAampGstPlayer, Flush(_,_,_)) .Times(1); mPrivateInstanceAAMP->SetPreferredTextLanguages("{\"rendition\":\"rend0\"}"); /* Verify the preferred rendition list. */ EXPECT_STREQ(mPrivateInstanceAAMP->preferredTextRenditionString.c_str(), "rend0"); - g_mockStreamAbstractionAAMP = NULL; } /** @@ -710,6 +714,9 @@ TEST_F(SetPreferredTextLanguagesTests, SetTsbSessionManagerNull) // This test sets IsLocalAAMPTsb=true, so the mock is not deleted by the code-under-test. EXPECT_CALL(*g_mockStreamAbstractionAAMP, Stop(_)).WillRepeatedly(Return()); + + EXPECT_CALL(*g_mockStreamAbstractionAAMP, SetCurrentTextTrackIndex(_)) + .Times(1); testp_aamp->SetPreferredTextLanguages("{\"languages\":\"lang1\"}"); @@ -763,6 +770,10 @@ TEST_F(SetPreferredTextLanguagesTests, ChangePrefTextLangWithTSB) .WillOnce(::testing::DoAll(::testing::SetArgReferee<0>(tracks[0]),Return(true))); // This test sets IsLocalAAMPTsb=true, so the mock is not deleted by the code-under-test. EXPECT_CALL(*g_mockStreamAbstractionAAMP, Stop(_)).Times(2).WillRepeatedly(Return()); + + // SetCurrentTextTrackIndex is called in TSB scenarios + EXPECT_CALL(*g_mockStreamAbstractionAAMP, SetCurrentTextTrackIndex(_)) + .Times(1); testp_aamp->SetPreferredTextLanguages("{\"languages\":\"lang1\"}"); @@ -805,6 +816,10 @@ TEST_F(SetPreferredTextLanguagesTests, ClosedCaptionTest1) .WillOnce(ReturnRef(tracks)); EXPECT_CALL(*g_mockStreamAbstractionAAMP, Stop(_)).Times(0); EXPECT_CALL(*g_mockPlayerCCManager, SetTrack("CC1",eCLOSEDCAPTION_FORMAT_608)).Times(1).WillRepeatedly(Return(0)); + + // SetCurrentTextTrackIndex is called for closed caption track changes + EXPECT_CALL(*g_mockStreamAbstractionAAMP, SetCurrentTextTrackIndex(_)) + .Times(1); mPrivateInstanceAAMP->SetPreferredTextLanguages("lang1"); @@ -813,6 +828,12 @@ TEST_F(SetPreferredTextLanguagesTests, ClosedCaptionTest1) EXPECT_EQ(mPrivateInstanceAAMP->preferredTextLanguagesList.size(), 1); EXPECT_STREQ(mPrivateInstanceAAMP->preferredTextLanguagesList.at(0).c_str(), "lang1"); + /* Verify SetPreferredTextTrack() was called and stored the track */ + const TextTrackInfo& preferredTrack = mPrivateInstanceAAMP->GetPreferredTextTrack(); + EXPECT_STREQ(preferredTrack.language.c_str(), "lang1"); + EXPECT_STREQ(preferredTrack.index.c_str(), "idx1"); + EXPECT_TRUE(preferredTrack.isCC); + } /**