From 799335c07ab95faa90e44696aec2d8808401449d Mon Sep 17 00:00:00 2001 From: varshnie Date: Fri, 13 Feb 2026 09:27:41 +0530 Subject: [PATCH] VPLAY-12682:SoC-independent underflow detection - follow-up improvements Reason for change:follow-up improvements for AampUnderflowMonitor Risks: Medium Signed-off-by: varshnie --- AampUnderflowMonitor.cpp | 37 ++++-------------- AampUnderflowMonitor.h | 22 +++-------- StreamAbstractionAAMP.h | 7 ---- fragmentcollector_hls.cpp | 10 +++++ fragmentcollector_mpd.cpp | 10 +++++ fragmentcollector_progressive.cpp | 10 +++++ priv_aamp.cpp | 52 ++++++++++++------------- priv_aamp.h | 4 +- streamabstraction.cpp | 63 ++++++++++++++----------------- 9 files changed, 99 insertions(+), 116 deletions(-) diff --git a/AampUnderflowMonitor.cpp b/AampUnderflowMonitor.cpp index 134563936..b531b6d01 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); } } @@ -226,7 +214,6 @@ void AampUnderflowMonitor::Run() bool pipelinePaused = false; { std::lock_guard lock(mMutex); - if (!mAamp) return; pipelinePaused = mAamp->pipeline_paused; } @@ -236,7 +223,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 +233,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 +243,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 0bb769906..a91c06b0c 100644 --- a/StreamAbstractionAAMP.h +++ b/StreamAbstractionAAMP.h @@ -2013,13 +2013,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 8b01dab5f..01d2bb99c 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"); + } + } } diff --git a/fragmentcollector_mpd.cpp b/fragmentcollector_mpd.cpp index 94217c377..b4d5ec2c0 100644 --- a/fragmentcollector_mpd.cpp +++ b/fragmentcollector_mpd.cpp @@ -10344,6 +10344,16 @@ void StreamAbstractionAAMP_MPD::Start(void) { StartLatencyMonitorThread(); } + + // 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/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 c3dac9822..008dd50b5 100644 --- a/priv_aamp.cpp +++ b/priv_aamp.cpp @@ -3201,19 +3201,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); @@ -3224,25 +3224,28 @@ void PrivateInstanceAAMP::SendBufferChangeEvent(bool bufferingStopped) */ void PrivateInstanceAAMP::SetBufferingState(bool buffering) { - if (buffering) + if(ISCONFIGSET_PRIV(eAAMPConfig_ReportBufferEvent)) { - SendBufferChangeEvent(true); - if (!pipeline_paused) + if (buffering) { - if (!PausePipeline(true, true)) + SendBufferChangeEvent(true); + if (!pipeline_paused) { - AAMPLOG_ERR("Failed to pause the Pipeline"); + if (!PausePipeline(true, true)) + { + AAMPLOG_ERR("Failed to pause the Pipeline"); + } } } - } - else - { - if (pipeline_paused) + else { - (void)PausePipeline(false, false); + if (pipeline_paused) + { + (void)PausePipeline(false, false); + } + UpdateSubtitleTimestamp(); + SendBufferChangeEvent(false); } - UpdateSubtitleTimestamp(); - SendBufferChangeEvent(false); } } @@ -5508,7 +5511,6 @@ void PrivateInstanceAAMP::TeardownStream(bool newTune, bool disableDownloads) // Using StreamLock to make sure this is not interfering with GetFile() from PreCachePlaylistDownloadTask AcquireStreamLock(); AAMPLOG_INFO("TeardownStream: Stopping StreamAbstraction"); - mpStreamAbstractionAAMP->StopUnderflowMonitor(); mpStreamAbstractionAAMP->Stop(disableDownloads); if(mContentType == ContentType_HDMIIN) @@ -6292,15 +6294,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) @@ -8344,6 +8337,11 @@ void PrivateInstanceAAMP::Stop( bool sendStateChangeEvent ) mAutoResumeTaskPending = false; } DisableDownloads(); + if (mpStreamAbstractionAAMP) + { + mpStreamAbstractionAAMP->StopUnderflowMonitor(); + } + //Moved the tsb delete request from XRE to AAMP to avoid the HTTP-404 erros if(IsFogTSBSupported()) { diff --git a/priv_aamp.h b/priv_aamp.h index 5f72cf9ff..1a6f96d40 100644 --- a/priv_aamp.h +++ b/priv_aamp.h @@ -1538,10 +1538,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 3e845df55..2b355fd4c 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()); } /**