diff --git a/abr/abr.cpp b/abr/abr.cpp index f99377f0d..36c849c8c 100644 --- a/abr/abr.cpp +++ b/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 availableProfiles = mProfiles; + std::vector availableProfiles; + { + std::lock_guard lock(mProfileLock); + availableProfiles = mProfiles; + } availableProfiles.erase( std::remove_if(availableProfiles.begin(), availableProfiles.end(), [](const ProfileInfo &p) { return p.isIframeTrack; }), diff --git a/support/aampabr/ABRManager.cpp b/support/aampabr/ABRManager.cpp index 3278dfe8a..db1a7089d 100644 --- a/support/aampabr/ABRManager.cpp +++ b/support/aampabr/ABRManager.cpp @@ -687,6 +687,14 @@ int ABRManager::removeProfiles(std::vector profileBPS, int currentProfileI return modifiedProfileIndex; } +/** + * @brief Get a thread-safe copy of the profile list + */ +std::vector ABRManager::getProfileInfoLocked() { + std::lock_guard lock(mProfileLock); + return mProfiles; +} + /** * @brief Clear profiles */ diff --git a/support/aampabr/ABRManager.h b/support/aampabr/ABRManager.h index e6fbee489..a93c0f361 100644 --- a/support/aampabr/ABRManager.h +++ b/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 getProfileInfoLocked(); + private: /** * @fn getProfileCountUnlocked diff --git a/support/aampabr/HybridABRManager.cpp b/support/aampabr/HybridABRManager.cpp index 9ac96f6f8..bd9092833 100644 --- a/support/aampabr/HybridABRManager.cpp +++ b/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 availableProfiles = getProfileInfo(); + std::vector availableProfiles = getProfileInfoLocked(); availableProfiles.erase( std::remove_if(availableProfiles.begin(), availableProfiles.end(), [](const ProfileInfo &p) { return p.isIframeTrack; }), diff --git a/test/utests/tests/AampAbrTests/AbrTests.cpp b/test/utests/tests/AampAbrTests/AbrTests.cpp index 58228bed0..f5aba75dc 100644 --- a/test/utests/tests/AampAbrTests/AbrTests.cpp +++ b/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); +} + diff --git a/test/utests/tests/AampAbrTests/HybridAbrTests.cpp b/test/utests/tests/AampAbrTests/HybridAbrTests.cpp index 405c66d22..3926278e4 100644 --- a/test/utests/tests/AampAbrTests/HybridAbrTests.cpp +++ b/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); +}