Revert "VPLAY-12701: Implement AampLatencyMonitor "

[pstroffolino]

Reverts #1178

Test to isolate reason for failed l2 tests

[Copilot]

Pull request overview

Reverts the previously introduced unified AampLatencyMonitor component and shifts low-latency DASH rate-correction/latency monitoring back into the MPD stream abstraction, along with related config key renames (ms → seconds) and unit test updates to isolate L2 failures.

Changes:

• Removes AampLatencyMonitor sources/tests and related config knobs.
• Adds MPD-side latency monitor thread + new PrivateInstanceAAMP LLD “adjust speed/current rate” accessors.
• Updates unit tests/mocks/docs to reflect the reverted architecture and renamed config keys.

Reviewed changes

Copilot reviewed 34 out of 34 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
test/utests/tests/fragmentcollector_mpd/fragmentcollector_mpd1.cpp	Adds mock for new LLD adjust-speed getter.
test/utests/tests/StreamAbstractionAAMP_MPD/MonitorLatencyTests.cpp	Adds new parameterized tests for MPD latency monitoring logic.
test/utests/tests/StreamAbstractionAAMP_MPD/FunctionalTests.cpp	Adds basic coverage calls for new latency APIs; updates LLD tests with new mock.
test/utests/tests/StreamAbstractionAAMP_MPD/CMakeLists.txt	Includes new MonitorLatencyTests.cpp in build.
test/utests/tests/PrivateInstanceAAMP/PauseOnPlaybackTests.cpp	Validates LLD adjust-speed behavior around pause paths.
test/utests/tests/PrivAampTests/PrivAampTests.cpp	Updates latency getter/setter names in tests.
test/utests/tests/CMakeLists.txt	Removes AampLatencyMonitorTests subdirectory.
test/utests/tests/AampLatencyMonitorTests/CMakeLists.txt	Deleted (removes dedicated latency monitor test target).
test/utests/tests/AampLatencyMonitorTests/AampLatencyMonitorTests.cpp	Deleted (test runner removed).
test/utests/tests/AampLatencyMonitorTests/AampLatencyMonitorTestCases.cpp	Deleted (monitor unit tests removed).
test/utests/mocks/MockStreamSink.h	Removes mocked SetPlayBackRate from StreamSink mock.
test/utests/mocks/MockPrivateInstanceAAMP.h	Adds LLD adjust-speed/current-rate mocks; removes latency-monitor mocks.
test/utests/fakes/FakePrivateInstanceAAMP.cpp	Removes unified monitor wiring; adds LLD adjust-speed/current-rate wrappers; renames latency getter.
test/utests/fakes/FakeAdManager.cpp	Removes ad “IsAdPlaying” fakes.
test/utests/fakes/FakeAampLatencyMonitor.cpp	Deleted (no longer needed after revert).
test/utests/drm/mocks/aampMocks.cpp	Removes latency monitor include/state; renames latency getter.
support/aampabr/HybridABRManager.h	Adds stored LLD current playback rate field.
streamabstraction.cpp	Comment tweak referencing monitorLatency.
priv_aamp.h	Removes unified monitor API/fields; adds LLD adjust-speed/current-rate API; renames latency getter/setter.
priv_aamp.cpp	Removes unified monitor creation/usage; updates rate-correction timing units and latency API; adds LLD adjust-speed/current-rate implementations.
main_aamp.cpp	Replaces latency monitor enable call with LLD adjust-speed toggle on pause.
fragmentcollector_mpd.h	Adds latency monitor thread hooks and LatencyStatus accessors/fields.
fragmentcollector_mpd.cpp	Implements MPD latency monitor thread and stop/join behavior; toggles LLD adjust-speed in tuning flows.
admanager_mpd.h	Removes IsAdPlaying methods from CDAI objects.
admanager_mpd.cpp	Removes IsAdPlaying implementations.
README.md	Renames latency monitor config keys (ms → seconds) and edits defaults text.
CMakeLists.txt	Removes AampLatencyMonitor from lib sources.
AdManagerBase.h	Removes default IsAdPlaying() API from base class.
AampLatencyMonitor.h	Deleted (reverted feature).
AampLatencyMonitor.cpp	Deleted (reverted feature).
AampDefine.h	Renames latency monitor defaults to seconds; adds LatencyStatus enum.
AampConfig.h	Renames latency monitor int settings; removes float settings tied to removed monitor.
AampConfig.cpp	Updates config lookup table for renamed latency monitor keys; removes removed float entries.
AAMP-UVE-API.md	Updates config documentation for renamed latency monitor keys/units.

