VPLAY-12682:SoC-independent underflow detection - follow-up improvements

varshnie · varshnie · commit 5d2b338ee928 · 2026-03-23T17:38:36.000+05:30
Reason for change:AampUnderflowMonitor Optimization
Risks: Medium

Signed-off-by: varshnie &lt;varshniblue14@gmail.com&gt;
diff --git 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
@@ -48,60 +48,51 @@ AampUnderflowMonitor::~AampUnderflowMonitor() {
     Stop();
 }
 
-void AampUnderflowMonitor::Start() {
-    // Use unique_lock to allow unlock around join operations
-    std::unique_lock<std::mutex> lock(mMutex);
+void AampUnderflowMonitor::Start()
+{
+    std::lock_guard<std::mutex> 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<std::mutex> 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<std::mutex> lock(mMutex);
-    // Signal thread to stop
+    std::lock_guard<std::mutex> 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<std::mutex> sleepLock(mSleepMutex);
+        mWakeupSignalled = true;
+        mSleepCv.notify_all();
+    }
+    if (mUnderflowMonitorThread.joinable())
+    {
+        mUnderflowMonitorThread.join();
         AAMPLOG_INFO("AampUnderflowMonitor thread joined");
-        lock.lock();
     }
 }
 
@@ -116,136 +107,75 @@ 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<std::mutex> 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<std::mutex> 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<std::mutex> 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 allowBufferCheck = (!isTrickplay && !isSeekingState);
-            if (((allowBufferCheck && bufferedTimeSec <= kUnderflowDetectThresholdSec && trackDownloadsEnabled)) || sinkCacheEmpty)
+            if (!allowBufferCheck)
             {
-                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<std::mutex> lock(mMutex);
-                    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<std::mutex> lock(mMutex);
-                        mAamp->ResumeTrackDownloads(eMEDIATYPE_VIDEO);
-                    }
-                }
+                AAMPLOG_TRACE("[video] skipping buffer-based underflow check (rate=%.2f, trickplay=%d, seeking=%d). buffered=%.3f",
+                    currentRate, (int)isTrickplay, (int)isSeekingState, bufferedTimeSec);
             }
             else
             {
-                if (!allowBufferCheck)
+                const bool sinkCacheEmpty = mAamp->IsSinkCacheEmpty(eMEDIATYPE_VIDEO);
+                if (bufferedTimeSec <= kUnderflowDetectThresholdSec || sinkCacheEmpty)
                 {
-                    // 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<std::mutex> lock(mMutex);
-                    if (!mAamp) return;
-                    pipelinePaused = mAamp->mSinkPaused.load();
-                }
-                
-                if (underflowActive && pipelinePaused)
-                {
-                    if (bufferedTimeSec >= kUnderflowResumeThresholdSec && !sinkCacheEmpty)
-                    {
-                        AAMPLOG_INFO("[video] underflow ended. buffered=%.3f cacheEmpty=%d", bufferedTimeSec, (int)sinkCacheEmpty);
-                        std::lock_guard<std::mutex> lock(mMutex);
-                        mAamp->SetBufferingState(false);
-                    }
-                    else
+                    if (!underflowActive)
                     {
-                        AAMPLOG_INFO("[video] waiting to end underflow. buffered=%.3f cacheEmpty=%d", bufferedTimeSec, (int)sinkCacheEmpty);
+                        AAMPLOG_INFO("[video] underflow detected. buffered=%.3f cacheEmpty=%d (rate=%.2f)", bufferedTimeSec, (int)sinkCacheEmpty, currentRate);
+                        mAamp->SetBufferingState(true);
                     }
                 }
-                else if (underflowActive && !trackDownloadsEnabled && sinkCacheEmpty)
+                else
                 {
-                    AAMPLOG_WARN("[video] underflow ongoing, downloads blocked and cache empty; resuming track downloads");
-                    mAamp->ResumeTrackDownloads(eMEDIATYPE_VIDEO);
+                    const bool pipelinePaused = mAamp->mSinkPaused.load();
+                    if (underflowActive && pipelinePaused)
+                    {
+                        if (bufferedTimeSec >= kUnderflowResumeThresholdSec && !sinkCacheEmpty)
+                        {
+                            AAMPLOG_INFO("[video] underflow ended. buffered=%.3f cacheEmpty=%d", bufferedTimeSec, (int)sinkCacheEmpty);
+                            mAamp->SetBufferingState(false);
+                        }
+                        else
+                        {
+                            AAMPLOG_INFO("[video] waiting to end underflow. buffered=%.3f cacheEmpty=%d", bufferedTimeSec, (int)sinkCacheEmpty);
+                        }
+                    }
                 }
+                // Audio underflow is not handled currently as we are aligning with the existing behavior
             }
-            // 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<std::mutex> 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
@@ -30,6 +30,8 @@
 #include <atomic>
 #include <memory>
 #include <mutex>
+#include <condition_variable>
+#include <chrono>
 
 class StreamAbstractionAAMP;
 class PrivateInstanceAAMP;
@@ -43,10 +45,9 @@ class AampUnderflowMonitor {
     * @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
+    *       Both pointers are immutable after construction.
+    * @note Thread Safety: `Stop()` signals the monitoring thread to exit and
+    *       joins it before returning. Call `Stop()` before destroying the
     *       `StreamAbstractionAAMP` or `PrivateInstanceAAMP` instances.
      */
     AampUnderflowMonitor(StreamAbstractionAAMP* stream, PrivateInstanceAAMP* aamp);
@@ -81,11 +82,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<bool> 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
@@ -3246,6 +3246,14 @@ void PrivateInstanceAAMP::SendBufferChangeEvent(bool bufferingStart)
 	BufferingChangedEventPtr e = std::make_shared<BufferingChangedEvent>(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
@@ -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
 			{

Original file line number	Diff line number	Diff line change
`@@ -2183,7 +2183,7 @@ StreamAbstractionAAMP::StreamAbstractionAAMP(PrivateInstanceAAMP* aamp, id3_call`
`2183`	`2183`
`2184`	`2184`	`if(GETCONFIGVALUE(eAAMPConfig_EnableAampUnderflowMonitor))`
`2185`	`2185`	`{`
`2186`		`- if(!mUnderflowMonitor)`
	`2186`	`+ if(!mUnderflowMonitor && (aamp->rate == AAMP_NORMAL_PLAY_RATE))`
`2187`	`2187`	`{`
`2188`	`2188`	`try`
`2189`	`2189`	`{`