Merge changes from dev_sprint_25_2 to feature branch - #1673
Merged
Conversation
…1462) * VPAAMP-363: L1 tests for parallel init segment ordering regression Add three GoogleTest cases to AampTrackWorkerTests/FunctionalTests.cpp to cover the race condition introduced by PR 114108 (RDKAAMP-4072): InitJob_HighPriority_ExecutesBeforeEnqueuedMediaJobs Verifies that SubmitJob(..., highPriority=true) places the init segment job at the front of the per-track worker queue, executing before any already-queued media jobs. This is the protection mechanism that FetchFragment relies on when profileChanged=true. InitJob_LowPriority_ExecutesAfterEnqueuedMediaJobs Documents the ordering hazard on the DetectDiscontinuityAndFetchInit path where profileChanged has already been cleared (highPriority=false). The init job goes to push_back and executes after queued media jobs. A code fix must ensure init always carries high priority on this path. StoppedWorker_SubmitInitJob_FutureIsInvalid Verifies that a job submitted to a stopped worker is silently dropped (returned future is invalid). Documents the stop/retune race: if StopWorker() wins, the init segment is lost and the decoder is never initialised for that tune attempt. * test: RDKAAMP-4072 deterministic SegmentBase profileChanged regression test Add FetcherLoopTests::SegmentBase_WaitForFreeFragmentFails_ProfileChangedMustBeCleared to the StreamAbstractionAAMP_MPD suite. Why the previously committed AampTrackWorker L1 tests do NOT catch the bug: The three worker tests validate AampTrackWorker::SubmitJob priority ordering in isolation. The worker mechanism is correct; the bug is in the CALLER (FetchAndInjectInitialization, SegmentBase path) which leaves profileChanged true when WaitForFreeFragmentAvailable(0) returns false. The worker is never invoked on that path. Why the new test FAILS on unpatched code / PASSES with the fix: - Uses mSegmentBaseManifest (SegmentBase, not SegmentTemplate) so that FetchAndInjectInitialization enters the only branch where profileChanged is conditionally cleared. - Sets numberOfFragmentsCached == GetCachedFragmentSize() to force WaitForFreeFragmentAvailable(0) to return false. - Asserts EXPECT_FALSE(videoTrack->profileChanged) after the call. - Without fix: profileChanged stays true -> FAIL. - With fix (else-branch added): profileChanged is cleared -> PASS. Changes: - Added mSegmentBaseManifest static constexpr (VOD SegmentBase manifest). - Added InvokeFetchAndInjectInitialization() to TestableStreamAbstractionAAMP_MPD. - Added regression test. * fix: RDKAAMP-4072 clear profileChanged on SegmentBase WaitForFreeFragment failure In FetchAndInjectInitialization, the SegmentBase path called WaitForFreeFragmentAvailable(0) and only cleared profileChanged inside the success branch. When the ring buffer was full (WaitForFreeFragmentAvailable returned false) there was no else-branch, so profileChanged remained true. On the next pass through the FetcherLoop every OnFragmentDownloadComplete callback saw profileChanged=true and re-invoked FetchAndInjectInitialization, which failed again for the same reason. The init segment was never delivered to the decoder, AAMP_EVENT_TUNED never fired, and the application observed a ~20-second silence / hang. The SegmentTemplate and SegmentList-with-sourceURL paths were unaffected because they clear profileChanged unconditionally. Fix: add an else-branch that clears profileChanged when WaitForFreeFragmentAvailable(0) returns false, allowing the FetcherLoop to drain the ring buffer and re-attempt the init fetch on the next profileChanged cycle rather than spinning indefinitely. * test: fix SegmentBase regression test — add LoadIDX mock expectation SegmentBase streams call LoadIDX to fetch and parse the Segment Index box (SIDX, pointed to by indexRange). The test used StrictMock so the unexpected LoadIDX call caused an immediate failure before the profileChanged assertion was ever reached. Add EXPECT_CALL(*g_mockPrivateInstanceAAMP, LoadIDX(...)) using the same sidxBox lambda pattern already used by other SegmentBase tests in this file, so InitializeMPD completes normally and the regression assertion is exercised. * test/prod: fix ASSERT_TRUE anti-pattern and DRY profileChanged assignment FunctionalTests.cpp: replace ASSERT_TRUE(future.wait_for(...) == ready) with ASSERT_EQ so GoogleTest prints actual vs expected future_status on timeout failures (L1 anti-pattern: EXPECT_TRUE with comparison operators). fragmentcollector_mpd.cpp (~line 8538): hoist the unconditional profileChanged = false out of the if/else branches into a single statement after the WaitForFreeFragmentAvailable block, removing the duplicated assignment and obsolete else-branch (DRY). * fix: clear profileChanged on SegmentList-range WaitForFreeFragment failure Mirror the RDKAAMP-4072 fix applied to the SegmentBase path: move profileChanged = false outside the WaitForFreeFragmentAvailable(0) block in the SegmentList-with-byte-range init segment sub-path. Previously, if the ring buffer was full and WaitForFreeFragmentAvailable(0) returned false, profileChanged was left true, causing OnFragmentDownloadComplete to re-invoke FetchAndInjectInitialization on every subsequent media segment — an infinite silent-skip loop identical to the SegmentBase bug. * Reason for Change: remove not-needed parallel download disable for segment base Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com> * VPAAMP-437: protect IDX buffer with mIdxMutex for parallel download thread safety SegmentBase streams set DashParallelFragDownload=false to avoid a race on the IDX (sidx) buffer between the FetcherLoop writer and download worker thread readers. Now that the false-override is removed (parallel download re-enabled for SegmentBase), introduce std::mutex mIdxMutex on MediaStreamContext to protect IDX access across threads. Write side (FetcherLoop / init path - all in fragmentcollector_mpd.cpp): - SkipFragments lazy LoadIDX call - SkipFragments ClearAndRelease on EOS - FetchAndInjectInitialization ClearAndRelease on profile change Read side (download worker thread - MediaStreamContext.cpp): - DownloadFragment bandwidth-change range recompute block The mutex is uncontested on every normal segment download; it is only contested during the narrow window where a live manifest refresh clears and reloads IDX while a worker is mid-ABR-switch range recompute. * fix: pass .get() to %p format specifier for shared_ptr in fake * fix: restore missing closing brace on GetPeriodEndTime() in FetcherLoopTests * fix: restore missing closing brace on HandleSeekEOS_UpdateTrackInfoFails test --------- Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com>
* VPAAMP-434 DirectRialto - Update StreamSink Reason for Change: Reduce the risk when delivering DirectRialto changes Summary of Changes: - Add new DirectRialto configuration (cannot be enabled yet) - Add some virtual functions to StreamSink API - Rename AampStreamSinkManager vars and comments to make it more generic Test Procedure: No regressions, since this is pure refactoring Priority: P1 Risks: Low * Rename vars and comments to make StreamSink more generic * Rename AAMP config setting to useDirectRialto * Create a AampGstPlayer even if useDirectRialto=true * Fix L1 MediaStreamContextTests * Make AampStreamSinkManager log messages more generic * Update log line and comment to address review comments
…esh attempts to stop (#1531) * VPAAMP-477: a single curl 56 on manifest refresh causes manifest refresh attempts to stop After a download failure mMPDData->mIsLiveManifest is false (only set during successful parse), so refreshNeeded was never set and the downloader thread exited on the first refresh error. Add an else-if branch below the existing timeout/COULDNT_CONNECT fast-retry block to catch all other non-success error codes (including CURLE_RECV_ERROR / curl 56) when past the first download. Sets a 500 ms retry interval and keeps the refresh loop alive so the next manifest fetch is attempted rather than killing the downloader thread. The !firstDownload guard ensures tune-time failures still exit the loop immediately, preserving existing behaviour for initial tune failures. * VPAAMP-477: add recovery logging and fix unnecessary 500ms interval override Two follow-up improvements to the manifest refresh retry fix: 1. Add downloadFailed flag to track transitions from failure back to success. Set in both the timeout/COULDNT_CONNECT branch and the new transient-error branch; cleared with a WARN log when the next manifest fetch succeeds. This makes it easy to count recovery events in production logs independently of the per-failure curl error logs. 2. Remove the mRefreshInterval=MIN_DELAY_BETWEEN_PLAYLIST_UPDATE_MS override from the transient-error (curl 56 / non-timeout) branch. mRefreshInterval is a member that persists across iterations and is set by getMeNextManifestDownloadWaitTime() on every successful parse, so it already holds the correct MPD-derived interval. The 500ms override is appropriate for timeout/COULDNT_CONNECT (fast connectivity detection) but not for transient receive errors where the stream's own minimumUpdatePeriod should be respected. * VPAAMP-477: restore tuneUrl effective URL update dropped in previous commit The tuneUrl = mMPDData->mMPDDownloadResponse->sEffectiveUrl assignment was accidentally removed during refactoring. Without it, redirects are not followed on subsequent refreshes — the downloader keeps hitting the original URL instead of the post-redirect effective URL. * fix: use 500ms fast-retry for non-timeout manifest download errors - Change preProcessCallback type from std::function<std::string()> to std::function<std::pair<std::string,int>()> so callers can supply a specific curl/HTTP error code rather than always producing CURLE_OPERATION_TIMEDOUT - In the else-if retry block, use a local fastRetry = 500ms instead of mRefreshInterval; mRefreshInterval is not permanently overwritten so the MPD-derived update period is preserved after recovery - Update SendManifestPreProcessEvent in priv_aamp and all fakes/mocks to return pair<string,int> with CURLE_OPERATION_TIMEDOUT on empty - Add L1 regression test AampMPDDownloader_LiveRefreshRetriesWhenFailureIsCurlRecvError: injects CURLE_RECV_ERROR after first successful live fetch and asserts the downloader loop continues producing further refresh attempts
…nkMode call (#1551) * VPAAMP-513: Fix SetCurlTimeout no-op on Stop due to late SetLLDashChunkMode call SetLLDashChunkMode(false) was called after TeardownStream() in Stop(), by which point CurlTerm() had already nulled out curl[0..AAMP_TRACK_COUNT-1]. The else-branch in SetCurlTimeout logged an error (now WARN) for each null handle, and the intended timeout restore was silently skipped. Fix: move SetLLDashChunkMode(false) to immediately before TeardownStream() so the curl handles are still alive when the timeout values are restored. Also downgrade the no-handle diagnostic in SetCurlTimeout from ERR to WARN with a clearer message (curl handle not initialized, skipping timeout update) since the condition is benign. * VPAAMP-513: Split SetCurlTimeout else branch into out-of-range (ERR) vs null handle (WARN) The combined else branch logged the same WARN for both 'instance >= MAX' (API misuse) and 'curl[instance] == nullptr' (handle not yet initialized or already torn down), making it impossible to distinguish real bugs from expected lifecycle gaps. Split into three explicit branches: - instance >= eCURLINSTANCE_MAX: ERR (caller passed invalid index) - curl[instance] == nullptr: WARN (handle not initialized, benign) - otherwise: perform the timeout update (unchanged)
* VPAAMP-508 Fix L1 test build on macOS Add stub systemd/sd-journal.h header with syslog.h include for LOG_NOTICE and sd_journal_printv declaration. Add sd_journal_printv definition to FakeSdJournal.cpp. Add fakes directory to include path so the stub header is found by all test targets. * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: pstroffolino <Philip_Stroffolino@cable.comcast.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…seMp4Demux=true (#1479) * VPLAY-13151 [LLD channel]: Trickplay not functioning correctly when useMp4Demux=true Reason for change: As the TrickmodeRestamp, is not handled for the Mp4segments, so sudden jumps in the pts value is noticed and the discontinuous segments injected to the pipeline causing trickplay failure. In order to resolve the above issue, handled TrickmodeRestamping accordingly for the Mp4segments Risks: Low Test Procedure: Refer jira ticket Priority: P1 Signed-off-by: lashminthaj <lashminthaj17@gmail.com> * VPLAY-13151 [LLD channel]: Trickplay not functioning correctly when useMp4Demux=true Reason for change:optimization and sending only the iframe samples to gstreamer handled in localtsb case Risks: Low Test Procedure: Refer jira ticket Priority: P1 Signed-off-by: varshnie <varshniblue14@gmail.com> * VPLAY-13151 [LLD channel]: Trickplay not functioning correctly when useMp4Demux=true Reason for change:modified trickphase mpode logic from 4 states to 2 states and overrided first pts in case of mp4demux and trickplay mode in getFirstPts Risks: Low Test Procedure: Refer jira ticket Priority: P1 Signed-off-by: varshnie <varshniblue14@gmail.com> --------- Signed-off-by: lashminthaj <lashminthaj17@gmail.com> Signed-off-by: varshnie <varshniblue14@gmail.com> Co-authored-by: lashmintha <lashminthaj17@gmail.com> Co-authored-by: pstroffolino <Philip_Stroffolino@cable.comcast.com>
Reason for Change: Attempting to create a reasonably comprehensive set of guidelines for both prompt creation and overall usage for Github Copilot. These guidelines attempt to take account of the new billing system, trying to avoid excessive token use as well as minimising computing use (so fewer endless prompts, for example). Bound to be edge cases that haven't been covered yet --------- Co-authored-by: pstroffolino <Philip_Stroffolino@cable.comcast.com>
Reason for Change: Minor changes to avoid stale header files in aamp repo - they were only being copied in if --player-interface-source=external Less of a sledgehammer than -t, faster and doesn't force a full rebuild Co-authored-by: pstroffolino <Philip_Stroffolino@cable.comcast.com>
Add override keyword to AAMPGstPlayer::GetEncryptedAampId declaration in aampgstplayer.h. Add override to ChangeAamp and SetEncryptedAamp MOCK_METHODs in MockAampGstPlayer.h. Eliminates all -Winconsistent-missing-override warnings from the L1 test build. Co-authored-by: pstroffolino <Philip_Stroffolino@cable.comcast.com>
…ping enabled, e.g. NBCNewsNOW (#1542) Reason for Change: instead of the AAMP layer rewriting the MPEGTS field in the VTT byte stream (fragile header surgery), the PTS offset is pushed into the subtec channel as a signed time offset, matching the pattern already used on the DASH path. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: dnfaulkner <115161165+dnfaulkner@users.noreply.github.com> Co-authored-by: pstroffolino <Philip_Stroffolino@cable.comcast.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Reason for Change: Fixes HLS-TS PTS restamping across SSAI (server-side ad insertion) transitions to prevent video freeze when restamping is enabled. The core change avoids using a stale base_pts at the ad boundary and adds logic to suppress false rollover detection immediately after discontinuities/offset changes. Changes: Update Demuxer::UpdateSegmentInfo() to compute restamped PTS/DTS from raw encoder ticks plus ptsOffset (avoiding stale base_pts arithmetic). Add suppress_rollover_detection and clear/suppress rollover handling around discontinuities via setPtsOffset() / init(). Add a new TsDemuxerTests L1 unit test suite covering restamp output and rollover suppression/correction. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: pstroffolino <Philip_Stroffolino@cable.comcast.com>
* VPAAMP-524 Fix UK spelling variants in Copilot guidelines * VPAAMP-524 Fix remaining UK spellings in Copilot guidelines
…brew/opt/gstreamer/lib/libgstvideo-1.0.0.dylib' which was built for newer version 26.0 (#1555) Reason for Change: target OSX 26.0 to avoid linker warnings for libgstvideo-1.0.0.dylib Risk: Low (assuming all Mac developers ok with OSX26 as baseline) Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com> --------- Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…g' not found (#1558) Reason for Change: avoid compilation noise Removed the 5 link_directories() calls in the Darwin block (for OPENSSL, GSTREAMER, GSTREAMERVIDEO, GSTREAMERBASE, GLIB) and replaced them with a comment explaining why. These were the direct source of all four warnings — Xcode appends $(CONFIGURATION) to every link_directories() entry, creating the nonexistent .../lib/Debug paths. Changed ${OPENSSL_LIBRARIES} → ${OPENSSL_LINK_LIBRARIES} in LIBAAMP_DEPENDS. _LIBRARIES holds short names like ssl;crypto and requires a directory hint to resolve; _LINK_LIBRARIES holds full absolute paths (e.g. [libssl.dylib|vscode-file://vscode-app/Users/pstrof200@cable.comcast.com/Downloads/Visual%20Studio%20Code.app/Contents/Resources/app/out/vs/code/electron-browser/workbench/workbench.html]) and needs no directory search at all. The rest of the packages (GSTREAMER, GSTREAMERBASE, CURL, etc.) were already using the _LINK_LIBRARIES form — OpenSSL was just inconsistent. Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com> * VPAAMP-525 Address review feedback - Narrow the comment in top-level CMakeLists.txt Darwin block: the claim that 'all libraries use _LINK_LIBRARIES' was wrong (e.g. LIBXML2_LIBRARIES still uses short names). Comment now names the specific deps (OpenSSL, GStreamer) that were changed. - Remove link_directories(${OPENSSL_LIBRARY_DIRS}) from middleware/CMakeLists.txt Darwin block (same Xcode Debug-path warning as the top-level fix). - Remove link_directories(${GSTREAMERVIDEO_LIBRARY_DIRS}) from middleware/gst-plugins/CMakeLists.txt Darwin block (same issue). * VPAAMP-525 Fix remaining Xcode search-path-not-found warning in subtec build GSTREAMERBASE_LIBRARY_DIRS from pkg-config includes transitive Homebrew paths (including OpenSSL), so link_directories() on those variables caused the Xcode generator to append Debug to all of them. Switch GSTSUBTEC_DEPENDENCIES from short-name _LIBRARIES to full-path _LINK_LIBRARIES and drop the link_directories() call. * VPAAMP-525 Fix Xcode cjson search-path-not-found warning link_directories(${LIBCJSON_LIBRARY_DIRS}) in middleware/playerJsonObject was redundant - target_link_libraries already uses LIBCJSON_LINK_LIBRARIES (full absolute paths). The bare link_directories() caused the Xcode generator to emit a Debug-suffixed search path that doesn't exist. * VPAAMP-525 Fix 'Ignoring duplicate libraries' Xcode warning for -ldl On macOS, libdl is part of libSystem.dylib and is always linked automatically. The explicit -ldl in LIBAAMP_DEPENDS was redundant on Darwin and triggered an Xcode 'Ignoring duplicate libraries' warning. Guard it to non-Darwin builds only. Note: the companion -lssl/-lcrypto duplicates were caused by a stale Xcode project generated before the OPENSSL_LIBRARIES->OPENSSL_LINK_LIBRARIES change; those will be gone after cmake regeneration. --------- Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…nctions/variables, incompatible types, and potential null pointer dereferences) (#1560) Reason for Change: This PR addresses a set of compiler/static-analysis warnings (unused symbols, signed/unsigned mismatch, and potential null dereferences) in several playback/streaming modules. Added null checks around potentially-null context/config pointers to avoid static-analysis flagged dereferences. Removed an unused static helper (isWebVttSegment) from streamabstraction.cpp. Fixed a signed/unsigned assignment warning in IsoBmffBuffer::getLastMdatBoxIndex(). - streamabstraction.cpp: guard pContext before accessing mSavedLatencyMonitorState in loadNewAudio block - fragmentcollector_mpd.cpp: guard mMediaStreamContext[eMEDIATYPE_VIDEO] and mMediaStreamContext[eMEDIATYPE_AUDIO] before dereference in audio-only period handling - AampCurlDownloader.cpp: guard mDnldCfg before dereferencing iDownload502RetryWaitMs/iDownloadRetryWaitMs in retry delay calculation - aampgstplayer.cpp: discard unused return value from FlushTrack() with (void)rate - isobmff/isobmffbuffer.cpp: cast size_t loop index to int to silence signed/unsigned mismatch warning in getLastMdatBoxIndex() * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…DEO_LINK_LIBRARIES VPAAMP-525 removed link_directories() for Homebrew GStreamer paths to avoid Xcode appending to search paths. It documented that GStreamer targets should use *_LINK_LIBRARIES full absolute paths instead. middleware/CMakeLists.txt line 282 still used a raw -lgstvideo-1.0 short flag (under CMAKE_USE_OPENCDM_ADAPTER), which relied on the now-removed directory entry. The parent CMakeLists.txt already runs pkg_check_modules(GSTREAMERVIDEO REQUIRED gstreamer-video-1.0) before add_subdirectory(middleware), so GSTREAMERVIDEO_LINK_LIBRARIES is available as an inherited variable containing the full absolute dylib path. Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com>
…DEO_LINK_LIBRARIES (#1563) Reason for Change: add guard to limit path fix to Darwin (OSX) builds VPAAMP-525 removed link_directories() for Homebrew GStreamer paths to avoid Xcode appending to search paths. It documented that GStreamer targets should use *_LINK_LIBRARIES full absolute paths instead. middleware/CMakeLists.txt line 282 still used a raw -lgstvideo-1.0 short flag (under CMAKE_USE_OPENCDM_ADAPTER), which relied on the now-removed directory entry. The parent CMakeLists.txt already runs pkg_check_modules(GSTREAMERVIDEO REQUIRED gstreamer-video-1.0) before add_subdirectory(middleware), so GSTREAMERVIDEO_LINK_LIBRARIES is available as an inherited variable containing the full absolute dylib path. Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com>
…rack switch latency monitor re-enable (#1567) Reason for Change: add a pContext null check before reading mSavedLatencyMonitorState in the subtitle track-switch re-enable logic
Reason for change: Fix compilation issues by patching libdash source code with the standard header files. To fix the linker error use _LINK_LIBRARIES pkg-config variables instead of _LIBRARIES Test Procedure: Build should pass successfully in Tahoe 26.4.1 and also in other platforms without any issues Risks: None Signed-off-by: Vinish100 <vinish.balan@gmail.com>
Reason for change: Fix copyright & Blackduck violation Risks: Low Test Procedure: Refer jira ticket Priority: P1 Signed-off-by: psiva01 <sivasubramanian.patchaiperumal@ltts.com> * RDK-61037:Update chart.min.js * BLDK-835: Missing Credits in aamp NOTICE Signed-off-by: psiva01 <sivasubramanian.patchaiperumal@ltts.com> --------- Signed-off-by: psiva01 <sivasubramanian.patchaiperumal@ltts.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: psiva01 <214551386+psiva01@users.noreply.github.com>
* VPAAMP-417: [World Cup 26] Period transition bug with CDAI Reason for change: Update mCurPlayingBreakId during transition to IN_ADBREAK_AD_NOT_PLAYING to IN_ADBREAK_AD_PLAYING. This ensures that when it ultimately moves to OUTSIDE_ADBREAK when ad has finished, mBasePeriodId is updated correctly. Replace AdEvent::DEFAULT with AdEvent::PERIOD_CHANGE. Extended L1s to confirm the CDAI object values are updated correctly and fixed issues identified from L1. Test Procedure: Confirm all L1 pass. Sanity test CDAI Linear Risks: Low Signed-off-by: Vinish100 <vinish.balan@gmail.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Signed-off-by: Vinish100 <vinish.balan@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…1564) Reason for change: underflow is now suppressed at VOD EOS boundary so the monitor does not post an underflow anomaly while EOS completion is in flight. --------- Signed-off-by: psiva01 <sivasubramanian.patchaiperumal@ltts.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: pstroffolino <Philip_Stroffolino@cable.comcast.com>
… with "Please wait until the program is loaded" error (#1582) * VPAAMP-529: Stop underflow monitor at VOD injection EOS instead of EOS boundary heuristic Replace the position-based EOS boundary guard (eosEndToleranceSec config) with a clean lifecycle fix: call StopUnderflowMonitor() from MediaTrack::InjectFragment() immediately after the last VOD video fragment's EOS sentinel is signalled to the pipeline. At that point no further fragment arrivals are possible, so underflow detection is meaningless and the monitor thread can be safely terminated before the GStreamer EOS bubble drains and eSTATE_COMPLETE is reached. Changes: - streamabstraction.cpp: call pContext->StopUnderflowMonitor() in the VOD video EOS path of InjectFragment(), after EndOfStreamReached() is called. - AampUnderflowMonitor.cpp: remove EOS boundary guard block (IsLive / duration / eosEndToleranceSec check). - AampConfig.h: remove eAAMPConfig_UnderflowEosEndToleranceSec enum entry. - AampConfig.cpp: remove underflowEosEndToleranceSec config table entry. - AampDefine.h: remove DEFAULT_UNDERFLOW_EOS_END_TOLERANCE_SEC constant. The eSTATE_COMPLETE exit guard added in VPAAMP-529 is retained as a safety net. * VPAAMP-529: Add L1 regression tests for VOD EOS StopUnderflowMonitor Two new tests in TrackInjectTests verify the VPAAMP-529-improved fix: - InjectFragment_VodEos_StopsUnderflowMonitor: asserts that StopUnderflowMonitor() is called exactly once when the video track's EOS sentinel is injected on a VOD stream (IsLive=false). - InjectFragment_LiveEos_DoesNotStopUnderflowMonitor: asserts that StopUnderflowMonitor() is NOT called when the same EOS sentinel path fires on a live stream, protecting that code path from regression. Supporting changes: - StreamAbstractionAAMP.h: make StopUnderflowMonitor() virtual so MockStreamAbstractionAAMP can intercept it via virtual dispatch. - MockStreamAbstractionAAMP.h: add MOCK_METHOD for StopUnderflowMonitor with override specifier. - TrackInjectTests.cpp: define g_mockStreamAbstractionAAMP locally to prevent FakeStreamAbstractionAamp.cpp.o from being loaded from libfakes.a, which would otherwise introduce 120 pre-existing duplicate symbols with the directly-compiled streamabstraction.cpp. * Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * VPAAMP-529: Add StopUnderflowMonitor EXPECT_CALL to PreferredLanguages L1 tests --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* VPAAMP-420 Phase 1: VOD CDAI scaffolding
Add RegisterVodAdBreak/CancelVodAdBreak API, VodAdBreakOpportunityEvent,
eAAMPConfig_VodAdBreakLookaheadSec config, and FetcherLoop lookahead hook.
Changes:
- AampEvent.h/cpp: AAMP_EVENT_VOD_ADBREAK_OPPORTUNITY (id=47),
VodAdBreakOpportunityEvent class (breakId, insertionPointSec,
breakDurationSec, breakType)
- AampDefine.h: DEFAULT_VOD_ADBREAK_LOOKAHEAD_SEC = 5
- AampConfig.h/cpp: eAAMPConfig_VodAdBreakLookaheadSec integer config
- AdManagerBase.h: virtual no-op stubs on CDAIObject base
- admanager_mpd.h: VodAdBreakInfo struct; mVodAdBreaks/mNextVodBreakToCheck
members; RegisterVodAdBreak/CancelVodAdBreak/CheckVodAdBreakLookahead decls
on both CDAIObjectMPD and PrivateCDAIObjectMPD
- admanager_mpd.cpp: full implementations with sentinel fast-path and
mutex-guarded map; fires VodAdBreakOpportunityEvent via SendEvent async
- priv_aamp.h/cpp, main_aamp.h/cpp: API wired through player stack
- jsbindings/jsutils.cpp: event name in both tables
- jsbindings/jsbindings.cpp: AAMP_JSListener_VodAdBreakOpportunity,
AAMP_registerVodAdBreak, AAMP_cancelVodAdBreak JS handlers + method table
- fragmentcollector_mpd.cpp: FetcherLoop VOD lookahead block (VOD+CDAI only)
- test/utests/fakes: FakeAdManager, FakeAampEvent, FakePrivateInstanceAAMP,
FakePlayerInstanceAamp updated with matching stubs
- test/utests/tests/StreamAbstractionAAMP_MPD: FetcherLoopTests and
FunctionalTests mDefaultIntConfigSettings updated with new config key
Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com>
* VPAAMP-420 address review comments for Phase 1 VOD CDAI
- admanager_mpd.cpp: fix data race in CheckVodAdBreakLookahead() by
acquiring mDaiMtx before the fast-path sentinel comparison, ensuring
mNextVodBreakToCheck is always read under the lock.
- fragmentcollector_mpd.cpp: clamp eAAMPConfig_VodAdBreakLookaheadSec
to >= 0.0 at the call-site so a misconfigured negative value degrades
to point-in-time matching rather than look-behind behaviour.
- AampEvent.cpp: reformat VodAdBreakOpportunityEvent constructor and
accessors to tab indentation with trailing ':' on the signature line,
matching the style of MonitorAVStatusEvent and the rest of the file.
- test/utests/tests/AdManagerMPDTests/FunctionalTests.cpp: add three
unit tests to AdManagerMPDTests:
VodAdBreak_OpportunityFiresOnceInWindow: event fires exactly once
entering the lookahead window; does not re-fire; sentinel resets
to max() after last break is consumed.
VodAdBreak_CancelledBreakNeverFires: cancelled break is skipped by
CheckVodAdBreakLookahead; sentinel advances to next active break.
VodAdBreak_SentinelTracksEarliestActiveBreak: sentinel correctly
follows earliest active break through Register/Cancel sequences.
Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* VPAAMP-420 add registerVodAdBreak/cancelVodAdBreak CLI commands, VOD opportunity event listener, and L1 config round-trip tests
* Reason for Change: config tests
Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com>
* VPAAMP-420 fix vodAdBreakOpportunity not firing for pre-tune registerVodAdBreak
Three bugs prevented the vodAdBreakOpportunity event from firing in the
L2 test AAMP-CDAI-8023:
1. priv_aamp.h/.cpp: RegisterVodAdBreak called before tune drops breaks
mCdaiObject is created during TuneHelper/Init, so a pre-tune call to
RegisterVodAdBreak found mCdaiObject null and silently discarded the
break. Fix: buffer registrations in mPendingVodAdBreaks; replay them
into mCdaiObject immediately after new CDAIObjectMPD(this) in
TuneHelper. CancelVodAdBreak removes from the pending queue when
mCdaiObject is null. mPendingVodAdBreaks is cleared on Stop.
2. admanager_mpd.cpp: ResetState() wiped registered breaks at Init time
StreamAbstractionAAMP_MPD::Init() calls mCdaiObject->ResetState() as
its first action. ResetState previously cleared mVodAdBreaks and
reset mNextVodBreakToCheck, erasing all breaks that had just been
replayed in step 1. Fix: remove those two lines from ResetState();
the entire CDAIObjectMPD object is destroyed by SAFE_DELETE on
retune/stop, so explicit clearing is not needed.
3. fragmentcollector_mpd.cpp: CheckVodAdBreakLookahead used wrong position
Both call sites passed mBasePeriodOffset (the downloader position),
which plateaus at the time-based buffer limit (~10 s) and never
advances to the insertion-point trigger threshold (25 s for a 30 s
break with 5 s lookahead). Fix: pass aamp->GetPositionSeconds()
(rendered playback position) instead. Also moved the check inside
the inner segment-download while-loop so it is evaluated on every
iteration; the outer period-selection do-while runs only once for a
single-period VOD and would miss the window entirely.
---------
Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Reason for change: Handle ad breaks closed with NotifyReservationComplete and invalidate empty ad breaks to prevent ad state issues during trickplay. Risks: Low Test Procedure: Test with various ad break scenarios during trickplay Priority: P1 Change-Id: Ie25d404ec0edf9bed7ed0c9299fa8a0eda0d360e Signed-off-by: Nandakishor U M <nu641001@gmail.com>
…1598) Reason for Change: Fixes a crash in AAMP’s curl handle reuse path (CurlStore::CreateCurlStore) by ensuring the libcurl share lock userdata has process lifetime, preventing use-after-free when libcurl invokes share lock/unlock callbacks asynchronously. Also: - remove vestigial pstShareLocks, dead NULL check, and no-op nullptr assignments - Remove CurlDataShareLock *pstShareLocks from curlstorestruct; lock lifetime is now managed solely by the static CurlStore::mSharedCurlLock - Remove dead NULL==CurlSock check in CreateCurlStore (plain new throws std::bad_alloc; never returns nullptr) - Replace no-op nullptr/0 field assignments that immediately precede erase() or SAFE_DELETE with explanatory comments in ~CurlStore, RemoveCurlSock, and FlushCurlSockForHost - Fix pre-existing typo: nulptr -> nullptr in SaveCurlHandle comment Test Procedure: refer ticket Risks: Low Priority: P1 --------- Co-authored-by: pstroffolino <Philip_Stroffolino@cable.comcast.com>
) Reason for Change: New test suite: test/utests/tests/AampCurlStore/ Four tests exercising the CURLSHOPT_USERDATA path: T1 CreateCurlStore_UserDataIsSameStaticPointerAcrossHosts Two different hostnames must receive the same CURLSHOPT_USERDATA pointer. On old code each host got a distinct heap allocation (different addresses -> FAIL); fixed code passes &mSharedCurlLock for every host (same address -> PASS). T2 CreateCurlStore_UserDataSameAfterEntryRecreation After a store entry is evicted and its hostname re-added, the new CURLSHOPT_USERDATA must match the original. Old code produced a fresh heap allocation on each CreateCurlStore -> FAIL. T3 LockCallback_SafeToCallAfterStoreEviction (ASAN regression) Captures CURLSHOPT_USERDATA and the lock/unlock function pointers during CreateCurlStore, then triggers eviction of that entry via RemoveCurlSock (old code: SAFE_DELETE(pstShareLocks) freed the lock object here), then invokes the production callbacks with the captured USERDATA. Under ASAN, old code triggers use-after-free; fixed code passes because USERDATA is the static lock. T4 LockCallback_HandlesAllLockDataTypes Invokes lock/unlock callbacks for CURL_LOCK_DATA_DNS, CURL_LOCK_DATA_SSL_SESSION, and the generic fallback (connects). Verifies all three CurlDataShareLock mutex fields are exercised. Supporting infrastructure changes: - MockCurl.h: add curl_share_init, curl_share_setopt_ptr/func_lock/ func_unlock/long, curl_share_cleanup mock methods - FakeCurl.cpp: wire curl_share_init, curl_share_setopt (variadic), curl_share_cleanup through MockCurl (replaces no-op stub) - MockAampUtils.h: add getHostFromURL and isLocalHost mock methods - FakeAampUtils.cpp: wire aamp_getHostFromURL and aamp_IsLocalHost through MockAampUtils when the mock is set - tests/CMakeLists.txt: register AampCurlStore subdirectory
* VPAAMP-31: VOD CDAI with mp4demux Reason for change: Make pre roll ad to play via encrypted pipeline Test Procedure: Updated in ticket Risks: Medium Signed-off-by: Reshma-JO07 <sreshmaraphaelk@gmail.com> * VPAAMP_31-1: add review comments and consolidate test cases - fragmentcollector_mpd.cpp: add block comment above mediaType/curlInstance assignment explaining the UseMp4Demux DownloadFragment vs CacheFragment path and the per-track curl slot selection logic; remove leftover debug AAMPLOG_WARN lines - FragmentCollectorMpdTestCases.cpp: merge three Mp4DemuxEnabled test cases (initSegment flag, URL propagation, ActiveDownloadInfo cleared) into single CacheEncryptedHeader_Mp4DemuxEnabled test; merge three Mp4DemuxDisabled test cases into single CacheEncryptedHeader_Mp4DemuxDisabled test - FetcherLoopTests.cpp: replace terse 'Initialize MPD' comment on all CacheFragment EXPECT_CALL registrations with explanatory comments describing why the expectation must be set before InitializeMPD() and what the isInitSegment=true constraint asserts * VPAAMP_31-1: fix DetectDiscotinuityAndFetchInitTests1 missing profileChanged setup Commit 64870ea added a Times(1) EXPECT_CALL for video_p1_init.mp4 in DetectDiscotinuityAndFetchInitTests1. This expectation is never satisfied because FetchAndInjectInitialization only downloads an init segment when profileChanged or discontinuity is true. In production, StreamSelection() resets enabled=false before UpdateTrackInfo is called, which causes UpdateTrackInfo to flip profileChanged=true on the false->true transition. The test calls InvokeUpdateTrackInfo directly, bypassing StreamSelection(), so profileChanged stays false and the init segment fetch is never triggered. Fix: set pMediaStreamContext->profileChanged=true explicitly after the period switch to replicate the state that exists when DetectDiscontinuityAndFetchInit is called in production. * Update fragmentcollector_mpd.cpp --------- Signed-off-by: Reshma-JO07 <sreshmaraphaelk@gmail.com> Co-authored-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com>
…es() (#1590) * Fix for out-of-bound access in CheckPreferredTextLanguages() * L1 test to validate the fix --------- Signed-off-by: Abhi-jith-S <abhijithssa7@gmail.com> Co-authored-by: pstroffolino <Philip_Stroffolino@cable.comcast.com>
* Revert "VPAAMP-529 [AAMP][SKY][XiOne-DE] HD VOD playback fails after ad start with "Please wait until the program is loaded" error (#1582)" This reverts commit 03d1b18. * VPAAMP-569: Bring back "VPAAMP-529 Stop Underflow monitoring at EOS during VOD" fix Reason for change: Replace the position-based EOS boundary guard (eosEndToleranceSec config) with a clean fix: call StopUnderflowMonitor() from MediaTrack::SignalIfEOSReached() so it stops for both normal EOS-sentinel injection path and aborted-wait EOS path. * VPAAMP-569: Optimize Stop Underflow monitoring logic at EOS during VOD Reason for change: Replace the position-based EOS boundary guard (eosEndToleranceSec config) with a clean fix: call StopUnderflowMonitor() from MediaTrack::SignalIfEOSReached() so it stops for both normal EOS-sentinel injection path and aborted-wait EOS path.
* VPAAMP-588: Disable pre-tune Reason for Change: fake tune disabled for all versions of AAMP Test Procedure: refer ticket Risks: Low Priority: P1 --------- Co-authored-by: pstroffolino <Philip_Stroffolino@cable.comcast.com>
* VPAAMP-455 AAMP L1 tests should not use middleware mocks Reason for Change: AAMP should be independent from middleware Summary of Changes: - Add MockPlayerScheduler.h - Add MockGStreamer.h - Add MockGstHandlerControl.h - Add MockPlayerUtils.h - Modify CMakeLists.txt files to use AAMP mocks instead of middleware Test Procedure: Run AAMP L1 tests Priority: P1 Risks: Low * Add AAMP L1 FakeGStreamer and address review comments * Fix L1 test build failure and address review comments * Modified gst_caps_new_simple() fake to address review comment * Fix L1 test failure after rebase * Remove gst_event_new_segment mock to address review comments --------- Co-authored-by: pstroffolino <Philip_Stroffolino@cable.comcast.com>
…tions (#1627) Reason for Change: Adds abr.instructions.md (scoped to abr/**) so Copilot automatically applies the normative ABR/latency-control spec when editing that module. Adds five /abr-* prompt files for structured compliance reviews, PR diffs, log validation, function-level checks, and instrumentation planning. Wires both into copilot-instructions.md and the instructions README. Co-authored-by: pstroffolino <Philip_Stroffolino@cable.comcast.com>
* VPAAMP-428: Add support for meta, sgpd, and sbgp MP4 boxes - ParseMetaBox: handles QTFF short-atom and ISO BMFF full-atom variants by peeking at payload bytes to detect encoding before recursing into child boxes (mirrors GStreamer qtdemux.c dual-variant logic) - ParseSampleGroupDescription: parses sgpd (ISO 14496-12 s8.9.2), reads grouping_type and version-dependent layout (v0/v1/v2), validates per-entry boundaries; seig full parsing deferred - ParseSampleToGroup: parses sbgp (ISO 14496-12 s8.9.3), reads grouping_type, optional grouping_type_parameter (v1), validates and consumes all sample_count/group_description_index pairs - DemuxHelper: add dispatch cases for meta, sgpd, sbgp * VPAAMP-428: fix sgpd v0 ptr not advanced, meta ilst/keys boundary mismatch; add tests - ParseSampleGroupDescription: after the entry loop, advance ptr to next for version 0 where no defaultLength field exists and per-entry sizes are unknown, preventing ptr != next DATA_BOUNDARY_MISMATCH in DemuxHelper. - DemuxHelper: add 'ilst' and 'keys' to the explicit skip-list so that Apple metadata children of a meta box are consumed cleanly rather than falling through to the default handler and leaving ptr unchanged. - Update ParseMetaBox doc comment to accurately describe the dispatch behaviour for unrecognised child boxes. - BoxParsingTests: add 13 new tests (suite Mp4Demux_NewBoxParsers) covering meta QTFF/ISO-BMFF/unknown variants, ilst and keys regression cases, sgpd v0/v1-fixed/v1-per-entry/v2/boundary-error, and sbgp v0/v1/boundary-error. * VPAAMP-428: fix sgpd version-dependent payload bounds check Replace the flat 8-byte minimum check in ParseSampleGroupDescription with a version-aware calculation: v0: 8 bytes (grouping_type + entry_count) v1: 12 bytes (+ default_length) v2: 16 bytes (+ default_group_description_index) The check is now validated against next (the box boundary) rather than endPtr, preventing a malformed short box from silently over-reading into adjacent sibling boxes within the same segment buffer. Add two unit tests: SgpdVersion1_PayloadTooShortForVersion_RaisesError SgpdVersion2_PayloadTooShortForVersion_RaisesError * VPAAMP-428: fix sbgp v1 bounds check; simplify meta peek with save/restore ptr * VPAAMP-428: fast retry on non-timeout manifest errors; add unit test
* VPAAMP-444 - aamp-cli - ff and rew not working fully Reason for change: To fix fragmentskip logic for segment base streams Fix wrong byte range after SegmentBase trickplay transition SkipFragments now unconditionally recomputes fragmentOffset from the sidx index range on every call, instead of only when fragmentIndex==0. Root cause: FetchAndInjectInitialization resets fragmentOffset to 0 and clears the IDX between the initial SeekInPeriod (which correctly computed the offset) and the FetcherLoop's trickplay SkipFragments call. Because fragmentIndex was non-zero, the old code skipped the offset initialisation, leaving fragmentOffset=0 and causing the first trickplay fragment to be fetched from byte 0 of the mp4 file instead of the correct iframe position. Fix SegmentBase rewind-to-beginning BOS bounce When rewinding in SegmentBase streams, the player would reach fragment 0 but never signal BOS/EOS, causing it to endlessly re-fetch the first fragment instead of bouncing back to 1x playback. Root cause: PushNextFragment increments fragmentIndex after fetching, so SkipFragments never sees fragmentIndex==0 on re-entry. The rewind walk path lands on fragment 0 each iteration but never set mReachedFirstFragOnRewind or eos. Fix: Two-pass detection in the rewind walk post-processing: - First landing on fragment 0: set mReachedFirstFragOnRewind - Second landing: mReachedFirstFragOnRewind already set, set eos=true to trigger the BOS bounce back to normal playback. Also added a guard for the (theoretical) case where fragmentIndex is literally 0 at SkipFragments entry with negative skipTime. Risks: Low Priority: P1 --------- Signed-off-by: Rajat <emailofrajatyadav@gmail.com> Co-authored-by: pstroffolino <Philip_Stroffolino@cable.comcast.com>
Reason for Change: Disabled L1 test no longer needed Summary of Changes: - Delete InterfacePlayerRDKTests files Test Procedure: Run AAMP L1 tests Priority: P2 Risks: Low Co-authored-by: pstroffolino <Philip_Stroffolino@cable.comcast.com>
Reason for Change: state check to avoid logging Audio/Video EOS Marked multiple times Risk: Low
#1548) * RDKEMW-19029: Observed Video Freeze & Fast-Forward Effect After Channel Change Reason for change: Added retry logic to AAMP downloads when curl returns error 55 (CURL_SEND_ERROR) Risks: Low Test Procedure: Refer jira ticket Priority: P0 Signed-off-by: Gnanesha <gnaani82@gmail.com> * Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * RDKEMW-19029: Added L1 test AampCurlDownloader_Retry_SendError Signed-off-by: psiva01 <sivasubramanian.patchaiperumal@ltts.com> --------- Signed-off-by: Gnanesha <gnaani82@gmail.com> Signed-off-by: psiva01 <sivasubramanian.patchaiperumal@ltts.com> Co-authored-by: Sivasubramanian Patchaiperumal <sivasubramanian.patchaiperumal@ltts.com> Co-authored-by: pstroffolino <Philip_Stroffolino@cable.comcast.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…is full (#1640) * VPAAMP-615 - Download stops when playing from TSB and fragment Cache is full The scenario occurred after tuning to channel and performing pause-play-pause sequence. If the fragment cache is full, the while loop will spin indefinitely preventing download. Fix: When playing from TSB any wait for free cache segment is bypassed Also adds L1 microtest to verify the fix Test Instructions: - Refer ticket Risks Low * Simplifying the comments --------- Co-authored-by: pstroffolino <Philip_Stroffolino@cable.comcast.com>
* VPAAMP-565: Deprecate ceaFormat from AAMP Config. Reason for Change: Deprecated ceaFormat from AAMP Test Procedure: see ticket Priority: P1 Risks: Low * VPAAMP-565: Deprecate ceaFormat from AAMP Config. Reason for Change: Deprecated ceaFormat from AAMP Test Procedure: see ticket Priority: P1 Risks: Low --------- Co-authored-by: pstroffolino <Philip_Stroffolino@cable.comcast.com>
* VPAAMP-473:[Timeshift DAI] Place CDAI ads for cold CDVR and iVOD Reason for change:Cold CDVR/static manifests never refresh, so ad placement was never triggered after fulfillment, causing playback to freeze waiting for ads that were never mapped.Fixed by immediately calling ad placement as soon as an ad is resolved, instead of waiting for a manifest refresh that never comes. Risks: p1 Signed-off-by: varshnie <varshniblue14@gmail.com> * VPAAMP-473:[Timeshift DAI] Place CDAI ads for cold CDVR and iVOD Reason for change: Fix L1 failures Test Procedure: L1 should pass Risks: None Signed-off-by: Vinish100 <vinish.balan@gmail.com> --------- Signed-off-by: varshnie <varshniblue14@gmail.com> Signed-off-by: Vinish100 <vinish.balan@gmail.com> Co-authored-by: Vinish K B <vinish.balan@gmail.com> Co-authored-by: pstroffolino <Philip_Stroffolino@cable.comcast.com>
Reason for change: Invoke FoundEventBreak for both static and dynamic manifests when CDAI is enabled to support CDAI on cold CDVR. Avoid calling onAdEvent from init() to prevent processing DAI events during the tune-time period for a new tune.Added L1s. Test Procedure: Validate CDAI ads are played in CDVR and iVOD content Priority: P1 Signed-off-by: srikanthreddybijjam-comcast <srikanthreddybijjam.2000@gmail.com> Co-authored-by: vinodkadungoth <vinodkadungoth@gmail.com> Co-authored-by: pstroffolino <Philip_Stroffolino@cable.comcast.com>
Reason for Change: Fix AAMP L1 test build failure on Ubuntu Summary of Changes: - Update the script to install the middleware interfaces Test Procedure: Build AAMP L1 tests Priority: P2 Risks: Low
…1658) Fixes steady memory growth in the local TSB trickplay path by ensuring ISO-BMFF segments truncated to a single I-frame do not retain their original over-allocated std::vector capacity, and simplifies the MP4 demuxer trickmode sample handling accordingly.
Reason for change: post roll cdai support on static manifest Risks: p1 This PR adds support for CDAI post-roll ads on cold-CDVR/iVOD (static MPD) so that, after the post-roll ad finishes, playback correctly transitions to EOS instead of attempting a manifest refresh, and queued CDAI ad events are flushed before the app receives EOS Signed-off-by: varshnie varshniblue14@gmail.com Co-authored-by: pstroffolino <Philip_Stroffolino@cable.comcast.com>
#1639) * test: L1 regression test for SegmentBase ABR switch byte-range base offset Add DownloadFragment_SegmentBase_ABRSwitch_UsesIdxBaseOffset to FragmentDownloadTests to deterministically cover the race introduced by VPAAMP-363 removing the serial-download guard for SegmentBase streams. Before the fix, DownloadFragment computed the byte range for an ABR-switched SegmentBase segment as (0 + 1 + first_offset), which landed inside the moov/SIDX prefix and returned duration=0 from the IsoBmff parser. This produced before=0/after=0 in RestampPts log, causing intermittent failure in test_8003_0[...main-segmentbase.mpd] (PTS actual 0s, expected 76800s). The test sets mIdxBaseOffset=1000, fragmentDescriptor.Bandwidth=5000000 (1080p), and passes a stale dlInfo with bandwidth=1400000 (480p) and an empty uriList. DownloadFragment executes the ABR switch range computation then returns false before issuing any network request (no mocks required). Assert: dlInfo->range == "17384-29671" (mIdxBaseOffset + ref[0].size as start, + ref[1].size - 1 as end) and fragmentDurationSec == 2.0f. * fix: thread mIdxBaseOffset through SegmentBase IDX load/clear/use VPAAMP-363 removed the DashParallelFragDownload=false override from SkipFragments for SegmentBase streams, enabling parallel downloads. This introduced a race: a stale ABR download job could call DownloadFragment while the FetcherLoop had already advanced to a new profile. The ABR switch branch recomputed fragmentOffset as (0 + 1 + first_offset), landing inside the moov/SIDX prefix and fetching garbage. The IsoBmff parser returned duration=0, producing before=0/after=0 in RestampPts log and causing intermittent failure in test_8003_0[...main-segmentbase.mpd] (PTS actual 0s, expected 76800s). Changes: - MediaStreamContext.h: add mIdxBaseOffset{0} (uint64_t) field alongside IDX; tracks the byte position of segment 0 in the file for the currently loaded IDX profile. Add mIdxMutex (std::mutex) to protect IDX and mIdxBaseOffset against concurrent access by FetcherLoop (writer) and download worker threads (reader). - fragmentcollector_mpd.cpp (FetchFragment lazy-load): after computing fragmentOffset = end_of_SIDX + 1 + first_offset, snapshot it into mIdxBaseOffset so DownloadFragment can use it on ABR switches without recomputing from zero. - fragmentcollector_mpd.cpp (FetchAndInjectInitialization): clear mIdxBaseOffset=0 inside the mIdxMutex scope alongside the existing ClearAndRelease(IDX), keeping the two values consistent. - MediaStreamContext.cpp (DownloadFragment ABR switch): replace the old fragmentOffset=0+1+first_offset recomputation with dlInfo->fragmentOffset = mIdxBaseOffset, then accumulate referenced_size values from ParseSegmentIndexBox to reach the correct segment start byte. * fix: address review comments - Lock mIdxMutex around IDX offset computation in PushNextFragment - Correct comment wording in MediaStreamContext (missed -> landed inside) - Normalize test indentation and replace std::cbegin/cend with pointer arithmetic * fix: snapshot IDX under mIdxMutex in PushNextFragment segment-fetch block The outer IDX.empty() check and both ParseSegmentIndexBox calls in the segment-fetch block of PushNextFragment ran without mIdxMutex, exposing a check-then-act race with FetchAndInjectInitialization (worker thread) which calls ClearAndRelease(IDX) under the mutex: Thread A (FetcherLoop): IDX.empty() -> false Thread B (worker): ClearAndRelease(IDX) <- IDX freed Thread A (FetcherLoop): IDX.data() dereference <- use-after-free Fix: take a local idxSnapshot copy of IDX under mIdxMutex before the segment-fetch block, then parse from the snapshot. The mutex is released before FetchFragment so we never hold it across blocking network I/O. Addresses review comment on feature/vpaamp-363-follow-up PR #1533. * fix: address PushNextFragment review comments (IDX load and EOS clear) Two issues raised in code review on PushNextFragment SegmentBase path: 1. mIdxMutex held across LoadIDX network I/O (lines R1629-R1632) LoadIDX calls GetFile() which performs network I/O and can block for hundreds of milliseconds. Holding mIdxMutex across this call stalls any concurrent thread needing the mutex (e.g. DownloadFragment during an ABR switch). Fix: download into a local loadedIdx buffer with no lock held, then move into IDX under mIdxMutex only if IDX is still empty (matching the pattern in SkipFragments). 2. mIdxBaseOffset not reset on EOS ClearAndRelease (lines R1750-R1754) When the SIDX is exhausted, IDX is cleared but mIdxBaseOffset was left at its last value. A subsequent ABR-switch job reading mIdxBaseOffset before the next IDX load would see a stale base offset. Fix: reset mIdxBaseOffset = 0 alongside ClearAndRelease(IDX) under mIdxMutex. * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix: VPAAMP-614 three remaining SegmentBase race conditions 1. PushNextFragment IDX.empty() TOCTOU (line ~1614): Read IDX.empty() without mIdxMutex exposed a check-then-act race against FetchAndInjectInitialization's ClearAndRelease. Fix: use the shouldLoadIdx pattern (same as SkipFragments) so the read is always under the lock. 2. FetchAndInjectInitialization atomicity (line ~8825): fragmentOffset=0 was written outside mIdxMutex, so a concurrent DownloadFragment ABR-switch thread could read IDX (non-empty) and then race to use fragmentOffset=0, computing a byte range starting at byte 0 of the media file (moov box) -> GStreamer error 80:1 on production VOD content. Fix: move fragmentOffset=0 inside the existing mIdxMutex critical section alongside ClearAndRelease(IDX) and mIdxBaseOffset=0. 3. Stop() path data race (line ~11208): ClearAndRelease(track->IDX) was called with no lock held, creating a potential use-after-free if PushNextFragment/DownloadFragment were concurrently dereferencing IDX.data(). Fix: wrap in mIdxMutex. Tests: three regression tests added to FragmentDownloadTests.cpp - SegmentBase_FetchAndInjectInit_ResetsAreAtomic - SegmentBase_IDXEmptyCheck_MutexGuard_NoStaleRead - SegmentBase_StopPath_ClearAndRelease_IsLocked * fix: VPAAMP-614 run SegmentBase init synchronously to prevent GStreamer invalid-data error For SegmentBase content the FetcherLoop returns immediately after SubmitJob() and races ahead: it loads IDX and submits media segment[0] to the worker queue while the async init download is still in flight. fragmentOffset is mutated on the FetcherLoop thread concurrently with the worker thread's DownloadFragment path, and GStreamer receives media data before the init segment, producing the 'file is invalid' error (code 80:1). Detect SegmentBase init via the non-empty byte-range on an init segment (segmentBaseInit = isInitializationSegment && !range.empty()) and fall through to the existing synchronous Execute() path for that case only. All other downloads (SegmentTemplate init, media segments) continue to use the parallel worker as before. * fix: VPAAMP-614 extend SegmentBase synchronous fix to all downloads Original fix only covered init segments (isInitializationSegment && !range.empty()) but SegmentBase media segments also need protection. The race condition occurs when FetcherLoop continues after SubmitJob returns and can race ahead with IDX loading and subsequent segment submissions while async downloads are still in flight. Extend the condition to cover all SegmentBase content (!range.empty()) so both init and media segments download synchronously, guaranteeing strict ordering and preventing GStreamer from receiving media data before init data. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* VPAAMP-454 Enable AAMP L1 AampGstPlayer tests Reason for Change: Protect against regressions in AampGstPlayer code Summary of Changes: - Add AampGstPlayer L1 test directory - Add L1 AampGstPlayerMain file - Remove middleware code from L1 AampGstPlayer CMake file - Delete tests that verified middleware code - Update existing L1 tests - Add middleware fakes Test Procedure: Run AAMP L1 tests Priority: P1 Risks: Low * Update copyright year to 2026 for new files added * Apply Copilot suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Do not initialize mProtectionLock for InterfacePlayerRDK fake --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
#1663) * VPAAMP-580 Disable LatencyMonitor earlier in pause and trickplay paths Reason for Change: Required for DirectRialto Summary of Changes: - Disable LatencyMonitor earlier in SetRateInternal(0) - Disable LatencyMonitor in SetRateInternal() for trickmodes Test Procedure: Check LatencyMonitor during pause and trick modes Priority: P1 Risks: Low * Add WARN when pipeline is running but position does not change * Verify changes in L1 test --------- Co-authored-by: pstroffolino <Philip_Stroffolino@cable.comcast.com>
…le (#1629) This PR addresses the issue of inconsistent CC behaviour on return to normal play after trick / seek where the app selected CC track is dropped in favour of the default TTML track with DASH streams containing both CC and OOB subs With fix, we skip OOB subtitle scoring on seek/trickplay resume when the app explicitly selected a CC track. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…nfig (#1545) * VPAAMP-490: Update latencyParams present in manifest as stream settings Reason for change: When latencyParams are present in the manifest, they should take precedence over the default values provided. Risks: Low Test Procedure: Test with LLD streams Priority: P1 Change-Id: I6a1efdee4c654012377604c31e9a221cd70aea68 Signed-off-by: Nandakishor U M <nu641001@gmail.com> * Reason for Change: l1test fix Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com> --------- Signed-off-by: Nandakishor U M <nu641001@gmail.com> Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com> Co-authored-by: pstroffolino <Philip_Stroffolino@cable.comcast.com>
suryaiyappan2k
merged commit Jun 30, 2026
52c070c
into
feature/RDKEMW-17870_DEBUG
4 of 5 checks passed
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
suryaiyappan2k
added a commit
that referenced
this pull request
Jun 30, 2026
Merge changes from dev_sprint_25_2 to feature branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
17 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merge changes from current branch to the feature branch for debugging.