Comments suppressed due to low confidence (1)

priv_aamp.cpp:2594

• IsAtLivePoint() now reads mpStreamAbstractionAAMP and mpStreamAbstractionAAMP->mIsAtLivePoint without holding mStreamLock. Since mpStreamAbstractionAAMP is mutated during tune/teardown, this can race and potentially dereference a freed pointer. Please restore appropriate locking (or another thread-safe mechanism) around accessing mpStreamAbstractionAAMP here.

bool PrivateInstanceAAMP::IsAtLivePoint()
{
	if (mpStreamAbstractionAAMP)
	{
		if (IsLiveStream())
		{
			return mpStreamAbstractionAAMP->mIsAtLivePoint;
		}
	}

[Copilot]

The README’s defaults for latencyMonitorDelay and latencyMonitorInterval (5 and 2) don’t match the code/config defaults (DEFAULT_LATENCY_MONITOR_DELAY is 9 and the interval default maps to 1 via AAMP_LLD_LATENCY_MONITOR_INTERVAL). Please update the README values (and explicitly call out the units) so they stay consistent with AampConfig.cpp / AampDefine.h and AAMP-UVE-API.md.

Suggested change

	latencyMonitorDelay Low Latency Monitor delay. Default is 5(DEFAULT_LATENCY_MONITOR_DELAY)
	latencyMonitorInterval Low Latency Monitor Interval. Default is 2(DEFAULT_LATENCY_MONITOR_INTERVAL)
	latencyMonitorDelay Low Latency Monitor delay. Default is 9s (DEFAULT_LATENCY_MONITOR_DELAY)
	latencyMonitorInterval Low Latency Monitor interval. Default is 1s (DEFAULT_LATENCY_MONITOR_INTERVAL)

[Copilot]

mCdaiObj is deleted unconditionally in TearDown(), but it is never initialized in SetUp(). If a test exits before calling InitializeMPD() (or a new test is added that doesn’t), this will delete an uninitialized pointer (UB). Please initialize mCdaiObj to nullptr in SetUp() and guard the delete with a null check (or manage it with a smart pointer).

[Copilot]

There are multiple assert(...) checks on pAampLLDashServiceData fields inside MonitorLatency(). Assertions in production code can crash debug builds and are compiled out in release, so they don’t provide reliable validation. Please convert these into runtime checks that log and skip/disable latency correction when the service description is missing/invalid.

Suggested change

	if( NULL != pAampLLDashServiceData )
	{
	assert(pAampLLDashServiceData->minLatency != 0 );
	assert(pAampLLDashServiceData->minLatency <= pAampLLDashServiceData->targetLatency);
	assert(pAampLLDashServiceData->targetLatency !=0 );
	assert(pAampLLDashServiceData->maxLatency !=0 );
	assert(pAampLLDashServiceData->maxLatency >= pAampLLDashServiceData->targetLatency);
	long currentLatency;
	if( NULL == pAampLLDashServiceData )
	{
	AAMPLOG_WARN("LLDASH: Service data is missing, skipping latency correction for this cycle");
	continue;
	}
	// Validate service latency configuration at runtime
	if( (pAampLLDashServiceData->minLatency == 0) ||
	(pAampLLDashServiceData->targetLatency == 0) ||
	(pAampLLDashServiceData->maxLatency == 0) ||
	(pAampLLDashServiceData->minLatency > pAampLLDashServiceData->targetLatency) ||
	(pAampLLDashServiceData->maxLatency < pAampLLDashServiceData->targetLatency) )
	{
	AAMPLOG_WARN(
	"LLDASH: Invalid service data (min=%lld, target=%lld, max=%lld), "
	"skipping latency correction for this cycle",
	(long long)pAampLLDashServiceData->minLatency,
	(long long)pAampLLDashServiceData->targetLatency,
	(long long)pAampLLDashServiceData->maxLatency);
	continue;
	}
	long currentLatency;

[Copilot]

SetLLDashCurrentPlayBackRate/GetLLDashCurrentPlayBackRate and SetLLDashAdjustSpeed/GetLLDashAdjustSpeed are used across multiple threads (latency monitor thread + main/playback threads), but they read/write plain double/bool without synchronization. This introduces a C++ data race (undefined behavior). Please make these fields atomic (e.g., std::atomic<bool> / std::atomic<double> with appropriate memory order) or protect accesses with an existing mutex.

[Copilot]

DEFAULT_LATENCY_MONITOR_INTERVAL is defined as 6, but the config default for latencyMonitorInterval is AAMP_LLD_LATENCY_MONITOR_INTERVAL (1) and the API docs list 1s. This mismatch makes defaults hard to reason about and can lead to incorrect documentation/config assumptions. Please align the macro(s) and config defaults (and clarify units) so there is a single consistent default interval value.

[Copilot]

mDefaultBoolConfigSettings initializes eAAMPConfig_EnableLowLatencyDash twice (first false, then true). With std::map initializer-list insertion, the second value is ignored, so the setting will remain false unexpectedly. Please remove the duplicate and keep a single intended value.

[Copilot]

StartLatencyMonitorThread() relies on assert(!latencyMonitorThreadID.joinable()) to ensure the thread isn’t already running. In release builds the assert is compiled out, and assigning to a joinable std::thread will call std::terminate(). Please replace the assert with a real runtime guard (e.g., check joinable() and return/log, or stop/join the existing thread first).

[Copilot]

currentLatency is calculated from DurationFromStartOfPlaybackMs()/GetPositionMs() (both long long) and then cast to long. On platforms where long is 32-bit this can truncate and produce incorrect latency, impacting rate correction decisions. Please keep this as a 64-bit type (e.g., long long/int64_t) end-to-end (including GetCurrentLatency() storage) or avoid the narrowing cast here.

Suggested change

	long currentLatency;
	if(aamp->mNewSeekInfo.GetInfo().isPopulated())
	{
	currentLatency = aamp->GetCurrentLatency();
	}
	else
	{
	currentLatency = static_cast<long>((aamp->DurationFromStartOfPlaybackMs()) - aamp->GetPositionMs());
	}
	AAMPLOG_TRACE("LiveLatency=%ld currentPlayRate=%lf dur:%lld pos:%lld",currentLatency, playRate, aamp->DurationFromStartOfPlaybackMs(), aamp->GetPositionMs());
	long long currentLatency;
	if(aamp->mNewSeekInfo.GetInfo().isPopulated())
	{
	currentLatency = aamp->GetCurrentLatency();
	}
	else
	{
	currentLatency = aamp->DurationFromStartOfPlaybackMs() - aamp->GetPositionMs();
	}
	AAMPLOG_TRACE("LiveLatency=%lld currentPlayRate=%lf dur:%lld pos:%lld", currentLatency, playRate, aamp->DurationFromStartOfPlaybackMs(), aamp->GetPositionMs());

[Copilot]

bufferLowCount / bufferLowHitCount are declared static inside MonitorLatency(). This makes the counters shared across all player instances/threads in the process, which can skew low-buffer detection and telemetry when multiple players run or when a new tune starts. Please make these per-instance state (member variables) or local non-static variables that reset appropriately.

Suggested change

	static int bufferLowCount = 0;
	static int bufferLowHitCount = 0;
	int bufferLowCount = 0;
	int bufferLowHitCount = 0;