Feature/vplay 12682 optimization by varshnie · Pull Request #1217 · rdkcentral/aamp

varshnie · 2026-03-23T12:12:16Z

Reason for change:AampUnderflowMonitor Optimization
Risks: Medium

Signed-off-by: varshnie <varshniblue14@gmail.com>

Reason for change:follow-up improvements for AampUnderflowMonitor Risks: Medium Signed-off-by: varshnie <varshniblue14@gmail.com> VPLAY-12605:[L1 Test] AampUnderflowMonitor Tests Reason for change:Added L1 for AampUnderflowMonitor Test Procedure:Refer Jira Ticket Risks: Medium Signed-off-by: varshnie <varshniblue14@gmail.com> Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…tionAAMP without stopping the underflow monitor first. Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com>

…monitor in streamabstarction stop

Copilot

Pull request overview

This PR optimizes and refactors the AampUnderflowMonitor lifecycle (creation/start/stop), adjusts buffering event semantics, and adds/extends unit test coverage around the monitor and its integration with StreamAbstractionAAMP.

Changes:

Initialize AampUnderflowMonitor from StreamAbstractionAAMP construction when enabled by config, and start/stop it from protocol-specific Start/Stop paths.
Update buffering-change API semantics (SendBufferChangeEvent(bool bufferingStart)) and tune LLD underflow buffer thresholds.
Add a new AampUnderflowMonitor unit test target plus additional integration tests/mocks.

Reviewed changes

Copilot reviewed 19 out of 21 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`streamabstraction.cpp`	Creates underflow monitor in `StreamAbstractionAAMP` ctor; simplifies Start/Stop/IsRunning wrappers.
`StreamAbstractionAAMP.h`	Adds stronger lifecycle note for `StopUnderflowMonitor`; removes mutex member.
`AampUnderflowMonitor.h`	Adds CV-based interruptible sleep infrastructure; renames thread member.
`AampUnderflowMonitor.cpp`	Refactors thread lifecycle + polling logic; adds `WaitMs()`.
`priv_aamp.h`	Renames/clarifies `SendBufferChangeEvent` parameter meaning.
`priv_aamp.cpp`	Updates buffering change emission and adjusts `SetBufferingState` flow; removes monitor start/stop from some tune/teardown paths.
`fragmentcollector_hls.cpp`	Starts/stops underflow monitor during HLS Start/Stop.
`fragmentcollector_mpd.cpp`	Starts/stops underflow monitor during MPD Start/Stop.
`fragmentcollector_progressive.cpp`	Minor formatting cleanup in `Stop()`.
`AampDefine.h`	Adds LLD-specific underflow low/high buffer constants.
`test/utests/tests/CMakeLists.txt`	Adds `AampUnderflowMonitor` test subdirectory.
`test/utests/tests/AampUnderflowMonitor/CMakeLists.txt`	New test executable for underflow monitor.
`test/utests/tests/AampUnderflowMonitor/FunctionalTests.cpp`	New functional tests for monitor Start/Stop and underflow transitions.
`test/utests/tests/AampUnderflowMonitor/AampUnderflowMonitorTest.cpp`	Test binary `main()`.
`test/utests/tests/StreamAbstractionAAMP/FunctionalTests.cpp`	Adds wrapper API tests for Start/Stop underflow monitor + mock wiring.
`test/utests/tests/StreamAbstractionAAMP/CMakeLists.txt`	Minor formatting tweak.
`test/utests/mocks/MockPrivateInstanceAAMP.h`	Adds mock hooks used by underflow monitor tests (`SetBufferingState`, `IsSinkCacheEmpty`).
`test/utests/mocks/MockAampUnderflowMonitor.h`	New mock interface + global pointer for fake underflow monitor plumbing.
`test/utests/fakes/FakePrivateInstanceAAMP.cpp`	Routes `SetBufferingState` / `IsSinkCacheEmpty` through the mock when present; signature rename.
`test/utests/fakes/FakeAampUnderflowMonitor.cpp`	Adds global mock pointer and forwards Start/Stop to it.
`test/utests/drm/mocks/aampMocks.cpp`	Updates signature name for `SendBufferChangeEvent`.

StreamAbstractionAAMP.h

fragmentcollector_hls.cpp

AampUnderflowMonitor.cpp

priv_aamp.cpp

Copilot

Pull request overview

Copilot reviewed 20 out of 22 changed files in this pull request and generated 6 comments.

test/utests/tests/AampUnderflowMonitor/FunctionalTests.cpp

priv_aamp.cpp

Copilot · 2026-03-23T17:27:48Z

fragmentcollector_mpd.cpp

+		if(ISCONFIGSET(eAAMPConfig_EnableAampUnderflowMonitor))
+		{
+			StopUnderflowMonitor();
+		}


StreamAbstractionAAMP_MPD::Stop() only stops the underflow monitor when clearChannelData is true and the config flag is set. If the monitor was started and Stop(false) is used during teardown/retune paths, the monitor thread may keep running while the stream is being torn down. Consider stopping the monitor based on mUnderflowMonitor existence/running state rather than IsConfigSet, and not gating it on clearChannelData if the stream is about to be destroyed.

