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

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<std::mutex> 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<std::mutex> lock(mMutex);
-    mAamp = nullptr;
-    mStream = nullptr;
 }
 
 void AampUnderflowMonitor::Run()
@@ -125,8 +124,6 @@ void AampUnderflowMonitor::Run()
 
         {
             std::lock_guard<std::mutex> 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<std::mutex> lock(mMutex);
-            if (!mAamp) return;
-            mAamp->interruptibleMsSleep(100);
-        }
+        mAamp->interruptibleMsSleep(100);
     }
 
     while (mRunning.load()) {
@@ -156,7 +148,6 @@ void AampUnderflowMonitor::Run()
 
         {
             std::lock_guard<std::mutex> 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<std::mutex> 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<std::mutex> 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<std::mutex> lock(mMutex);
-                        if (!mAamp) return;
                         mAamp->ResumeTrackDownloads(eMEDIATYPE_VIDEO);
                     }
                 }
@@ -226,7 +214,6 @@ void AampUnderflowMonitor::Run()
                 bool pipelinePaused = false;
                 {
                     std::lock_guard<std::mutex> 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<std::mutex> 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<std::mutex> 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<std::mutex> lock(mMutex);
-            if (!mAamp) return;
-            mAamp->interruptibleMsSleep(sleepMs);
-        }
+        mAamp->interruptibleMsSleep(sleepMs);
     }
     mRunning.store(false);
 }

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,39 +52,30 @@ 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.
      */
     bool IsRunning() const { return mRunning.load(); }
 
 private:
     /**
-     * @fn run
      * @brief Thread entry routine that polls/awaits underflow conditions
      *        and triggers coordinated handling.
      */

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.

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");
+		}
+	}
 }
 
 

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");
+		}
+	}
 }
 
 /**

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");
+		}
+	}
 }
 
 /**

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<BufferingChangedEvent>(!bufferingStopped, GetSessionId());
+	bool bufferAvailable = !bufferingStart; // Buffering stop notification need to be inverted to indicate if buffer available or not
+	BufferingChangedEventPtr e = std::make_shared<BufferingChangedEvent>(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())
 	{

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