From b9c2d4d58e1fcc2f63278efa7ddc79b2996ff3c8 Mon Sep 17 00:00:00 2001 From: Jose Fagundez Date: Fri, 23 Jan 2026 11:06:02 +0100 Subject: [PATCH 1/3] VPLAY-11582 ProcessFragmentChunk() uses timescale of segment downloaded Reason for Change: Avoid potential edge case and make code correct Summary of Changes: * Use timescale of fragment injected in ProcessFragmentChunk() * Verify timescale used by InjectFragment() in L1 tests Test Procedure: L1 tests should pass and no AAMP TSB + CDA regressions Risk: Low --- streamabstraction.cpp | 37 ++++-- .../tests/MediaTrackTests/MediaTrackTests.cpp | 110 ++++++++++++++++++ 2 files changed, 136 insertions(+), 11 deletions(-) diff --git a/streamabstraction.cpp b/streamabstraction.cpp index 9e8f011ec..d4bd4a221 100644 --- a/streamabstraction.cpp +++ b/streamabstraction.cpp @@ -1031,18 +1031,33 @@ bool MediaTrack::ProcessFragmentChunk() } //Print box details //isobuf.printBoxes(); - uint32_t timeScale = 0; - if(type == eTRACK_VIDEO) - { - timeScale = aamp->GetVidTimeScale(); - } - else if(type == eTRACK_AUDIO) - { - timeScale = aamp->GetAudTimeScale(); - } - else if (type == eTRACK_SUBTITLE) + + // Use the timescale stored in the cached fragment, which represents the timescale + // of the segment being injected. This is critical when using TSB, as the segment + // being downloaded at the live edge may have a different timescale (e.g., an ad) + // than the segment being injected from TSB (e.g., base content). + uint32_t timeScale = cachedFragment->timeScale; + //uint32_t timeScale = 0; // Jose + if(!timeScale) { - timeScale = aamp->GetSubTimeScale(); + AAMPLOG_WARN("[%s] Cached fragment timescale is 0, fragment URI: %s", name, cachedFragment->uri.c_str()); + // Fallback to global timescale if cached fragment doesn't have it set + if(type == eTRACK_VIDEO) + { + timeScale = aamp->GetVidTimeScale(); + } + else if(type == eTRACK_AUDIO) + { + timeScale = aamp->GetAudTimeScale(); + } + else if (type == eTRACK_SUBTITLE) + { + timeScale = aamp->GetSubTimeScale(); + } + else + { + AAMPLOG_WARN("[%s] Unknown track type %d, cannot get timescale", name, type); + } } if(!timeScale) { diff --git a/test/utests/tests/MediaTrackTests/MediaTrackTests.cpp b/test/utests/tests/MediaTrackTests/MediaTrackTests.cpp index 4ad8d22bc..e7cc45f6c 100644 --- a/test/utests/tests/MediaTrackTests/MediaTrackTests.cpp +++ b/test/utests/tests/MediaTrackTests/MediaTrackTests.cpp @@ -820,3 +820,113 @@ TEST_F(MediaTrackTests, MediaTrackConstructorChunkModeTest) TestableMediaTrack videoTrack{eTRACK_VIDEO, mPrivateInstanceAAMP, "video", mStreamAbstractionAAMP_MPD}; EXPECT_EQ(videoTrack.GetCachedFragmentChunksSize(), kMaxFragmentChunkCached); } + +/** + * @brief Test data for ProcessFragmentChunk timescale tests + * + * This structure holds the test parameters for verifying that ProcessFragmentChunk + * uses the correct timescale from the cached fragment (critical for TSB playback + * where injected segments may have different timescales than live edge segments). + */ +struct ChunkTimescaleTestData +{ + uint32_t cachedFragmentTimeScale; /**< Timescale stored in the cached fragment */ + uint32_t globalTimeScale; /**< Global timescale from AAMP instance */ + uint32_t expectedTimeScale; /**< Expected timescale to be used in ParseChunkData */ +}; + +class MediaTrackProcessFragmentChunkTimescaleTests + : public MediaTrackTests, + public testing::WithParamInterface +{ +}; + +/** + * @brief Test that ProcessFragmentChunk uses the cached fragment's timescale + * + * This test verifies that when processing fragment chunks, the timescale from + * the cached fragment is used (if set), rather than falling back to global + * timescales. This is critical for TSB (Time-Shift Buffer) scenarios where: + * - The segment being downloaded at the live edge may be from an ad with one timescale + * - The segment being injected from TSB may be from base content with a different timescale + * + * The test would fail if ProcessFragmentChunk used global timescales instead of the + * cached fragment's timescale, as the ParseChunkData mock expects the exact timescale + * value from the cached fragment. + */ +TEST_P(MediaTrackProcessFragmentChunkTimescaleTests, UseCachedFragmentTimescaleInParseChunkData) +{ + ChunkTimescaleTestData testData = GetParam(); + CachedFragment* bufferedFragment{nullptr}; + + // Configure low-latency mode since ProcessFragmentChunk is used for chunk-based injection + SetLowLatencyMode(true); + mPrivateInstanceAAMP->rate = AAMP_NORMAL_PLAY_RATE; + mPrivateInstanceAAMP->mMediaFormat = eMEDIAFORMAT_DASH; + mStreamAbstractionAAMP_MPD->trickplayMode = false; + + // Set up mock to return the global timescale (used as fallback when cached fragment timescale is 0) + EXPECT_CALL(*g_mockPrivateInstanceAAMP, GetVidTimeScale()) + .WillRepeatedly(Return(testData.globalTimeScale)); + + EXPECT_CALL(*g_mockAampConfig, IsConfigSet(eAAMPConfig_OverrideMediaHeaderDuration)) + .WillRepeatedly(Return(false)); + EXPECT_CALL(*g_mockAampConfig, IsConfigSet(eAAMPConfig_CurlThroughput)) + .WillRepeatedly(Return(false)); + EXPECT_CALL(*g_mockAampConfig, IsConfigSet(eAAMPConfig_EnablePTSReStamp)) + .WillRepeatedly(Return(false)); + EXPECT_CALL(*g_mockAampConfig, GetConfigValue(eAAMPConfig_MaxFragmentCached)) + .WillRepeatedly(Return(1)); + EXPECT_CALL(*g_mockAampConfig, GetConfigValue(eAAMPConfig_MaxFragmentChunkCached)) + .WillRepeatedly(Return(1)); + EXPECT_CALL(*g_mockIsoBmffBuffer, parseBuffer(_, _)).WillRepeatedly(Return(true)); + EXPECT_CALL(*g_mockPrivateInstanceAAMP, GetLLDashChunkMode()).WillRepeatedly(Return(true)); + + TestableMediaTrack videoTrack{eTRACK_VIDEO, mPrivateInstanceAAMP, "video", + mStreamAbstractionAAMP_MPD}; + + // First inject an init fragment (required before media fragments) + CachedFragment initFragment; + initFragment.initFragment = true; + initFragment.fragment.AppendBytes(FRAGMENT_TEST_DATA, strlen(FRAGMENT_TEST_DATA)); + bufferedFragment = videoTrack.GetFetchChunkBuffer(true); + videoTrack.numberOfFragmentChunksCached = 1; + bufferedFragment->Copy(&initFragment, initFragment.fragment.GetLen()); + ASSERT_TRUE(videoTrack.InjectFragment()); + + // Now inject a media fragment with a specific timescale + CachedFragment testFragment; + testFragment.initFragment = false; + testFragment.duration = FRAGMENT_DURATION.inSeconds(); + testFragment.position = FIRST_PTS.inSeconds(); + testFragment.timeScale = testData.cachedFragmentTimeScale; + testFragment.uri = "test_segment.m4s"; + testFragment.fragment.AppendBytes(FRAGMENT_TEST_DATA, strlen(FRAGMENT_TEST_DATA)); + + bufferedFragment = videoTrack.GetFetchChunkBuffer(true); + videoTrack.numberOfFragmentChunksCached = 1; + bufferedFragment->Copy(&testFragment, testFragment.fragment.GetLen()); + + // Key assertion: ParseChunkData should be called with the expected timescale + // This tests that ProcessFragmentChunk uses cachedFragment->timeScale when set, + // or falls back to global timescale when cachedFragment->timeScale is 0 + EXPECT_CALL(*g_mockIsoBmffBuffer, + ParseChunkData(_, _, testData.expectedTimeScale, _, _, _, _)) + .WillOnce(DoAll(SetArgReferee<5>(bufferedFragment->position), + SetArgReferee<6>(bufferedFragment->duration), Return(true))); + + ASSERT_TRUE(videoTrack.InjectFragment()); +} + +ChunkTimescaleTestData chunkTimescaleTestData[] = { + // When cached fragment has a valid timescale, it should be used + {90000, 48000, 90000}, // Fragment timescale 90000 (typical video), global 48000 -> use 90000 + {48000, 90000, 48000}, // Fragment timescale 48000 (typical audio), global 90000 -> use 48000 + {10000000, 90000, 10000000}, // Fragment timescale 10000000 (100ns units), global 90000 -> use 10000000 + // When cached fragment timescale is 0, fall back to global timescale + {0, 90000, 90000}, // Fragment timescale 0, global 90000 -> use global + {0, 48000, 48000}, // Fragment timescale 0, global 48000 -> use global +}; + +INSTANTIATE_TEST_SUITE_P(MediaTrackTests, MediaTrackProcessFragmentChunkTimescaleTests, + ValuesIn(chunkTimescaleTestData)); From b7a892f6357e81239dc611dcc4085b8b235ec516 Mon Sep 17 00:00:00 2001 From: Jose Fagundez Date: Fri, 23 Jan 2026 12:09:53 +0100 Subject: [PATCH 2/3] Remove left over, commented out code used for debug --- streamabstraction.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/streamabstraction.cpp b/streamabstraction.cpp index d4bd4a221..5d2353d1f 100644 --- a/streamabstraction.cpp +++ b/streamabstraction.cpp @@ -1037,7 +1037,6 @@ bool MediaTrack::ProcessFragmentChunk() // being downloaded at the live edge may have a different timescale (e.g., an ad) // than the segment being injected from TSB (e.g., base content). uint32_t timeScale = cachedFragment->timeScale; - //uint32_t timeScale = 0; // Jose if(!timeScale) { AAMPLOG_WARN("[%s] Cached fragment timescale is 0, fragment URI: %s", name, cachedFragment->uri.c_str()); From d446a63e4120b43df8683860e2a785b08f7e9c5e Mon Sep 17 00:00:00 2001 From: Jose Fagundez Date: Fri, 23 Jan 2026 12:49:05 +0100 Subject: [PATCH 3/3] Remove the code that tries to handle timescale=0 --- streamabstraction.cpp | 38 ++------------------------------------ 1 file changed, 2 insertions(+), 36 deletions(-) diff --git a/streamabstraction.cpp b/streamabstraction.cpp index 5d2353d1f..a890cbe45 100644 --- a/streamabstraction.cpp +++ b/streamabstraction.cpp @@ -1039,42 +1039,8 @@ bool MediaTrack::ProcessFragmentChunk() uint32_t timeScale = cachedFragment->timeScale; if(!timeScale) { - AAMPLOG_WARN("[%s] Cached fragment timescale is 0, fragment URI: %s", name, cachedFragment->uri.c_str()); - // Fallback to global timescale if cached fragment doesn't have it set - if(type == eTRACK_VIDEO) - { - timeScale = aamp->GetVidTimeScale(); - } - else if(type == eTRACK_AUDIO) - { - timeScale = aamp->GetAudTimeScale(); - } - else if (type == eTRACK_SUBTITLE) - { - timeScale = aamp->GetSubTimeScale(); - } - else - { - AAMPLOG_WARN("[%s] Unknown track type %d, cannot get timescale", name, type); - } - } - if(!timeScale) - { - //FIX-ME-Read from MPD INSTEAD - if(pContext) - { - timeScale = pContext->GetCurrPeriodTimeScale(); - if(!timeScale) - { - timeScale = 10000000.0; - AAMPLOG_WARN("[%s] Empty timeScale!!! Using default timeScale=%d", name, timeScale); - } - } - else - { - timeScale = 1000.0; - AAMPLOG_WARN("[%s] Invalid play context maybe test setup, timeScale=%d", name, timeScale); - } + AAMPLOG_ERR("[%s] Cached fragment timescale is 0, fragment URI: %s", name, cachedFragment->uri.c_str()); + return true; } double fpts = 0.0, fduration = 0.0; bool ret = isobuf.ParseChunkData(name, unParsedBuffer, timeScale, parsedBufferSize, unParsedBufferSize, fpts, fduration);