From ab96c8c31bdb06edc21b42d34e3ba53f49191460 Mon Sep 17 00:00:00 2001 From: varshnie Date: Fri, 13 Feb 2026 09:27:41 +0530 Subject: [PATCH 1/4] VPLAY-12682:SoC-independent underflow detection - follow-up improvements Reason for change:follow-up improvements for AampUnderflowMonitor Risks: Medium Signed-off-by: varshnie VPLAY-12605:[L1 Test] AampUnderflowMonitor Tests Reason for change:Added L1 for AampUnderflowMonitor Test Procedure:Refer Jira Ticket Risks: Medium Signed-off-by: varshnie Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- AampUnderflowMonitor.cpp | 36 +-- AampUnderflowMonitor.h | 22 +- StreamAbstractionAAMP.h | 7 - fragmentcollector_hls.cpp | 15 +- fragmentcollector_mpd.cpp | 13 + fragmentcollector_progressive.cpp | 10 + priv_aamp.cpp | 47 ++-- priv_aamp.h | 4 +- streamabstraction.cpp | 63 +++-- .../utests/fakes/FakeAampUnderflowMonitor.cpp | 59 +++-- test/utests/fakes/FakePrivateInstanceAAMP.cpp | 11 +- test/utests/mocks/MockAampUnderflowMonitor.h | 36 +++ test/utests/mocks/MockPrivateInstanceAAMP.h | 3 + .../AampUnderflowMonitorTest.cpp | 26 ++ .../tests/AampUnderflowMonitor/CMakeLists.txt | 51 ++++ .../AampUnderflowMonitor/FunctionalTests.cpp | 225 ++++++++++++++++++ test/utests/tests/CMakeLists.txt | 1 + .../StreamAbstractionAAMP/CMakeLists.txt | 2 +- .../StreamAbstractionAAMP/FunctionalTests.cpp | 51 +++- 19 files changed, 537 insertions(+), 145 deletions(-) create mode 100644 test/utests/mocks/MockAampUnderflowMonitor.h create mode 100644 test/utests/tests/AampUnderflowMonitor/AampUnderflowMonitorTest.cpp create mode 100644 test/utests/tests/AampUnderflowMonitor/CMakeLists.txt create mode 100644 test/utests/tests/AampUnderflowMonitor/FunctionalTests.cpp diff --git a/AampUnderflowMonitor.cpp b/AampUnderflowMonitor.cpp index fe3599237..869f2c342 100644 --- a/AampUnderflowMonitor.cpp +++ b/AampUnderflowMonitor.cpp @@ -90,20 +90,19 @@ void AampUnderflowMonitor::Start() { void AampUnderflowMonitor::Stop() { + // Use unique_lock so we can release the mutex before joining + std::unique_lock lock(mMutex); // Signal thread to stop mRunning.store(false); - - // Wait for thread to terminate + + // If a thread is joinable, release the lock before joining to avoid deadlock if (mThread.joinable()) { + lock.unlock(); mThread.join(); AAMPLOG_INFO("AampUnderflowMonitor thread joined"); + lock.lock(); } - - // Nullify pointers under mutex to prevent any race with thread cleanup - std::lock_guard lock(mMutex); - mAamp = nullptr; - mStream = nullptr; } void AampUnderflowMonitor::Run() @@ -125,8 +124,6 @@ void AampUnderflowMonitor::Run() { std::lock_guard lock(mMutex); - if (!mAamp) return; // Stop() was called - state = mAamp->GetState(); if (state == eSTATE_STOPPED || state == eSTATE_RELEASED || state == eSTATE_ERROR) { mRunning.store(false); @@ -139,12 +136,7 @@ void AampUnderflowMonitor::Run() if (shouldBreak) { break; } - - { - std::lock_guard lock(mMutex); - if (!mAamp) return; - mAamp->interruptibleMsSleep(100); - } + mAamp->interruptibleMsSleep(100); } while (mRunning.load()) { @@ -156,7 +148,6 @@ void AampUnderflowMonitor::Run() { std::lock_guard lock(mMutex); - if (!mAamp || !mStream) return; // Stop() was called underflowActive = mAamp->GetBufUnderFlowStatus(); playerState = mAamp->GetState(); @@ -184,7 +175,6 @@ void AampUnderflowMonitor::Run() { std::lock_guard lock(mMutex); - if (!mAamp) return; trackDownloadsEnabled = mAamp->TrackDownloadsAreEnabled(eMEDIATYPE_VIDEO); sinkCacheEmpty = mAamp->IsSinkCacheEmpty(eMEDIATYPE_VIDEO); } @@ -198,7 +188,6 @@ void AampUnderflowMonitor::Run() AAMPLOG_INFO("[video] underflow detected. buffered=%.3f cacheEmpty=%d (rate=%.2f, trickplay=%d, seeking=%d)", bufferedTimeSec, (int)sinkCacheEmpty, currentRate, (int)isTrickplay, (int)isSeekingState); std::lock_guard lock(mMutex); - if (!mAamp) return; mAamp->SetBufferingState(true); PlaybackErrorType errorType = eGST_ERROR_UNDERFLOW; mAamp->SendAnomalyEvent(ANOMALY_WARNING, "%s %s", GetMediaTypeName(eMEDIATYPE_VIDEO), mAamp->getStringForPlaybackError(errorType)); @@ -209,7 +198,6 @@ void AampUnderflowMonitor::Run() { AAMPLOG_WARN("[video] downloads blocked with empty cache during underflow; resuming"); std::lock_guard lock(mMutex); - if (!mAamp) return; mAamp->ResumeTrackDownloads(eMEDIATYPE_VIDEO); } } @@ -236,7 +224,6 @@ void AampUnderflowMonitor::Run() { AAMPLOG_INFO("[video] underflow ended. buffered=%.3f cacheEmpty=%d", bufferedTimeSec, (int)sinkCacheEmpty); std::lock_guard lock(mMutex); - if (!mAamp) return; mAamp->SetBufferingState(false); } else @@ -247,8 +234,6 @@ void AampUnderflowMonitor::Run() else if (underflowActive && !trackDownloadsEnabled && sinkCacheEmpty) { AAMPLOG_WARN("[video] underflow ongoing, downloads blocked and cache empty; resuming track downloads"); - std::lock_guard lock(mMutex); - if (!mAamp) return; mAamp->ResumeTrackDownloads(eMEDIATYPE_VIDEO); } } @@ -259,12 +244,7 @@ void AampUnderflowMonitor::Run() const int sleepMs = (bufferedTimeSec < kLowBufferSec) ? kLowBufferPollMs : (bufferedTimeSec >= kHighBufferSec) ? kHighBufferPollMs : kMediumBufferPollMs; - - { - std::lock_guard lock(mMutex); - if (!mAamp) return; - mAamp->interruptibleMsSleep(sleepMs); - } + mAamp->interruptibleMsSleep(sleepMs); } mRunning.store(false); } diff --git a/AampUnderflowMonitor.h b/AampUnderflowMonitor.h index 59890e522..9d45fdf13 100644 --- a/AampUnderflowMonitor.h +++ b/AampUnderflowMonitor.h @@ -37,7 +37,6 @@ class PrivateInstanceAAMP; class AampUnderflowMonitor { public: /** - * @fn AampUnderflowMonitor * @brief Construct an `AampUnderflowMonitor`. * @param[in] stream Stream abstraction used to query buffered video duration * and playback state relevant to underflow detection. @@ -53,31 +52,23 @@ class AampUnderflowMonitor { AampUnderflowMonitor(StreamAbstractionAAMP* stream, PrivateInstanceAAMP* aamp); /** - * @fn ~AampUnderflowMonitor * @brief Destructor. Ensures monitoring has been stopped. */ ~AampUnderflowMonitor(); - /** - * @fn Start - * @brief Start the monitoring thread. If already running, returns immediately. - * @return void - */ + /** + * @brief Start the monitoring thread. If already running, returns immediately. + * @return void + */ void Start(); /** - * @fn Stop - * @brief Request the monitoring thread to stop and join it if joinable. - * Safe to call multiple times. Nullifies internal pointers after - * thread termination to prevent use-after-free. - * @return void - * @note After `Stop()` returns, the monitoring thread has fully terminated - * and will not access `StreamAbstractionAAMP` or `PrivateInstanceAAMP`. + * @brief Stop and join the monitoring thread. + * @return void */ void Stop(); /** - * @fn isRunning * @brief Check whether the monitoring thread is currently active. * @return true if running, false otherwise. */ @@ -85,7 +76,6 @@ class AampUnderflowMonitor { private: /** - * @fn run * @brief Thread entry routine that polls/awaits underflow conditions * and triggers coordinated handling. */ diff --git a/StreamAbstractionAAMP.h b/StreamAbstractionAAMP.h index c8455006a..deb4fc0c7 100644 --- a/StreamAbstractionAAMP.h +++ b/StreamAbstractionAAMP.h @@ -2033,13 +2033,6 @@ class StreamAbstractionAAMP : public AampLicenseFetcher void ReinitializeInjection(double rate); protected: - /** - * Mutex used to serialize UnderflowMonitor lifecycle in const methods. - * Declared mutable to allow locking within const functions such as - * IsUnderflowMonitorRunning(). - */ - mutable std::mutex mUnderflowMonitorMutex; - /** * Underflow monitor instance owned by Stream; manages detection and * handling of underflow conditions. diff --git a/fragmentcollector_hls.cpp b/fragmentcollector_hls.cpp index 77f519ffc..5ae820ba1 100644 --- a/fragmentcollector_hls.cpp +++ b/fragmentcollector_hls.cpp @@ -4977,6 +4977,16 @@ void StreamAbstractionAAMP_HLS::Start(void) track->Start(); } } + + // Start underflow monitor after successful initialization and Start() + if (mUnderflowMonitor) + { + StartUnderflowMonitor(); + if (!IsUnderflowMonitorRunning()) + { + AAMPLOG_WARN("UnderflowMonitor did not start; continuing without AampUnderflowMonitor"); + } + } } @@ -4989,7 +4999,10 @@ void StreamAbstractionAAMP_HLS::Stop(bool clearChannelData) aamp->DisableDownloads(); ReassessAndResumeAudioTrack(true); AbortWaitForAudioTrackCatchup(false); - + if(ISCONFIGSET(eAAMPConfig_EnableAampUnderflowMonitor)) + { + StopUnderflowMonitor(); + } //This is purposefully kept in a separate loop to avoid being hung //on pthread_join of fragmentCollectorThread for (int iTrack = 0; iTrack < AAMP_TRACK_COUNT; iTrack++) diff --git a/fragmentcollector_mpd.cpp b/fragmentcollector_mpd.cpp index db379b519..67d0acdb8 100644 --- a/fragmentcollector_mpd.cpp +++ b/fragmentcollector_mpd.cpp @@ -10464,6 +10464,15 @@ void StreamAbstractionAAMP_MPD::Start(void) { StartFromOtherThanAampLocalTsb(); } + // Start underflow monitor after successful initialization and Start() + if (mUnderflowMonitor) + { + StartUnderflowMonitor(); + if (!IsUnderflowMonitorRunning()) + { + AAMPLOG_WARN("UnderflowMonitor did not start; continuing without AampUnderflowMonitor"); + } + } } /** @@ -10477,6 +10486,10 @@ void StreamAbstractionAAMP_MPD::Stop(bool clearChannelData) aamp->DisableDownloads(); mCdaiObject->AbortWaitForNextAdResolved(); aamp->mAampTsbLanguageChangeInProgress = false; + if(ISCONFIGSET(eAAMPConfig_EnableAampUnderflowMonitor)) + { + StopUnderflowMonitor(); + } } ReassessAndResumeAudioTrack(true); diff --git a/fragmentcollector_progressive.cpp b/fragmentcollector_progressive.cpp index 424137847..a0ed72f4c 100644 --- a/fragmentcollector_progressive.cpp +++ b/fragmentcollector_progressive.cpp @@ -238,6 +238,16 @@ void StreamAbstractionAAMP_PROGRESSIVE::Start(void) { AAMPLOG_ERR("Failed to create FragmentCollector thread : %s", e.what()); } + + // Start underflow monitor after successful initialization and Start() + if (mUnderflowMonitor) + { + StartUnderflowMonitor(); + if (!IsUnderflowMonitorRunning()) + { + AAMPLOG_WARN("UnderflowMonitor did not start; continuing without AampUnderflowMonitor"); + } + } } /** diff --git a/priv_aamp.cpp b/priv_aamp.cpp index 98550418b..2cdd6461c 100644 --- a/priv_aamp.cpp +++ b/priv_aamp.cpp @@ -3237,19 +3237,19 @@ void PrivateInstanceAAMP::UpdateRefreshPlaylistInterval(float maxIntervalSecs) /** * @brief Sends UnderFlow Event messages */ -void PrivateInstanceAAMP::SendBufferChangeEvent(bool bufferingStopped) +void PrivateInstanceAAMP::SendBufferChangeEvent(bool bufferingStart) { // Buffer Change event indicate buffer availability - // Buffering stop notification need to be inverted to indicate if buffer available or not // BufferChangeEvent with False = Underflow / non-availability of buffer to play // BufferChangeEvent with True = Availability of buffer to play - BufferingChangedEventPtr e = std::make_shared(!bufferingStopped, GetSessionId()); + bool bufferAvailable = !bufferingStart; // Buffering stop notification need to be inverted to indicate if buffer available or not + BufferingChangedEventPtr e = std::make_shared(bufferAvailable, GetSessionId()); - SetBufUnderFlowStatus(bufferingStopped); + SetBufUnderFlowStatus(bufferingStart); AAMPLOG_INFO("PrivateInstanceAAMP: Sending Buffer Change event status (Buffering): %s", (e->buffering() ? "End": "Start")); #ifdef AAMP_TELEMETRY_SUPPORT AAMPTelemetry2 at2(mAppName); - std::string telemetryName = bufferingStopped?"VideoBufferingStart":"VideoBufferingEnd"; + std::string telemetryName = bufferingStart?"VideoBufferingStart":"VideoBufferingEnd"; at2.send(telemetryName,{/*int data*/},{/*string data*/},{/*float data*/}); #endif //AAMP_TELEMETRY_SUPPORT SendEvent(e,AAMP_EVENT_ASYNC_MODE); @@ -3266,25 +3266,28 @@ void PrivateInstanceAAMP::SendBufferChangeEvent(bool bufferingStopped) */ void PrivateInstanceAAMP::SetBufferingState(bool buffering) { - if (buffering) + if(ISCONFIGSET_PRIV(eAAMPConfig_ReportBufferEvent)) { - SendBufferChangeEvent(true); - if (!mSinkPaused.load()) + if (buffering) { - if (!PausePipeline(true, true)) + SendBufferChangeEvent(true); + if (!mSinkPaused.load()) { - AAMPLOG_ERR("Failed to pause the Pipeline"); + if (!PausePipeline(true, true)) + { + AAMPLOG_ERR("Failed to pause the Pipeline"); + } } } - } - else - { - if (mSinkPaused.load()) + else { - (void)PausePipeline(false, false); + if (mSinkPaused.load()) + { + (void)PausePipeline(false, false); + } + UpdateSubtitleTimestamp(); + SendBufferChangeEvent(false); } - UpdateSubtitleTimestamp(); - SendBufferChangeEvent(false); } } @@ -5559,7 +5562,6 @@ void PrivateInstanceAAMP::TeardownStream(bool newTune, bool disableDownloads) if (mpStreamAbstractionAAMP) { AAMPLOG_INFO("TeardownStream: Stopping StreamAbstraction"); - mpStreamAbstractionAAMP->StopUnderflowMonitor(); mpStreamAbstractionAAMP->Stop(disableDownloads); if(mContentType == ContentType_HDMIIN) @@ -6350,15 +6352,6 @@ void PrivateInstanceAAMP::TuneHelper(TuneType tuneType, bool seekWhilePaused) mpStreamAbstractionAAMP->ReSetPipelineFlushStatus(); mpStreamAbstractionAAMP->Start(); - // Start underflow monitor after successful initialization and Start() - if (mpStreamAbstractionAAMP && ISCONFIGSET_PRIV(eAAMPConfig_EnableAampUnderflowMonitor)) - { - mpStreamAbstractionAAMP->StartUnderflowMonitor(); - if (!mpStreamAbstractionAAMP->IsUnderflowMonitorRunning()) - { - AAMPLOG_WARN("UnderflowMonitor did not start; continuing without AampUnderflowMonitor"); - } - } if (!mbUsingExternalPlayer) { if (mbPlayEnabled) diff --git a/priv_aamp.h b/priv_aamp.h index 783eedb30..74450d59c 100644 --- a/priv_aamp.h +++ b/priv_aamp.h @@ -1562,10 +1562,10 @@ class PrivateInstanceAAMP : public DrmCallbacks, public std::enable_shared_from_ /** * @fn SendBufferChangeEvent * - * @param[in] bufferingStopped- Flag to indicate buffering stopped.Underflow = True + * @param[in] bufferingStart Flag indicating whether buffering has started; true when underflow begins, false when it ends * @return void */ - void SendBufferChangeEvent(bool bufferingStopped=false); + void SendBufferChangeEvent(bool bufferingStart=false); /** * @fn SendTuneMetricsEvent diff --git a/streamabstraction.cpp b/streamabstraction.cpp index 28a064106..88169e161 100644 --- a/streamabstraction.cpp +++ b/streamabstraction.cpp @@ -2180,6 +2180,23 @@ StreamAbstractionAAMP::StreamAbstractionAAMP(PrivateInstanceAAMP* aamp, id3_call { mBitrateReason = (aamp->rate != AAMP_NORMAL_PLAY_RATE) ? eAAMP_BITRATE_CHANGE_BY_TRICKPLAY : eAAMP_BITRATE_CHANGE_BY_SEEK; } + + if(GETCONFIGVALUE(eAAMPConfig_EnableAampUnderflowMonitor)) + { + if(!mUnderflowMonitor) + { + try + { + mUnderflowMonitor = std::make_unique(this, aamp); + AAMPLOG_INFO("Initialized AampUnderflowMonitor"); + } + catch (const std::exception &e) + { + AAMPLOG_ERR("Failed to initialize AampUnderflowMonitor: %s", e.what()); + mUnderflowMonitor.reset(); + } + } + } } @@ -3004,63 +3021,39 @@ bool StreamAbstractionAAMP::UpdateProfileBasedOnFragmentCache() void StreamAbstractionAAMP::StartUnderflowMonitor() { - std::lock_guard lock(mUnderflowMonitorMutex); - // Run underflow monitor only when explicitly enabled via config - if (!GETCONFIGVALUE(eAAMPConfig_EnableAampUnderflowMonitor)) - { - AAMPLOG_TRACE("UnderflowMonitor gated off by config; skipping"); - return; - } if (!GetMediaTrack(eTRACK_VIDEO)) { - AAMPLOG_WARN("StartUnderflowMonitor: video track unavailable"); - return; - } - if (!mUnderflowMonitor) - { - try + if (mUnderflowMonitor) { - mUnderflowMonitor = std::make_unique(this, aamp); - mUnderflowMonitor->Start(); - AAMPLOG_INFO("Started AampUnderflowMonitor for video"); + // No video track: delete the monitor to avoid wasted resources + mUnderflowMonitor.reset(); + AAMPLOG_INFO("StartUnderflowMonitor: no video track; deleted AampUnderflowMonitor"); } - catch (const std::exception &e) + else { - AAMPLOG_ERR("Failed to create/start AampUnderflowMonitor: %s", e.what()); - // Ensure future calls can attempt creation again - mUnderflowMonitor.reset(); + AAMPLOG_WARN("StartUnderflowMonitor: video track unavailable"); } + return; } - else + if (mUnderflowMonitor) { - // Attempt to start existing monitor; Start() is idempotent - try - { - mUnderflowMonitor->Start(); - } - catch (const std::exception &e) - { - AAMPLOG_ERR("Failed to start existing AampUnderflowMonitor: %s", e.what()); - // Reset to allow recreation on next call - mUnderflowMonitor.reset(); - } + mUnderflowMonitor->Start(); + AAMPLOG_INFO("Started AampUnderflowMonitor for video"); } } void StreamAbstractionAAMP::StopUnderflowMonitor() { - std::lock_guard lock(mUnderflowMonitorMutex); if (mUnderflowMonitor) { mUnderflowMonitor->Stop(); mUnderflowMonitor.reset(); - AAMPLOG_INFO("Stopped AampUnderflowMonitor for video"); + AAMPLOG_INFO("Stopped AampUnderflowMonitor for video; resetting monitor instance"); } } bool StreamAbstractionAAMP::IsUnderflowMonitorRunning() const { - std::lock_guard lock(mUnderflowMonitorMutex); return (mUnderflowMonitor && mUnderflowMonitor->IsRunning()); } /** diff --git a/test/utests/fakes/FakeAampUnderflowMonitor.cpp b/test/utests/fakes/FakeAampUnderflowMonitor.cpp index 7fa265eaf..06e79554d 100644 --- a/test/utests/fakes/FakeAampUnderflowMonitor.cpp +++ b/test/utests/fakes/FakeAampUnderflowMonitor.cpp @@ -1,31 +1,28 @@ /* - * If not stated otherwise in this file or this component's license file the - * following copyright and licenses apply: - * - * Copyright 2026 RDK Management - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -/** - * @file FakeAampUnderflowMonitor.cpp - * @brief Fake implementation of AampUnderflowMonitor for unit testing. - */ +* If not stated otherwise in this file or this component's license file the +* following copyright and licenses apply: +* +* Copyright 2026 RDK Management +* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ #include "AampUnderflowMonitor.h" +#include "MockAampUnderflowMonitor.h" + +MockAampUnderflowMonitor *g_mockAampUnderflowMonitor = nullptr; -AampUnderflowMonitor::AampUnderflowMonitor(StreamAbstractionAAMP* stream, PrivateInstanceAAMP* aamp) - : mStream(stream), mAamp(aamp) +AampUnderflowMonitor::AampUnderflowMonitor(StreamAbstractionAAMP* stream, PrivateInstanceAAMP* aamp) : mStream(stream), mAamp(aamp) { } @@ -35,8 +32,22 @@ AampUnderflowMonitor::~AampUnderflowMonitor() void AampUnderflowMonitor::Start() { + mRunning.store(true); + if(g_mockAampUnderflowMonitor != nullptr) + { + g_mockAampUnderflowMonitor->Start(); + } } void AampUnderflowMonitor::Stop() +{ + mRunning.store(false); + if(g_mockAampUnderflowMonitor != nullptr) + { + g_mockAampUnderflowMonitor->Stop(); + } +} + +void AampUnderflowMonitor::Run() { } diff --git a/test/utests/fakes/FakePrivateInstanceAAMP.cpp b/test/utests/fakes/FakePrivateInstanceAAMP.cpp index 05ba7a98d..b79498ac4 100644 --- a/test/utests/fakes/FakePrivateInstanceAAMP.cpp +++ b/test/utests/fakes/FakePrivateInstanceAAMP.cpp @@ -175,7 +175,10 @@ int PrivateInstanceAAMP::HandleSSLProgressCallback ( void *clientp, double dltot void PrivateInstanceAAMP::SetBufferingState(bool buffering) { - (void)buffering; + if (g_mockPrivateInstanceAAMP != nullptr) + { + g_mockPrivateInstanceAAMP->SetBufferingState(buffering); + } } void PrivateInstanceAAMP::UpdateUseSinglePipeline( void ) @@ -1141,7 +1144,11 @@ bool PrivateInstanceAAMP::IsDiscontinuityProcessPending() bool PrivateInstanceAAMP::IsSinkCacheEmpty(AampMediaType mediaType) { - return true; + if (g_mockPrivateInstanceAAMP != nullptr) + { + return g_mockPrivateInstanceAAMP->IsSinkCacheEmpty(mediaType); + } + return false; } void PrivateInstanceAAMP::NotifyBitRateChangeEvent( BitsPerSecond bitrate, BitrateChangeReason reason, int width, int height, double frameRate, double position, bool GetBWIndex, VideoScanType scantype, int aspectRatioWidth, int aspectRatioHeight) diff --git a/test/utests/mocks/MockAampUnderflowMonitor.h b/test/utests/mocks/MockAampUnderflowMonitor.h new file mode 100644 index 000000000..ef05d3255 --- /dev/null +++ b/test/utests/mocks/MockAampUnderflowMonitor.h @@ -0,0 +1,36 @@ +/* +* If not stated otherwise in this file or this component's license file the +* following copyright and licenses apply: +* +* Copyright 2026 RDK Management +* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ + +#ifndef AAMP_MOCK_UNDERFLOW_MONITOR_H +#define AAMP_MOCK_UNDERFLOW_MONITOR_H + +#include +#include "AampUnderflowMonitor.h" + +class MockAampUnderflowMonitor +{ +public: + MOCK_METHOD(void, Start, ()); + MOCK_METHOD(void, Stop,()); + MOCK_METHOD(bool, IsRunning, ()); +}; + +extern MockAampUnderflowMonitor *g_mockAampUnderflowMonitor; + +#endif /* AAMP_MOCK_UNDERFLOW_MONITOR_H */ \ No newline at end of file diff --git a/test/utests/mocks/MockPrivateInstanceAAMP.h b/test/utests/mocks/MockPrivateInstanceAAMP.h index ffc509a9a..7041ec1cf 100644 --- a/test/utests/mocks/MockPrivateInstanceAAMP.h +++ b/test/utests/mocks/MockPrivateInstanceAAMP.h @@ -101,6 +101,9 @@ class MockPrivateInstanceAAMP MOCK_METHOD(void, NotifyReservationComplete, (const std::string& reservationId)); MOCK_METHOD(void, LoadIDX, (ProfilerBucketType bucketType, std::string fragmentUrl, std::string& effectiveUrl, std::vector& fragment, unsigned int curlInstance, const char *range, int& http_code, double *downloadTime, AampMediaType mediaType, int *fogError)); MOCK_METHOD(void, UpdateUseSinglePipeline, ()); + // Hooks needed by AampUnderflowMonitor tests + MOCK_METHOD(void, SetBufferingState, (bool buffering)); + MOCK_METHOD(bool, IsSinkCacheEmpty, (AampMediaType mediaType)); }; extern MockPrivateInstanceAAMP *g_mockPrivateInstanceAAMP; diff --git a/test/utests/tests/AampUnderflowMonitor/AampUnderflowMonitorTest.cpp b/test/utests/tests/AampUnderflowMonitor/AampUnderflowMonitorTest.cpp new file mode 100644 index 000000000..58547a089 --- /dev/null +++ b/test/utests/tests/AampUnderflowMonitor/AampUnderflowMonitorTest.cpp @@ -0,0 +1,26 @@ +/* +* If not stated otherwise in this file or this component's license file the +* following copyright and licenses apply: +* +* Copyright 2026 RDK Management +* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ + +#include + +int main(int argc, char** argv) +{ + testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/test/utests/tests/AampUnderflowMonitor/CMakeLists.txt b/test/utests/tests/AampUnderflowMonitor/CMakeLists.txt new file mode 100644 index 000000000..dea8bf373 --- /dev/null +++ b/test/utests/tests/AampUnderflowMonitor/CMakeLists.txt @@ -0,0 +1,51 @@ +# If not stated otherwise in this file or this component's license file the +# following copyright and licenses apply: +# +# Copyright 2026 RDK Management +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +include(GoogleTest) + +set(AAMP_ROOT "../../../../") +set(UTESTS_ROOT "../../") +set(EXEC_NAME AampUnderflowMonitor) + +include(${CMAKE_CURRENT_LIST_DIR}/../CommonTestIncludes.cmake) + +set(TEST_SOURCES FunctionalTests.cpp + AampUnderflowMonitorTest.cpp) + +set(AAMP_SOURCES ${AAMP_ROOT}/streamabstraction.cpp + ${AAMP_ROOT}/CachedFragment.cpp + ${AAMP_ROOT}/AampUnderflowMonitor.cpp) + +add_executable(${EXEC_NAME} + ${TEST_SOURCES} + ${AAMP_SOURCES}) + +if (CMAKE_XCODE_BUILD_SYSTEM) + xcode_define_schema(${EXEC_NAME}) +endif() + +if (COVERAGE_ENABLED) + include(CodeCoverage) + APPEND_COVERAGE_COMPILER_FLAGS() +endif() + +target_link_libraries(${EXEC_NAME} fakes -lpthread ${LIBDASH_LINK_LIBRARIES} ${GLIB_LINK_LIBRARIES} ${OS_LD_FLAGS} ${GMOCK_LINK_LIBRARIES} ${GTEST_LINK_LIBRARIES} ${LIBCJSON_LINK_LIBRARIES}) + +set_target_properties(${EXEC_NAME} PROPERTIES FOLDER "utests") +set_target_properties(${EXEC_NAME} PROPERTIES COMPILE_FLAGS "-Wno-effc++ -Wno-multichar") + +aamp_utest_run_add(${EXEC_NAME}) diff --git a/test/utests/tests/AampUnderflowMonitor/FunctionalTests.cpp b/test/utests/tests/AampUnderflowMonitor/FunctionalTests.cpp new file mode 100644 index 000000000..fb4d0ded6 --- /dev/null +++ b/test/utests/tests/AampUnderflowMonitor/FunctionalTests.cpp @@ -0,0 +1,225 @@ +/* +* If not stated otherwise in this file or this component's license file the +* following copyright and licenses apply: +* +* Copyright 2026 RDK Management +* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ + +#include +#include +#include +#include +#include +#include + +#include "priv_aamp.h" +#include "AampConfig.h" +#include "AampUnderflowMonitor.h" +#include "StreamAbstractionAAMP.h" +#include "MockAampConfig.h" +#include "MockPrivateInstanceAAMP.h" +#include "MockMediaTrack.h" + +using ::testing::AnyNumber; +using ::testing::AtLeast; +using ::testing::NiceMock; +using ::testing::Return; + +static AampConfig *gUnderflowConfig = nullptr; + +class AampUnderflowMonitor_FunctionalTests : public ::testing::Test +{ +protected: + class TestStreamAbstraction : public StreamAbstractionAAMP + { + public: + explicit TestStreamAbstraction(PrivateInstanceAAMP* aamp) + : StreamAbstractionAAMP(aamp), videoTrack(nullptr) + { + } + ~TestStreamAbstraction() override + { + delete videoTrack; + videoTrack = nullptr; + } + MockMediaTrack* videoTrack; + + AAMPStatusType Init(TuneType) override { return eAAMPSTATUS_OK; } + void Start() override {} + void Stop(bool) override {} + void GetStreamFormat(StreamOutputFormat&, StreamOutputFormat&, StreamOutputFormat&) override {} + MediaTrack* GetMediaTrack(TrackType t) override + { + if (t == eTRACK_VIDEO) return videoTrack; + return nullptr; + } + }; + + PrivateInstanceAAMP* aamp = nullptr; + TestStreamAbstraction* stream = nullptr; + AampUnderflowMonitor* monitor = nullptr; + + void SetUp() override + { + if (!gUnderflowConfig) gUnderflowConfig = new AampConfig(); + g_mockAampConfig = new NiceMock(); + if (!g_mockPrivateInstanceAAMP) g_mockPrivateInstanceAAMP = new NiceMock(); + + aamp = new PrivateInstanceAAMP(gUnderflowConfig); + stream = new TestStreamAbstraction(aamp); + stream->videoTrack = new NiceMock(eTRACK_VIDEO, aamp, "video"); + monitor = new AampUnderflowMonitor(stream, aamp); + + EXPECT_CALL(*g_mockAampConfig, GetConfigValue(eAAMPConfig_MaxFragmentCached)) + .Times(AnyNumber()).WillRepeatedly(Return(0)); + EXPECT_CALL(*g_mockAampConfig, GetConfigValue(eAAMPConfig_MaxFragmentChunkCached)) + .Times(AnyNumber()).WillRepeatedly(Return(0)); + + EXPECT_CALL(*g_mockAampConfig, GetConfigValue(eAAMPConfig_UnderflowDetectThresholdSec)) + .Times(AnyNumber()).WillRepeatedly(Return(1.0)); + EXPECT_CALL(*g_mockAampConfig, GetConfigValue(eAAMPConfig_UnderflowResumeThresholdSec)) + .Times(AnyNumber()).WillRepeatedly(Return(2.0)); + EXPECT_CALL(*g_mockAampConfig, GetConfigValue(eAAMPConfig_UnderflowLowBufferSec)) + .Times(AnyNumber()).WillRepeatedly(Return(1.0)); + EXPECT_CALL(*g_mockAampConfig, GetConfigValue(eAAMPConfig_UnderflowHighBufferSec)) + .Times(AnyNumber()).WillRepeatedly(Return(10.0)); + EXPECT_CALL(*g_mockAampConfig, GetConfigValue(eAAMPConfig_UnderflowLowBufferPollMs)) + .Times(AnyNumber()).WillRepeatedly(Return(10)); + EXPECT_CALL(*g_mockAampConfig, GetConfigValue(eAAMPConfig_UnderflowMediumBufferPollMs)) + .Times(AnyNumber()).WillRepeatedly(Return(10)); + EXPECT_CALL(*g_mockAampConfig, GetConfigValue(eAAMPConfig_UnderflowHighBufferPollMs)) + .Times(AnyNumber()).WillRepeatedly(Return(10)); + } + + void TearDown() override + { + delete monitor; monitor = nullptr; + delete stream; stream = nullptr; + delete aamp; aamp = nullptr; + delete g_mockPrivateInstanceAAMP; g_mockPrivateInstanceAAMP = nullptr; + delete g_mockAampConfig; g_mockAampConfig = nullptr; + delete gUnderflowConfig; gUnderflowConfig = nullptr; + } +}; + +TEST_F(AampUnderflowMonitor_FunctionalTests, ThrowsOnNullPointers) +{ + EXPECT_THROW(AampUnderflowMonitor(nullptr, nullptr), std::invalid_argument); +} + +TEST_F(AampUnderflowMonitor_FunctionalTests, StartStopTransitionsRunningState) +{ + EXPECT_CALL(*g_mockPrivateInstanceAAMP, GetState()) + .Times(AnyNumber()) + .WillRepeatedly(Return(eSTATE_PLAYING)); + + monitor->Start(); + EXPECT_TRUE(monitor->IsRunning()); + + monitor->Stop(); + EXPECT_FALSE(monitor->IsRunning()); +} + +TEST_F(AampUnderflowMonitor_FunctionalTests, StartTwiceIsIdempotent) +{ + EXPECT_CALL(*g_mockPrivateInstanceAAMP, GetState()) + .Times(AnyNumber()) + .WillRepeatedly(Return(eSTATE_PLAYING)); + + monitor->Start(); + EXPECT_TRUE(monitor->IsRunning()); + EXPECT_NO_THROW(monitor->Start()); + + monitor->Stop(); + EXPECT_FALSE(monitor->IsRunning()); +} + +TEST_F(AampUnderflowMonitor_FunctionalTests, RunDetectsUnderflowByBufferThreshold) +{ + aamp->rate = AAMP_NORMAL_PLAY_RATE; + + EXPECT_CALL(*g_mockPrivateInstanceAAMP, GetState()) + .Times(AnyNumber()) + .WillRepeatedly(Return(eSTATE_PLAYING)); + EXPECT_CALL(*stream->videoTrack, GetBufferedDuration()) + .Times(AnyNumber()) + .WillRepeatedly(Return(0.0)); + EXPECT_CALL(*g_mockPrivateInstanceAAMP, TrackDownloadsAreEnabled(eMEDIATYPE_VIDEO)) + .WillRepeatedly(Return(true)); + EXPECT_CALL(*g_mockPrivateInstanceAAMP, IsSinkCacheEmpty(eMEDIATYPE_VIDEO)) + .WillRepeatedly(Return(false)); + + std::atomic bufferingStarted{false}; + EXPECT_CALL(*g_mockPrivateInstanceAAMP, SetBufferingState(true)) + .Times(AtLeast(1)) + .WillOnce(::testing::Invoke([&](bool){ bufferingStarted.store(true); })); + + monitor->Start(); + auto start = std::chrono::steady_clock::now(); + while (!bufferingStarted.load() && (std::chrono::steady_clock::now() - start) < std::chrono::milliseconds(1000)) + { + std::this_thread::yield(); + } + monitor->Stop(); + EXPECT_TRUE(bufferingStarted.load()); +} + +TEST_F(AampUnderflowMonitor_FunctionalTests, RunEndsUnderflowDuringTrickplayWhenResumeThresholdMet) +{ + aamp->SetBufUnderFlowStatus(true); + aamp->mSinkPaused.store(true); + aamp->rate = 2.0f; + + EXPECT_CALL(*g_mockAampConfig, GetConfigValue(eAAMPConfig_UnderflowResumeThresholdSec)) + .Times(AnyNumber()).WillRepeatedly(Return(0.0)); + EXPECT_CALL(*g_mockPrivateInstanceAAMP, GetState()) + .Times(AnyNumber()) + .WillRepeatedly(Return(eSTATE_PLAYING)); + EXPECT_CALL(*stream->videoTrack, GetBufferedDuration()) + .Times(AnyNumber()) + .WillRepeatedly(Return(0.0)); + EXPECT_CALL(*g_mockPrivateInstanceAAMP, IsSinkCacheEmpty(eMEDIATYPE_VIDEO)) + .Times(AtLeast(1)) + .WillRepeatedly(Return(false)); + EXPECT_CALL(*g_mockPrivateInstanceAAMP, TrackDownloadsAreEnabled(eMEDIATYPE_VIDEO)) + .Times(AnyNumber()) + .WillRepeatedly(Return(true)); + + std::atomic bufferingEnded{false}; + EXPECT_CALL(*g_mockPrivateInstanceAAMP, SetBufferingState(false)) + .Times(AtLeast(1)) + .WillOnce(::testing::Invoke([&](bool){ bufferingEnded.store(true); })); + + monitor->Start(); + auto start = std::chrono::steady_clock::now(); + while (!bufferingEnded.load() && (std::chrono::steady_clock::now() - start) < std::chrono::milliseconds(1000)) + { + std::this_thread::yield(); + } + monitor->Stop(); + EXPECT_TRUE(bufferingEnded.load()); +} + +TEST_F(AampUnderflowMonitor_FunctionalTests, RunExitsWhenPlayerStopped) +{ + EXPECT_CALL(*g_mockPrivateInstanceAAMP, GetState()) + .Times(AnyNumber()) + .WillRepeatedly(Return(eSTATE_STOPPED)); + + monitor->Start(); + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + EXPECT_FALSE(monitor->IsRunning()); + monitor->Stop(); +} diff --git a/test/utests/tests/CMakeLists.txt b/test/utests/tests/CMakeLists.txt index d75154ddf..74190f553 100644 --- a/test/utests/tests/CMakeLists.txt +++ b/test/utests/tests/CMakeLists.txt @@ -17,6 +17,7 @@ include(GoogleTest) +add_subdirectory(AampUnderflowMonitor) add_subdirectory(NetworkBandwidthEstimator) add_subdirectory(JsBindingTests) add_subdirectory(AampEventTests) diff --git a/test/utests/tests/StreamAbstractionAAMP/CMakeLists.txt b/test/utests/tests/StreamAbstractionAAMP/CMakeLists.txt index f3370e6db..e4d9024d1 100644 --- a/test/utests/tests/StreamAbstractionAAMP/CMakeLists.txt +++ b/test/utests/tests/StreamAbstractionAAMP/CMakeLists.txt @@ -37,7 +37,7 @@ set (DASH_PARSER_SOURCES ${AAMP_ROOT}/dash/xml/DomDocument.cpp set(TEST_SOURCES FunctionalTests.cpp StreamAbstractionAAMP.cpp) -set(AAMP_SOURCES ${AAMP_ROOT}/streamabstraction.cpp ${AAMP_ROOT}/CachedFragment.cpp ) +set(AAMP_SOURCES ${AAMP_ROOT}/streamabstraction.cpp ${AAMP_ROOT}/CachedFragment.cpp) add_executable(${EXEC_NAME} ${TEST_SOURCES} diff --git a/test/utests/tests/StreamAbstractionAAMP/FunctionalTests.cpp b/test/utests/tests/StreamAbstractionAAMP/FunctionalTests.cpp index f3ff6d441..e7973c80a 100644 --- a/test/utests/tests/StreamAbstractionAAMP/FunctionalTests.cpp +++ b/test/utests/tests/StreamAbstractionAAMP/FunctionalTests.cpp @@ -25,6 +25,7 @@ #include "AampLogManager.h" #include "MediaStreamContext.h" #include "MockAampConfig.h" +#include "MockAampUnderflowMonitor.h" #include "MockAampUtils.h" #include "MockPrivateInstanceAAMP.h" #include "MockMediaTrack.h" @@ -96,12 +97,12 @@ class StreamAbstractionAAMP_Test : public ::testing::Test { mTrackState = state; } - MOCK_METHOD(void, clearFirstPTS, (), (override)); }; PrivateInstanceAAMP *mPrivateInstanceAAMP; + AampUnderflowMonitor *mUnderflowMonitor; TestableStreamAbstractionAAMP *mStreamAbstractionAAMP; AampConfig *mConfig; std::shared_ptr mMockMediaProcessor; @@ -119,10 +120,19 @@ class StreamAbstractionAAMP_Test : public ::testing::Test g_mockPrivateInstanceAAMP = new NiceMock(); } + EXPECT_CALL(*g_mockAampConfig, GetConfigValue(eAAMPConfig_EnableAampUnderflowMonitor)) + .WillRepeatedly(Return(true)); mPrivateInstanceAAMP = new PrivateInstanceAAMP(mConfig); + + mPrivateInstanceAAMP->mMediaFormat = eMEDIAFORMAT_DASH; // Underflow monitor only enabled for DASH in StreamAbstractionAAMP mStreamAbstractionAAMP = new TestableStreamAbstractionAAMP(mPrivateInstanceAAMP); - // For initialisation of mediatrack + if(g_mockAampUnderflowMonitor == nullptr) + { + g_mockAampUnderflowMonitor = new NiceMock(); + } + + // For initialisation of mediatrack EXPECT_CALL(*g_mockAampConfig, GetConfigValue(eAAMPConfig_MaxFragmentCached)) .Times(AnyNumber()) .WillRepeatedly(Return(0)); @@ -160,6 +170,9 @@ class StreamAbstractionAAMP_Test : public ::testing::Test delete g_mockAampConfig; g_mockAampConfig = nullptr; + delete g_mockAampUnderflowMonitor; + g_mockAampUnderflowMonitor = nullptr; + mMockMediaProcessor.reset(); } }; @@ -233,3 +246,37 @@ TEST_F(StreamAbstractionAAMP_Test, ReinitializeInjection_LLDashChunkModeDisabled mStreamAbstractionAAMP->ReinitializeInjection(test_rate); } + +TEST_F(StreamAbstractionAAMP_Test, StartUnderflowMonitor_NoVideoTrack_DoesNotRun) +{ + EXPECT_CALL(*g_mockAampUnderflowMonitor, Start()).Times(0); + delete mStreamAbstractionAAMP->mMockVideoTrack; + mStreamAbstractionAAMP->mMockVideoTrack = nullptr; + + mStreamAbstractionAAMP->StartUnderflowMonitor(); + EXPECT_FALSE(mStreamAbstractionAAMP->IsUnderflowMonitorRunning()); +} + +TEST_F(StreamAbstractionAAMP_Test, StartAndStopUnderflowMonitor_WrapperApi) +{ + EXPECT_CALL(*g_mockAampUnderflowMonitor, Start()).Times(1); + EXPECT_CALL(*g_mockAampUnderflowMonitor, Stop()).Times(1); + mStreamAbstractionAAMP->StartUnderflowMonitor(); + EXPECT_TRUE(mStreamAbstractionAAMP->IsUnderflowMonitorRunning()); + mStreamAbstractionAAMP->StopUnderflowMonitor(); + EXPECT_FALSE(mStreamAbstractionAAMP->IsUnderflowMonitorRunning()); +} + +TEST_F(StreamAbstractionAAMP_Test, StartUnderflowMonitor_CalledTwice_IsIdempotent) +{ + EXPECT_CALL(*g_mockAampUnderflowMonitor, Start()).Times(2); + EXPECT_CALL(*g_mockAampUnderflowMonitor, Stop()).Times(1); + + mStreamAbstractionAAMP->StartUnderflowMonitor(); + EXPECT_TRUE(mStreamAbstractionAAMP->IsUnderflowMonitorRunning()); + EXPECT_NO_THROW(mStreamAbstractionAAMP->StartUnderflowMonitor()); + EXPECT_TRUE(mStreamAbstractionAAMP->IsUnderflowMonitorRunning()); + + mStreamAbstractionAAMP->StopUnderflowMonitor(); + EXPECT_FALSE(mStreamAbstractionAAMP->IsUnderflowMonitorRunning()); +} From 44d2069c23dc2eb7c7b9d22436d8c3a57937eaa2 Mon Sep 17 00:00:00 2001 From: Philip Stroffolino Date: Sat, 28 Feb 2026 01:45:04 -0500 Subject: [PATCH 2/4] Reason for Change: TeardownStream() stops and deletes mpStreamAbstractionAAMP without stopping the underflow monitor first. Signed-off-by: Philip Stroffolino --- StreamAbstractionAAMP.h | 6 ++++- fragmentcollector_progressive.cpp | 10 -------- test/utests/mocks/MockStreamAbstractionAAMP.h | 2 ++ .../tests/PrivAampTests/PrivAampTests.cpp | 25 +++++++++++++++++++ 4 files changed, 32 insertions(+), 11 deletions(-) diff --git a/StreamAbstractionAAMP.h b/StreamAbstractionAAMP.h index deb4fc0c7..b27775a88 100644 --- a/StreamAbstractionAAMP.h +++ b/StreamAbstractionAAMP.h @@ -1732,9 +1732,13 @@ class StreamAbstractionAAMP : public AampLicenseFetcher /** * @fn StopUnderflowMonitor * @brief Stop UnderflowMonitor Thread. + * @note Must be called before Stop() or before destroying the StreamAbstractionAAMP + * instance. The underflow monitor holds raw pointers back into the stream and + * the AAMP instance; stopping it joins the background thread and prevents any + * use-after-free that would otherwise arise during teardown. * @return void */ - void StopUnderflowMonitor(); + virtual void StopUnderflowMonitor(); /** * @fn IsUnderflowMonitorRunning diff --git a/fragmentcollector_progressive.cpp b/fragmentcollector_progressive.cpp index a0ed72f4c..424137847 100644 --- a/fragmentcollector_progressive.cpp +++ b/fragmentcollector_progressive.cpp @@ -238,16 +238,6 @@ void StreamAbstractionAAMP_PROGRESSIVE::Start(void) { AAMPLOG_ERR("Failed to create FragmentCollector thread : %s", e.what()); } - - // Start underflow monitor after successful initialization and Start() - if (mUnderflowMonitor) - { - StartUnderflowMonitor(); - if (!IsUnderflowMonitorRunning()) - { - AAMPLOG_WARN("UnderflowMonitor did not start; continuing without AampUnderflowMonitor"); - } - } } /** diff --git a/test/utests/mocks/MockStreamAbstractionAAMP.h b/test/utests/mocks/MockStreamAbstractionAAMP.h index 9483eb2fe..c18213599 100644 --- a/test/utests/mocks/MockStreamAbstractionAAMP.h +++ b/test/utests/mocks/MockStreamAbstractionAAMP.h @@ -37,6 +37,8 @@ class MockStreamAbstractionAAMP : public StreamAbstractionAAMP MOCK_METHOD(void, Stop, (bool clearChannelData), (override)); + MOCK_METHOD(void, StopUnderflowMonitor, (), (override)); + MOCK_METHOD(void, GetStreamFormat, (StreamOutputFormat &primaryOutputFormat, StreamOutputFormat &audioOutputFormat, StreamOutputFormat &subtitleOutputFormat), (override)); MOCK_METHOD(double, GetStreamPosition, (), (override)); diff --git a/test/utests/tests/PrivAampTests/PrivAampTests.cpp b/test/utests/tests/PrivAampTests/PrivAampTests.cpp index 70f9fc26a..9b8244db7 100644 --- a/test/utests/tests/PrivAampTests/PrivAampTests.cpp +++ b/test/utests/tests/PrivAampTests/PrivAampTests.cpp @@ -2476,6 +2476,31 @@ TEST_F(PrivAampTests,TeardownStreamTest) p_aamp->TeardownStream(newTune); } +/** + * @brief Verify that TeardownStream() stops the underflow monitor before stopping + * or deleting the StreamAbstractionAAMP object. + * + * The AampUnderflowMonitor holds raw pointers to StreamAbstractionAAMP and + * PrivateInstanceAAMP and accesses them from a background thread. Calling + * StopUnderflowMonitor() joins that thread; only then is it safe to call Stop() + * or free the object via SAFE_DELETE. Reversing the order is a use-after-free. + */ +TEST_F(PrivAampTests, TeardownStream_StopsUnderflowMonitorBeforeStream) +{ + p_aamp->mpStreamAbstractionAAMP = g_mockStreamAbstractionAAMP; + + ::testing::InSequence seq; + // StopUnderflowMonitor() MUST be called before Stop(). + EXPECT_CALL(*g_mockStreamAbstractionAAMP, StopUnderflowMonitor()).Times(1); + EXPECT_CALL(*g_mockStreamAbstractionAAMP, Stop(_)).Times(1); + + p_aamp->TeardownStream(false); + + // mpStreamAbstractionAAMP is set to null / deleted by TeardownStream; restore + // the pointer in the mock so the fixture TearDown does not double-delete. + p_aamp->mpStreamAbstractionAAMP = nullptr; +} + TEST_F(PrivAampTests,TeardownStreamTest_1) { p_aamp->TeardownStream(false); From 859e69eb84509137a37eebdde12a2e8bd3324c22 Mon Sep 17 00:00:00 2001 From: varshnie Date: Mon, 2 Mar 2026 21:23:37 +0530 Subject: [PATCH 3/4] Reason for Change:Addressed the L1 failure and stopped the underflow monitor in streamabstarction stop --- StreamAbstractionAAMP.h | 2 +- fragmentcollector_progressive.cpp | 10 ++++---- priv_aamp.cpp | 2 +- streamabstraction.cpp | 24 ++++++++---------- test/utests/drm/mocks/aampMocks.cpp | 2 +- test/utests/fakes/FakePrivateInstanceAAMP.cpp | 2 +- test/utests/mocks/MockStreamAbstractionAAMP.h | 2 -- .../tests/PrivAampTests/PrivAampTests.cpp | 25 ------------------- .../StreamAbstractionAAMP/FunctionalTests.cpp | 5 ++-- 9 files changed, 22 insertions(+), 52 deletions(-) diff --git a/StreamAbstractionAAMP.h b/StreamAbstractionAAMP.h index b27775a88..3f9a23a44 100644 --- a/StreamAbstractionAAMP.h +++ b/StreamAbstractionAAMP.h @@ -1738,7 +1738,7 @@ class StreamAbstractionAAMP : public AampLicenseFetcher * use-after-free that would otherwise arise during teardown. * @return void */ - virtual void StopUnderflowMonitor(); + void StopUnderflowMonitor(); /** * @fn IsUnderflowMonitorRunning diff --git a/fragmentcollector_progressive.cpp b/fragmentcollector_progressive.cpp index 424137847..ef91e21f8 100644 --- a/fragmentcollector_progressive.cpp +++ b/fragmentcollector_progressive.cpp @@ -246,11 +246,11 @@ void StreamAbstractionAAMP_PROGRESSIVE::Start(void) void StreamAbstractionAAMP_PROGRESSIVE::Stop(bool clearChannelData) { if(fragmentCollectorThreadID.joinable()) - { - aamp->DisableDownloads(); - fragmentCollectorThreadID.join(); - aamp->EnableDownloads(); - } + { + aamp->DisableDownloads(); + fragmentCollectorThreadID.join(); + aamp->EnableDownloads(); + } } /** diff --git a/priv_aamp.cpp b/priv_aamp.cpp index 2cdd6461c..06bf6384b 100644 --- a/priv_aamp.cpp +++ b/priv_aamp.cpp @@ -3255,7 +3255,7 @@ void PrivateInstanceAAMP::SendBufferChangeEvent(bool bufferingStart) SendEvent(e,AAMP_EVENT_ASYNC_MODE); // If rebuffering started, notify latency monitor to relax the latency band - if (bufferingStopped) + if (bufferingStart && mLatencyMonitor) { mLatencyMonitor->OnRebufferingStart(); } diff --git a/streamabstraction.cpp b/streamabstraction.cpp index 88169e161..7c76b9122 100644 --- a/streamabstraction.cpp +++ b/streamabstraction.cpp @@ -3023,22 +3023,21 @@ void StreamAbstractionAAMP::StartUnderflowMonitor() { if (!GetMediaTrack(eTRACK_VIDEO)) { - if (mUnderflowMonitor) + // No video track: keep existing monitor instance (if any), just skip starting it + AAMPLOG_INFO("StartUnderflowMonitor: video track unavailable; not starting AampUnderflowMonitor"); + return; + } + if (mUnderflowMonitor) + { + if (!mUnderflowMonitor->IsRunning()) { - // No video track: delete the monitor to avoid wasted resources - mUnderflowMonitor.reset(); - AAMPLOG_INFO("StartUnderflowMonitor: no video track; deleted AampUnderflowMonitor"); + mUnderflowMonitor->Start(); + AAMPLOG_INFO("Started AampUnderflowMonitor for video"); } else { - AAMPLOG_WARN("StartUnderflowMonitor: video track unavailable"); + AAMPLOG_WARN("StartUnderflowMonitor: AampUnderflowMonitor already running for video"); } - return; - } - if (mUnderflowMonitor) - { - mUnderflowMonitor->Start(); - AAMPLOG_INFO("Started AampUnderflowMonitor for video"); } } @@ -3047,8 +3046,7 @@ void StreamAbstractionAAMP::StopUnderflowMonitor() if (mUnderflowMonitor) { mUnderflowMonitor->Stop(); - mUnderflowMonitor.reset(); - AAMPLOG_INFO("Stopped AampUnderflowMonitor for video; resetting monitor instance"); + AAMPLOG_INFO("Stopped AampUnderflowMonitor for video"); } } diff --git a/test/utests/drm/mocks/aampMocks.cpp b/test/utests/drm/mocks/aampMocks.cpp index 2f652b0eb..aaba359d4 100644 --- a/test/utests/drm/mocks/aampMocks.cpp +++ b/test/utests/drm/mocks/aampMocks.cpp @@ -1309,7 +1309,7 @@ bool PrivateInstanceAAMP::PausePipeline(bool pause, bool forceStopGstreamerPreBu return false; } -void PrivateInstanceAAMP::SendBufferChangeEvent(bool bufferingStopped) +void PrivateInstanceAAMP::SendBufferChangeEvent(bool bufferingStart) { } diff --git a/test/utests/fakes/FakePrivateInstanceAAMP.cpp b/test/utests/fakes/FakePrivateInstanceAAMP.cpp index b79498ac4..d6d59433b 100644 --- a/test/utests/fakes/FakePrivateInstanceAAMP.cpp +++ b/test/utests/fakes/FakePrivateInstanceAAMP.cpp @@ -1604,7 +1604,7 @@ bool PrivateInstanceAAMP::PausePipeline(bool pause, bool forceStopGstreamerPreBu return false; } -void PrivateInstanceAAMP::SendBufferChangeEvent(bool bufferingStopped) +void PrivateInstanceAAMP::SendBufferChangeEvent(bool bufferingStart) { } diff --git a/test/utests/mocks/MockStreamAbstractionAAMP.h b/test/utests/mocks/MockStreamAbstractionAAMP.h index c18213599..9483eb2fe 100644 --- a/test/utests/mocks/MockStreamAbstractionAAMP.h +++ b/test/utests/mocks/MockStreamAbstractionAAMP.h @@ -37,8 +37,6 @@ class MockStreamAbstractionAAMP : public StreamAbstractionAAMP MOCK_METHOD(void, Stop, (bool clearChannelData), (override)); - MOCK_METHOD(void, StopUnderflowMonitor, (), (override)); - MOCK_METHOD(void, GetStreamFormat, (StreamOutputFormat &primaryOutputFormat, StreamOutputFormat &audioOutputFormat, StreamOutputFormat &subtitleOutputFormat), (override)); MOCK_METHOD(double, GetStreamPosition, (), (override)); diff --git a/test/utests/tests/PrivAampTests/PrivAampTests.cpp b/test/utests/tests/PrivAampTests/PrivAampTests.cpp index 9b8244db7..70f9fc26a 100644 --- a/test/utests/tests/PrivAampTests/PrivAampTests.cpp +++ b/test/utests/tests/PrivAampTests/PrivAampTests.cpp @@ -2476,31 +2476,6 @@ TEST_F(PrivAampTests,TeardownStreamTest) p_aamp->TeardownStream(newTune); } -/** - * @brief Verify that TeardownStream() stops the underflow monitor before stopping - * or deleting the StreamAbstractionAAMP object. - * - * The AampUnderflowMonitor holds raw pointers to StreamAbstractionAAMP and - * PrivateInstanceAAMP and accesses them from a background thread. Calling - * StopUnderflowMonitor() joins that thread; only then is it safe to call Stop() - * or free the object via SAFE_DELETE. Reversing the order is a use-after-free. - */ -TEST_F(PrivAampTests, TeardownStream_StopsUnderflowMonitorBeforeStream) -{ - p_aamp->mpStreamAbstractionAAMP = g_mockStreamAbstractionAAMP; - - ::testing::InSequence seq; - // StopUnderflowMonitor() MUST be called before Stop(). - EXPECT_CALL(*g_mockStreamAbstractionAAMP, StopUnderflowMonitor()).Times(1); - EXPECT_CALL(*g_mockStreamAbstractionAAMP, Stop(_)).Times(1); - - p_aamp->TeardownStream(false); - - // mpStreamAbstractionAAMP is set to null / deleted by TeardownStream; restore - // the pointer in the mock so the fixture TearDown does not double-delete. - p_aamp->mpStreamAbstractionAAMP = nullptr; -} - TEST_F(PrivAampTests,TeardownStreamTest_1) { p_aamp->TeardownStream(false); diff --git a/test/utests/tests/StreamAbstractionAAMP/FunctionalTests.cpp b/test/utests/tests/StreamAbstractionAAMP/FunctionalTests.cpp index e7973c80a..b6391c098 100644 --- a/test/utests/tests/StreamAbstractionAAMP/FunctionalTests.cpp +++ b/test/utests/tests/StreamAbstractionAAMP/FunctionalTests.cpp @@ -102,7 +102,6 @@ class StreamAbstractionAAMP_Test : public ::testing::Test }; PrivateInstanceAAMP *mPrivateInstanceAAMP; - AampUnderflowMonitor *mUnderflowMonitor; TestableStreamAbstractionAAMP *mStreamAbstractionAAMP; AampConfig *mConfig; std::shared_ptr mMockMediaProcessor; @@ -267,9 +266,9 @@ TEST_F(StreamAbstractionAAMP_Test, StartAndStopUnderflowMonitor_WrapperApi) EXPECT_FALSE(mStreamAbstractionAAMP->IsUnderflowMonitorRunning()); } -TEST_F(StreamAbstractionAAMP_Test, StartUnderflowMonitor_CalledTwice_IsIdempotent) +TEST_F(StreamAbstractionAAMP_Test, StartUnderflowMonitor_CalledTwice_StartsOnceAndRemainsRunning) { - EXPECT_CALL(*g_mockAampUnderflowMonitor, Start()).Times(2); + EXPECT_CALL(*g_mockAampUnderflowMonitor, Start()).Times(1); EXPECT_CALL(*g_mockAampUnderflowMonitor, Stop()).Times(1); mStreamAbstractionAAMP->StartUnderflowMonitor(); From b50643f482c8033deff78abe080a38f5143feec8 Mon Sep 17 00:00:00 2001 From: varshnie Date: Mon, 23 Mar 2026 17:38:36 +0530 Subject: [PATCH 4/4] VPLAY-12682:SoC-independent underflow detection - follow-up improvements Reason for change:AampUnderflowMonitor Optimization Risks: Medium Signed-off-by: varshnie --- AampDefine.h | 4 + AampUnderflowMonitor.cpp | 189 ++++++------------ AampUnderflowMonitor.h | 53 +++-- priv_aamp.cpp | 10 + streamabstraction.cpp | 2 +- .../AampUnderflowMonitor/FunctionalTests.cpp | 14 +- .../tests/PrivAampTests/PrivAampTests.cpp | 2 + .../StreamAbstractionAAMP/FunctionalTests.cpp | 7 +- 8 files changed, 127 insertions(+), 154 deletions(-) diff --git a/AampDefine.h b/AampDefine.h index 6e9f95b6c..b4912c03d 100644 --- a/AampDefine.h +++ b/AampDefine.h @@ -154,8 +154,12 @@ #define DEFAULT_UNDERFLOW_DETECT_THRESHOLD_SEC 0.0 /**< Threshold to detect underflow in seconds */ #define DEFAULT_UNDERFLOW_RESUME_THRESHOLD_SEC 1.0 /**< Threshold to resume from underflow in seconds */ + #define DEFAULT_UNDERFLOW_LOW_BUFFER_SEC 5.0 /**< Low buffer threshold in seconds */ +#define UNDERFLOW_LOW_BUFFER_SEC_FOR_LLD 1.0 /**< Low buffer threshold in seconds for LLD stream */ + #define DEFAULT_UNDERFLOW_HIGH_BUFFER_SEC 10.0 /**< High buffer threshold in seconds */ +#define UNDERFLOW_HIGH_BUFFER_SEC_FOR_LLD 4.0 /**< High buffer threshold in seconds for LLD stream */ #define DEFAULT_BUFFER_LEVEL_TO_ENABLE_LATENCY_SEC 0.0 /*< Default is 0.0 means latency correction is enabled at all buffer values */ #define DEFAULT_REBUFFER_LATENCY_STEP_SEC 1.0 /*< Step value for latency increase when rebuffering occurs in seconds */ diff --git a/AampUnderflowMonitor.cpp b/AampUnderflowMonitor.cpp index 869f2c342..68a556326 100644 --- a/AampUnderflowMonitor.cpp +++ b/AampUnderflowMonitor.cpp @@ -48,60 +48,51 @@ AampUnderflowMonitor::~AampUnderflowMonitor() { Stop(); } -void AampUnderflowMonitor::Start() { - // Use unique_lock to allow unlock around join operations - std::unique_lock lock(mMutex); +void AampUnderflowMonitor::Start() +{ + std::lock_guard lock(mStartStopMutex); - // If a previous thread exists but is not running, join it before starting a new - if (mThread.joinable()) + if (mUnderflowMonitorThread.joinable()) { if (mRunning.load()) { AAMPLOG_INFO("AampUnderflowMonitor already running; skipping start"); return; } - lock.unlock(); - mThread.join(); + mUnderflowMonitorThread.join(); AAMPLOG_INFO("AampUnderflowMonitor previous thread joined before restart"); - lock.lock(); } try { + { + std::lock_guard sleepLock(mSleepMutex); + mWakeupSignalled = false; + } mRunning.store(true); - mThread = std::thread(&AampUnderflowMonitor::Run, this); - AAMPLOG_INFO("AampUnderflowMonitor thread created [%zx]", GetPrintableThreadID(mThread)); + mUnderflowMonitorThread = std::thread(&AampUnderflowMonitor::Run, this); + AAMPLOG_INFO("AampUnderflowMonitor thread created [%zx]", GetPrintableThreadID(mUnderflowMonitorThread)); } catch(const std::exception& e) { mRunning.store(false); AAMPLOG_WARN("Failed to create AampUnderflowMonitor thread : %s", e.what()); - return; - } - - // If the thread exited immediately (e.g., due to player state), ensure we join it to avoid a dangling joinable thread - if (!mRunning.load() && mThread.joinable()) - { - lock.unlock(); - mThread.join(); - AAMPLOG_WARN("AampUnderflowMonitor thread exited immediately after start; joined"); } } void AampUnderflowMonitor::Stop() { - // Use unique_lock so we can release the mutex before joining - std::unique_lock lock(mMutex); - // Signal thread to stop + std::lock_guard lock(mStartStopMutex); mRunning.store(false); - - // If a thread is joinable, release the lock before joining to avoid deadlock - if (mThread.joinable()) { - lock.unlock(); - mThread.join(); + std::lock_guard sleepLock(mSleepMutex); + mWakeupSignalled = true; + mSleepCv.notify_all(); + } + if (mUnderflowMonitorThread.joinable()) + { + mUnderflowMonitorThread.join(); AAMPLOG_INFO("AampUnderflowMonitor thread joined"); - lock.lock(); } } @@ -116,114 +107,49 @@ void AampUnderflowMonitor::Run() const int kMediumBufferPollMs = mAamp->mConfig->GetConfigValue(eAAMPConfig_UnderflowMediumBufferPollMs); const int kHighBufferPollMs = mAamp->mConfig->GetConfigValue(eAAMPConfig_UnderflowHighBufferPollMs); - // Wait until playback enters PLAYING state or underflow becomes active; exit if playback stops - while (mRunning.load()) { - AAMPPlayerState state; - bool shouldBreak = false; - bool underflowStatus = false; - - { - std::lock_guard lock(mMutex); - state = mAamp->GetState(); - if (state == eSTATE_STOPPED || state == eSTATE_RELEASED || state == eSTATE_ERROR) { - mRunning.store(false); - return; - } - underflowStatus = mAamp->GetBufUnderFlowStatus(); - shouldBreak = (state == eSTATE_PLAYING || underflowStatus); - } - - if (shouldBreak) { - break; - } - mAamp->interruptibleMsSleep(100); - } + while (mRunning.load()) + { + AAMPPlayerState playerState = mAamp->GetState(); + bool underflowActive = mAamp->GetBufUnderFlowStatus(); + double bufferedTimeSec = 0.0; - while (mRunning.load()) { - // Check player state and underflow status under mutex - bool underflowActive; - AAMPPlayerState playerState; - float currentRate; - double bufferedTimeSec; - + const bool isPlayingOrUnderflow = (playerState == eSTATE_PLAYING) || underflowActive; + if (isPlayingOrUnderflow) { - std::lock_guard lock(mMutex); - - underflowActive = mAamp->GetBufUnderFlowStatus(); - playerState = mAamp->GetState(); - // Exit when playback transitions to stopped/released/error/idle - if (playerState == eSTATE_STOPPED || playerState == eSTATE_RELEASED || playerState == eSTATE_ERROR || playerState == eSTATE_IDLE) { - break; - } - - // Skip buffer-based underflow checks during trickplay or seeking - currentRate = mAamp->rate; - - // Query buffered duration once and reuse for detection and sleep cadence bufferedTimeSec = mStream->GetBufferedVideoDurationSec(); if (bufferedTimeSec < 0.0) bufferedTimeSec = 0.0; - } - - const bool inPlayOrUnderflow = (playerState == eSTATE_PLAYING) || underflowActive; - const bool isTrickplay = (currentRate != AAMP_NORMAL_PLAY_RATE && currentRate != AAMP_SLOWMOTION_RATE && currentRate != AAMP_RATE_PAUSE); - const bool isSeekingState = (playerState == eSTATE_SEEKING); - - if (inPlayOrUnderflow) { - // Video underflow detection/resume (query under mutex) - bool trackDownloadsEnabled; - bool sinkCacheEmpty; - - { - std::lock_guard lock(mMutex); - trackDownloadsEnabled = mAamp->TrackDownloadsAreEnabled(eMEDIATYPE_VIDEO); - sinkCacheEmpty = mAamp->IsSinkCacheEmpty(eMEDIATYPE_VIDEO); - } - // Only evaluate buffer threshold when not in trickplay/seeking; still honor sink cache emptiness + const float currentRate = mAamp->rate; + const bool isTrickplay = (currentRate != AAMP_NORMAL_PLAY_RATE && currentRate != AAMP_SLOWMOTION_RATE && currentRate != AAMP_RATE_PAUSE); + const bool isSeekingState = (playerState == eSTATE_SEEKING); + + const bool sinkCacheEmpty = mAamp->IsSinkCacheEmpty(eMEDIATYPE_VIDEO); const bool allowBufferCheck = (!isTrickplay && !isSeekingState); - if (((allowBufferCheck && bufferedTimeSec <= kUnderflowDetectThresholdSec && trackDownloadsEnabled)) || sinkCacheEmpty) + + // Detection: only when not in trickplay/seeking + if (allowBufferCheck && (bufferedTimeSec <= kUnderflowDetectThresholdSec || sinkCacheEmpty)) { if (!underflowActive) { - AAMPLOG_INFO("[video] underflow detected. buffered=%.3f cacheEmpty=%d (rate=%.2f, trickplay=%d, seeking=%d)", bufferedTimeSec, (int)sinkCacheEmpty, currentRate, (int)isTrickplay, (int)isSeekingState); - - std::lock_guard lock(mMutex); + AAMPLOG_INFO("[video] underflow detected. buffered=%.3f cacheEmpty=%d (rate=%.2f)", bufferedTimeSec, (int)sinkCacheEmpty, currentRate); mAamp->SetBufferingState(true); - PlaybackErrorType errorType = eGST_ERROR_UNDERFLOW; - mAamp->SendAnomalyEvent(ANOMALY_WARNING, "%s %s", GetMediaTypeName(eMEDIATYPE_VIDEO), mAamp->getStringForPlaybackError(errorType)); - } - else - { - if (!trackDownloadsEnabled && sinkCacheEmpty) - { - AAMPLOG_WARN("[video] downloads blocked with empty cache during underflow; resuming"); - std::lock_guard lock(mMutex); - mAamp->ResumeTrackDownloads(eMEDIATYPE_VIDEO); - } } } - else + else if (!allowBufferCheck) { - if (!allowBufferCheck) - { - // Informational: buffer-based underflow checks suppressed during trickplay/seeking - AAMPLOG_TRACE("[video] skipping buffer-based underflow check (rate=%.2f, trickplay=%d, seeking=%d). cacheEmpty=%d buffered=%.3f", - currentRate, (int)isTrickplay, (int)isSeekingState, (int)sinkCacheEmpty, bufferedTimeSec); - } - - bool pipelinePaused = false; - { - std::lock_guard lock(mMutex); - if (!mAamp) return; - pipelinePaused = mAamp->mSinkPaused.load(); - } - - if (underflowActive && pipelinePaused) + AAMPLOG_TRACE("[video] skipping underflow detection (rate=%.2f, trickplay=%d, seeking=%d). buffered=%.3f", + currentRate, (int)isTrickplay, (int)isSeekingState, bufferedTimeSec); + } + + // Resume: always evaluate when underflow is active so it can clear during trickplay/seeking + if (underflowActive) + { + const bool pipelinePaused = mAamp->mSinkPaused.load(); + if (pipelinePaused) { if (bufferedTimeSec >= kUnderflowResumeThresholdSec && !sinkCacheEmpty) { AAMPLOG_INFO("[video] underflow ended. buffered=%.3f cacheEmpty=%d", bufferedTimeSec, (int)sinkCacheEmpty); - std::lock_guard lock(mMutex); mAamp->SetBufferingState(false); } else @@ -231,21 +157,26 @@ void AampUnderflowMonitor::Run() AAMPLOG_INFO("[video] waiting to end underflow. buffered=%.3f cacheEmpty=%d", bufferedTimeSec, (int)sinkCacheEmpty); } } - else if (underflowActive && !trackDownloadsEnabled && sinkCacheEmpty) - { - AAMPLOG_WARN("[video] underflow ongoing, downloads blocked and cache empty; resuming track downloads"); - mAamp->ResumeTrackDownloads(eMEDIATYPE_VIDEO); - } } // Audio underflow is not handled currently as we are aligning with the existing behavior } - // Choose sleep interval based on buffer level (branchless style) - const int sleepMs = (bufferedTimeSec < kLowBufferSec) ? kLowBufferPollMs - : (bufferedTimeSec >= kHighBufferSec) ? kHighBufferPollMs - : kMediumBufferPollMs; - mAamp->interruptibleMsSleep(sleepMs); + const int sleepMs = (bufferedTimeSec < kLowBufferSec) ? kLowBufferPollMs + : (bufferedTimeSec >= kHighBufferSec) ? kHighBufferPollMs + : kMediumBufferPollMs; + WaitMs(sleepMs); } mRunning.store(false); } +void AampUnderflowMonitor::WaitMs(int ms) +{ + std::unique_lock lk(mSleepMutex); + mSleepCv.wait_for(lk, + std::chrono::milliseconds(ms), + [this]() { + return mWakeupSignalled || !mRunning.load(); + }); + mWakeupSignalled = false; +} + diff --git a/AampUnderflowMonitor.h b/AampUnderflowMonitor.h index 9d45fdf13..82209605d 100644 --- a/AampUnderflowMonitor.h +++ b/AampUnderflowMonitor.h @@ -30,29 +30,45 @@ #include #include #include +#include +#include class StreamAbstractionAAMP; class PrivateInstanceAAMP; +/** + * @class AampUnderflowMonitor + * @brief Monitors video buffer underflow and drives coordinated pipeline resume. + * + * ## Purpose + * Polls the buffered video duration at a configurable interval and sets/clears + * the buffering state on the AAMP instance when underflow thresholds are crossed. + * + * ## State machine + * @code + * stopped --Start()--> running --Stop()--> stopped + * @endcode + * + * ## Thread safety + * Start() and Stop() are serialised by mStartStopMutex and may safely be + * called from any thread, including concurrently. + * Stop() blocks until the monitor thread has exited. + */ class AampUnderflowMonitor { public: /** - * @brief Construct an `AampUnderflowMonitor`. - * @param[in] stream Stream abstraction used to query buffered video duration - * and playback state relevant to underflow detection. - * @param[in] aamp AAMP instance used for coordination (downloads, buffering - * state) and event emission. - * @note Ownership: The caller retains ownership of both `stream` and `aamp`. - * All pointer accesses are protected by an internal mutex to prevent - * TOCTOU races during shutdown. - * @note Thread Safety: `Stop()` safely terminates the monitoring thread and - * prevents further pointer access. Call `Stop()` before destroying the - * `StreamAbstractionAAMP` or `PrivateInstanceAAMP` instances. + * @brief Constructor. + * @param[in] stream Stream abstraction used to query buffered video duration. + * Must remain valid until Stop() returns. + * (The monitor does not take ownership of the pointer.) + * @param[in] aamp AAMP instance used to query state and control buffering. + * Must remain valid until Stop() returns. + * (The monitor does not take ownership of the pointer.) */ AampUnderflowMonitor(StreamAbstractionAAMP* stream, PrivateInstanceAAMP* aamp); /** - * @brief Destructor. Ensures monitoring has been stopped. + * @brief Destructor. Calls Stop() to ensure the monitor thread exits. */ ~AampUnderflowMonitor(); @@ -81,11 +97,20 @@ class AampUnderflowMonitor { */ void Run(); + /** + * @brief Sleep for up to @p ms milliseconds, returning early if Stop() signals. + * @param[in] ms Maximum wait duration in milliseconds. + */ + void WaitMs(int ms); + StreamAbstractionAAMP* mStream; /** Stream abstraction used to query buffered duration and playback state. */ PrivateInstanceAAMP* mAamp; /** AAMP instance used to emit events, control downloads, and query state. */ - std::thread mThread; /** Background thread that performs underflow monitoring. */ + std::thread mUnderflowMonitorThread; /** Background thread that performs underflow monitoring. */ std::atomic mRunning{false}; /** Atomic running flag indicating thread active state. */ - std::mutex mMutex; /** Protects pointer access in Run() and serializes Start/Stop. */ + std::mutex mStartStopMutex; /** Serializes concurrent Start/Stop calls. */ + std::mutex mSleepMutex; /** Guards mSleepCv waits inside Run(). */ + std::condition_variable mSleepCv; /** Signalled by Stop() to unblock WaitMs() immediately. */ + bool mWakeupSignalled{false}; /** Explicit wakeup flag, set under mSleepMutex before notify. */ }; #endif // AAMP_UNDERFLOW_MONITOR_H diff --git a/priv_aamp.cpp b/priv_aamp.cpp index 06bf6384b..602ab1d64 100644 --- a/priv_aamp.cpp +++ b/priv_aamp.cpp @@ -3246,6 +3246,14 @@ void PrivateInstanceAAMP::SendBufferChangeEvent(bool bufferingStart) BufferingChangedEventPtr e = std::make_shared(bufferAvailable, GetSessionId()); SetBufUnderFlowStatus(bufferingStart); + if (bufferingStart) + { + SendAnomalyEvent(ANOMALY_WARNING, "Buffering started"); + } + else + { + SendAnomalyEvent(ANOMALY_WARNING, "Buffering stopped"); + } AAMPLOG_INFO("PrivateInstanceAAMP: Sending Buffer Change event status (Buffering): %s", (e->buffering() ? "End": "Start")); #ifdef AAMP_TELEMETRY_SUPPORT AAMPTelemetry2 at2(mAppName); @@ -14648,6 +14656,8 @@ void PrivateInstanceAAMP::SetLLDashChunkMode(bool enable) if(enable) { mMPDDownloaderInstance->SetNetworkTimeout(MANIFEST_TIMEOUT_FOR_LLD); + SETCONFIGVALUE_PRIV(AAMP_TUNE_SETTING,eAAMPConfig_UnderflowLowBufferSec,UNDERFLOW_LOW_BUFFER_SEC_FOR_LLD); + SETCONFIGVALUE_PRIV(AAMP_TUNE_SETTING,eAAMPConfig_UnderflowHighBufferSec,UNDERFLOW_HIGH_BUFFER_SEC_FOR_LLD); SETCONFIGVALUE_PRIV(AAMP_TUNE_SETTING,eAAMPConfig_ManifestTimeout,MANIFEST_TIMEOUT_FOR_LLD); SETCONFIGVALUE_PRIV(AAMP_TUNE_SETTING,eAAMPConfig_MinABRNWBufferRampDown,AAMP_LOW_BUFFER_BEFORE_RAMPDOWN_FOR_LLD); SETCONFIGVALUE_PRIV(AAMP_TUNE_SETTING,eAAMPConfig_MaxABRNWBufferRampUp,AAMP_HIGH_BUFFER_BEFORE_RAMPUP_FOR_LLD); diff --git a/streamabstraction.cpp b/streamabstraction.cpp index 7c76b9122..4aa22cf4b 100644 --- a/streamabstraction.cpp +++ b/streamabstraction.cpp @@ -2183,7 +2183,7 @@ StreamAbstractionAAMP::StreamAbstractionAAMP(PrivateInstanceAAMP* aamp, id3_call if(GETCONFIGVALUE(eAAMPConfig_EnableAampUnderflowMonitor)) { - if(!mUnderflowMonitor) + if(!mUnderflowMonitor && (aamp->rate == AAMP_NORMAL_PLAY_RATE)) { try { diff --git a/test/utests/tests/AampUnderflowMonitor/FunctionalTests.cpp b/test/utests/tests/AampUnderflowMonitor/FunctionalTests.cpp index fb4d0ded6..1f66441fb 100644 --- a/test/utests/tests/AampUnderflowMonitor/FunctionalTests.cpp +++ b/test/utests/tests/AampUnderflowMonitor/FunctionalTests.cpp @@ -156,8 +156,6 @@ TEST_F(AampUnderflowMonitor_FunctionalTests, RunDetectsUnderflowByBufferThreshol EXPECT_CALL(*stream->videoTrack, GetBufferedDuration()) .Times(AnyNumber()) .WillRepeatedly(Return(0.0)); - EXPECT_CALL(*g_mockPrivateInstanceAAMP, TrackDownloadsAreEnabled(eMEDIATYPE_VIDEO)) - .WillRepeatedly(Return(true)); EXPECT_CALL(*g_mockPrivateInstanceAAMP, IsSinkCacheEmpty(eMEDIATYPE_VIDEO)) .WillRepeatedly(Return(false)); @@ -193,9 +191,6 @@ TEST_F(AampUnderflowMonitor_FunctionalTests, RunEndsUnderflowDuringTrickplayWhen EXPECT_CALL(*g_mockPrivateInstanceAAMP, IsSinkCacheEmpty(eMEDIATYPE_VIDEO)) .Times(AtLeast(1)) .WillRepeatedly(Return(false)); - EXPECT_CALL(*g_mockPrivateInstanceAAMP, TrackDownloadsAreEnabled(eMEDIATYPE_VIDEO)) - .Times(AnyNumber()) - .WillRepeatedly(Return(true)); std::atomic bufferingEnded{false}; EXPECT_CALL(*g_mockPrivateInstanceAAMP, SetBufferingState(false)) @@ -212,14 +207,19 @@ TEST_F(AampUnderflowMonitor_FunctionalTests, RunEndsUnderflowDuringTrickplayWhen EXPECT_TRUE(bufferingEnded.load()); } -TEST_F(AampUnderflowMonitor_FunctionalTests, RunExitsWhenPlayerStopped) +TEST_F(AampUnderflowMonitor_FunctionalTests, RunKeepsPollingWhenPlayerStoppedUntilExternalStop) { + // Monitor does not self-exit on eSTATE_STOPPED — it relies entirely on + // Stop() being called externally (via privaamp stop or destructor). EXPECT_CALL(*g_mockPrivateInstanceAAMP, GetState()) .Times(AnyNumber()) .WillRepeatedly(Return(eSTATE_STOPPED)); monitor->Start(); + EXPECT_TRUE(monitor->IsRunning()); std::this_thread::sleep_for(std::chrono::milliseconds(50)); - EXPECT_FALSE(monitor->IsRunning()); + EXPECT_TRUE(monitor->IsRunning()); + monitor->Stop(); + EXPECT_FALSE(monitor->IsRunning()); } diff --git a/test/utests/tests/PrivAampTests/PrivAampTests.cpp b/test/utests/tests/PrivAampTests/PrivAampTests.cpp index 70f9fc26a..89e477928 100644 --- a/test/utests/tests/PrivAampTests/PrivAampTests.cpp +++ b/test/utests/tests/PrivAampTests/PrivAampTests.cpp @@ -5077,6 +5077,8 @@ TEST_F(PrivAampPrivTests,SetLLDashChunkModeTrueTest) int fragment_duration = 0; EXPECT_CALL(*g_mockAampConfig, SetConfigValue(eAAMPConfig_ManifestTimeout,MANIFEST_TIMEOUT_FOR_LLD)); + EXPECT_CALL(*g_mockAampConfig, SetConfigValue(eAAMPConfig_UnderflowLowBufferSec,UNDERFLOW_LOW_BUFFER_SEC_FOR_LLD)); + EXPECT_CALL(*g_mockAampConfig, SetConfigValue(eAAMPConfig_UnderflowHighBufferSec,UNDERFLOW_HIGH_BUFFER_SEC_FOR_LLD)); EXPECT_CALL(*g_mockAampConfig, SetConfigValue(eAAMPConfig_MinABRNWBufferRampDown,AAMP_LOW_BUFFER_BEFORE_RAMPDOWN_FOR_LLD)); EXPECT_CALL(*g_mockAampConfig, SetConfigValue(eAAMPConfig_MaxABRNWBufferRampUp,AAMP_HIGH_BUFFER_BEFORE_RAMPUP_FOR_LLD)); EXPECT_CALL(*g_mockAampConfig, SetConfigValue(eAAMPConfig_CurlDownloadStartTimeout,fragment_duration)); diff --git a/test/utests/tests/StreamAbstractionAAMP/FunctionalTests.cpp b/test/utests/tests/StreamAbstractionAAMP/FunctionalTests.cpp index b6391c098..2114b90f3 100644 --- a/test/utests/tests/StreamAbstractionAAMP/FunctionalTests.cpp +++ b/test/utests/tests/StreamAbstractionAAMP/FunctionalTests.cpp @@ -123,14 +123,15 @@ class StreamAbstractionAAMP_Test : public ::testing::Test .WillRepeatedly(Return(true)); mPrivateInstanceAAMP = new PrivateInstanceAAMP(mConfig); - mPrivateInstanceAAMP->mMediaFormat = eMEDIAFORMAT_DASH; // Underflow monitor only enabled for DASH in StreamAbstractionAAMP - mStreamAbstractionAAMP = new TestableStreamAbstractionAAMP(mPrivateInstanceAAMP); - if(g_mockAampUnderflowMonitor == nullptr) { g_mockAampUnderflowMonitor = new NiceMock(); } + mPrivateInstanceAAMP->rate = AAMP_NORMAL_PLAY_RATE; // Required: constructor guards on rate == AAMP_NORMAL_PLAY_RATE to create mUnderflowMonitor + mPrivateInstanceAAMP->mMediaFormat = eMEDIAFORMAT_DASH; // Underflow monitor only enabled for DASH in StreamAbstractionAAMP + mStreamAbstractionAAMP = new TestableStreamAbstractionAAMP(mPrivateInstanceAAMP); + // For initialisation of mediatrack EXPECT_CALL(*g_mockAampConfig, GetConfigValue(eAAMPConfig_MaxFragmentCached)) .Times(AnyNumber())