Fix FragmentfailureRampdown thread safety and divide-by-zero

abr/abr.cpp

@@ -1118,10 +1118,20 @@ void ABRManager::CheckLLDashABRSpeedStoreSize(struct SpeedCache *speedcache,Bits
  */
 BitsPerSecond ABRManager::FragmentfailureRampdown(int currentBuffer, int currentProfileIndex)
 {
+	if (eAAMPAbrConfig.abrMaxBuffer <= 0)
+	{
+		AAMPLOG_ERR("abrMaxBuffer is %d, cannot compute buffer percentage",
+			eAAMPAbrConfig.abrMaxBuffer);
+		return 0;
+	}
 	double bufferPercentage = ((double)currentBuffer / eAAMPAbrConfig.abrMaxBuffer) * 100;
 	BitsPerSecond desiredProfilebw = 0;
 	BitsPerSecond currentbw = getBandwidthOfProfile(currentProfileIndex);
-	std::vector<ProfileInfo> availableProfiles = mProfiles;
+	std::vector<ProfileInfo> availableProfiles;
+	{
+		std::lock_guard<std::mutex> lock(mProfileLock);
+		availableProfiles = mProfiles;
+	}
 	availableProfiles.erase(
 		std::remove_if(availableProfiles.begin(), availableProfiles.end(),
 			[](const ProfileInfo &p) { return p.isIframeTrack; }),

support/aampabr/ABRManager.cpp

@@ -687,6 +687,14 @@ int ABRManager::removeProfiles(std::vector<long> profileBPS, int currentProfileI
   return modifiedProfileIndex;
 }
 
+/**
+ *  @brief Get a thread-safe copy of the profile list
+ */
+std::vector<ABRManager::ProfileInfo> ABRManager::getProfileInfoLocked() {
+  std::lock_guard<std::mutex> lock(mProfileLock);
+  return mProfiles;
+}
+
 /**
  *  @brief Clear profiles
  */

support/aampabr/ABRManager.h

@@ -291,6 +291,12 @@ class ABRManager {
 	 */
 	static void logprintf( const char *fmt, ... ) __attribute__ ((format (printf, 1, 2)));
 
+	/**
+	 * @brief Get a thread-safe copy of the profile list
+	 * @return Copy of mProfiles taken under mProfileLock
+	 */
+	std::vector<ProfileInfo> getProfileInfoLocked();
+
 private:
   /**
    * @fn getProfileCountUnlocked

support/aampabr/HybridABRManager.cpp

@@ -406,10 +406,16 @@ void HybridABRManager::CheckLLDashABRSpeedStoreSize(struct SpeedCache *speedcach
  */
 long HybridABRManager::FragmentfailureRampdown(int currentBuffer, int currentProfileIndex)
 {
+	if (eAAMPAbrConfig.abrMaxBuffer <= 0)
+	{
+		logprintf("%s:%d abrMaxBuffer is %d, cannot compute buffer percentage",
+			__FUNCTION__, __LINE__, eAAMPAbrConfig.abrMaxBuffer);
+		return 0;
+	}
 	double bufferPercentage = ((double)currentBuffer / eAAMPAbrConfig.abrMaxBuffer) * 100;
 	long desiredProfilebw = 0;
 	long currentbw = getBandwidthOfProfile(currentProfileIndex);
-	std::vector<ProfileInfo> availableProfiles = getProfileInfo();
+	std::vector<ProfileInfo> availableProfiles = getProfileInfoLocked();
 	availableProfiles.erase(
 		std::remove_if(availableProfiles.begin(), availableProfiles.end(),
 			[](const ProfileInfo &p) { return p.isIframeTrack; }),

test/utests/tests/AampAbrTests/AbrTests.cpp

@@ -435,7 +435,7 @@ TEST_F(AbrTests, UpdateProfile_DefaultIframeBitrate_SelectsBelowDefault)
 }
 
 /**
- * @brief Bug #11: FragmentfailureRampdown must skip iframe tracks when
+ * @brief FragmentfailureRampdown must skip iframe tracks when
  *        selecting a rampdown target, matching every other ABR function.
  */
 TEST_F(AbrTests, FragmentfailureRampdown_SkipsIframeTrack)
@@ -540,3 +540,24 @@ TEST_F(AbrTests, FragmentfailureRampdown_FallbackLowestIsNotIframe)
 	EXPECT_EQ(result, 500000);
 }
 
+/**
+ * @brief FragmentfailureRampdown must return 0 when abrMaxBuffer
+ *        is zero to avoid floating-point divide-by-zero.
+ */
+TEST_F(AbrTests, FragmentfailureRampdown_ZeroMaxBuffer_ReturnsZero)
+{
+	eAAMPAbrConfig.abrMaxBuffer = 0;
+
+	ABRManager abrManager;
+	abrManager.ReadPlayerConfig(&eAAMPAbrConfig);
+
+	ABRManager::ProfileInfo p{};
+	p.isIframeTrack = false;
+	p.bandwidthBitsPerSecond = 1000000;
+	p.width = 640; p.height = 360;
+	abrManager.addProfile(p);
+
+	BitsPerSecond result = abrManager.FragmentfailureRampdown(5, 0);
+	EXPECT_EQ(result, 0);
+}
+

test/utests/tests/AampAbrTests/HybridAbrTests.cpp

@@ -118,7 +118,7 @@ TEST_F(HybridAbrTests, UpdateABRBitrateDataBasedOnCacheOutlierOdd)
 }
 
 /**
- * @brief Bug #6: CheckAbrThresholdSize must multiply before dividing to
+ * @brief CheckAbrThresholdSize must multiply before dividing to
  *        avoid integer truncation when bufferlen < downloadTimeMs.
  */
 TEST_F(HybridAbrTests, CheckAbrThresholdSize_SmallFragment_NoTruncation)
@@ -143,7 +143,7 @@ TEST_F(HybridAbrTests, CheckAbrThresholdSize_LargeFragment_Correct)
 }
 
 /**
- * @brief Bug #17: CheckLLDashABRSpeedStoreSize must multiply before dividing
+ * @brief CheckLLDashABRSpeedStoreSize must multiply before dividing
  *        to avoid integer truncation when total_dl_diff < time_diff.
  */
 TEST_F(HybridAbrTests, CheckLLDashABRSpeedStoreSize_SmallChunk_NoTruncation)
@@ -166,7 +166,7 @@ TEST_F(HybridAbrTests, CheckLLDashABRSpeedStoreSize_LargeChunk_Correct)
 }
 
 /**
- * @brief Bug #11: FragmentfailureRampdown must skip iframe tracks when
+ * @brief FragmentfailureRampdown must skip iframe tracks when
  *        selecting a rampdown target, matching every other ABR function.
  */
 TEST_F(HybridAbrTests, FragmentfailureRampdown_SkipsIframeTrack)
@@ -272,3 +272,24 @@ TEST_F(HybridAbrTests, FragmentfailureRampdown_FallbackLowestIsNotIframe)
 	long result = mgr.FragmentfailureRampdown(1, 2);
 	EXPECT_EQ(result, 500000);
 }
+
+/**
+ * @brief FragmentfailureRampdown must return 0 when abrMaxBuffer
+ *        is zero to avoid floating-point divide-by-zero (legacy ABR).
+ */
+TEST_F(HybridAbrTests, FragmentfailureRampdown_ZeroMaxBuffer_ReturnsZero)
+{
+	eAAMPAbrConfig.abrMaxBuffer = 0;
+
+	HybridABRManager mgr;
+	mgr.ReadPlayerConfig(&eAAMPAbrConfig);
+
+	ABRManager::ProfileInfo p{};
+	p.isIframeTrack = false;
+	p.bandwidthBitsPerSecond = 1000000;
+	p.width = 640; p.height = 360;
+	mgr.addProfile(p);
+
+	long result = mgr.FragmentfailureRampdown(5, 0);
+	EXPECT_EQ(result, 0);
+}