Copilot · 2026-03-23T17:27:48Z

fragmentcollector_hls.cpp

+	if(ISCONFIGSET(eAAMPConfig_EnableAampUnderflowMonitor))
+	{
+		StopUnderflowMonitor();
+	}


StreamAbstractionAAMP_HLS::Stop() stops the underflow monitor only when the config flag is set. If the monitor exists/runs but the config value changes (or differs between construction and stop), this can leave the monitor thread running during teardown. Prefer stopping based on mUnderflowMonitor existence/running state (or always calling StopUnderflowMonitor() and letting it no-op when null).

Suggested change

if(ISCONFIGSET(eAAMPConfig_EnableAampUnderflowMonitor))

{

StopUnderflowMonitor();

}

StopUnderflowMonitor();

Copilot · 2026-03-23T17:27:49Z

test/utests/tests/AampUnderflowMonitor/CMakeLists.txt

+set(AAMP_SOURCES ${AAMP_ROOT}/streamabstraction.cpp
+                 ${AAMP_ROOT}/CachedFragment.cpp
+                 ${AAMP_ROOT}/AampUnderflowMonitor.cpp)
+
+add_executable(${EXEC_NAME}
+               ${TEST_SOURCES}
+               ${AAMP_SOURCES})
+
+if (CMAKE_XCODE_BUILD_SYSTEM)
+  xcode_define_schema(${EXEC_NAME})
+endif()
+
+if (COVERAGE_ENABLED)
+    include(CodeCoverage)
+    APPEND_COVERAGE_COMPILER_FLAGS()
+endif()
+
+target_link_libraries(${EXEC_NAME} fakes -lpthread ${LIBDASH_LINK_LIBRARIES} ${GLIB_LINK_LIBRARIES} ${OS_LD_FLAGS} ${GMOCK_LINK_LIBRARIES} ${GTEST_LINK_LIBRARIES} ${LIBCJSON_LINK_LIBRARIES})


The new AampUnderflowMonitor test target links against the fakes library (which includes test/utests/fakes/FakeAampUnderflowMonitor.cpp via a file(GLOB ...)), while also compiling the real ${AAMP_ROOT}/AampUnderflowMonitor.cpp. That will produce duplicate symbol definitions for AampUnderflowMonitor::* at link time. Consider either (1) removing AampUnderflowMonitor.cpp from this test executable and relying on the fake, or (2) not linking fakes for this target (or excluding FakeAampUnderflowMonitor.cpp from fakes) so the real implementation is tested.

test/utests/tests/AampUnderflowMonitor/FunctionalTests.cpp

Reason for change:AampUnderflowMonitor Optimization Risks: Medium Signed-off-by: varshnie <varshniblue14@gmail.com>

varshnie and others added 3 commits March 23, 2026 12:53

Reason for Change: TeardownStream() stops and deletes mpStreamAbstrac…

44d2069

…tionAAMP without stopping the underflow monitor first. Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com>

Reason for Change:Addressed the L1 failure and stopped the underflow …

859e69e

…monitor in streamabstarction stop

varshnie requested a review from a team as a code owner March 23, 2026 12:12

Copilot AI review requested due to automatic review settings March 23, 2026 12:12

Copilot started reviewing on behalf of varshnie March 23, 2026 12:12 View session

Copilot AI reviewed Mar 23, 2026

View reviewed changes

StreamAbstractionAAMP.h Show resolved Hide resolved

fragmentcollector_hls.cpp Show resolved Hide resolved

AampUnderflowMonitor.cpp Show resolved Hide resolved

AampUnderflowMonitor.cpp Show resolved Hide resolved

priv_aamp.cpp Show resolved Hide resolved

varshnie force-pushed the feature/VPLAY-12682_optimization branch from 5d2b338 to 7134431 Compare March 23, 2026 14:57

Copilot AI review requested due to automatic review settings March 23, 2026 17:20

varshnie force-pushed the feature/VPLAY-12682_optimization branch from 7134431 to 0581631 Compare March 23, 2026 17:20

Copilot started reviewing on behalf of varshnie March 23, 2026 17:20 View session

Copilot AI reviewed Mar 23, 2026

View reviewed changes

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

b50643f

Reason for change:AampUnderflowMonitor Optimization Risks: Medium Signed-off-by: varshnie <varshniblue14@gmail.com>

varshnie force-pushed the feature/VPLAY-12682_optimization branch from 0581631 to b50643f Compare March 24, 2026 04:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/vplay 12682 optimization#1217

Feature/vplay 12682 optimization#1217
varshnie wants to merge 4 commits intodev_sprint_25_2from
feature/VPLAY-12682_optimization

varshnie commented Mar 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 23, 2026

Uh oh!

Copilot AI Mar 23, 2026

Uh oh!

Copilot AI Mar 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

varshnie commented Mar 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants