Skip to content

VPAAMP-406 suppress noisy logging#1498

Closed
pstroffolino wants to merge 6 commits into
dev_sprint_25_2from
feature/VPAAMP-406
Closed

VPAAMP-406 suppress noisy logging#1498
pstroffolino wants to merge 6 commits into
dev_sprint_25_2from
feature/VPAAMP-406

Conversation

@pstroffolino

Copy link
Copy Markdown
Contributor

Reason for Change: tame noisy aampcli logging

aampcli is logging every manifest refresh.
This is excessive - there are events for this, that include manifest size, but logging every few seconds is excessive. This should be moved to trace level or removed.
We also have warning level logs present for segment and manifest downloads - this appears to be accidentally included as part of the network persona support in aamp.
As warn level logs, it'll show up by default, spewing noise into logs.
Lastly, we have a longstanding behavior where aamp by default logs all info level log messages, and 'info' log level choice is only honored once tune is complete. This is normally fine, but we have tests involving suppressDecode where the tune never succeeds, and we don't necessarily care about constant spew of info level logs.
Here, propose new semantic where log=WARN will force suppression of info level logs, even at tune time

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to reduce noisy default logging (especially in aampcli and network-persona paths), and to make “quiet” log-level selections (WARN/MIL/ERROR) take effect even during tune start. It also adds an optional MP4 segment integrity monitor and improves macOS build robustness around GStreamer discovery/installation.

Changes:

  • Suppress overly chatty aampcli manifest-refresh prints and comment out some accidental WARN-level persona download logs.
  • Add monitorMp4Integrity config + per-track persistent Mp4Demux validators to detect and harvest corrupt MP4 segments.
  • Adjust ConfigureLogSettings() so log=warn|mil|error locks the log level (preventing tune-time INFO elevation).

Reviewed changes

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

Show a summary per file
File Description
test/aampcli/Aampcli.cpp Removes noisy manifest refresh / manifest-size prints (currently via commented-out lines).
scripts/install_dependencies.sh Ensures GStreamer is available on macOS (framework if present, else Homebrew install) to avoid pkg-config/CMake failures.
scripts/install_aampcli.sh Makes macOS aampcli build pick GStreamer pkg-config paths from framework or Homebrew, with clearer errors.
priv_aamp.h Adds MP4 integrity monitoring API + persistent per-track Mp4Demux members.
priv_aamp.cpp Implements MP4 integrity parsing/dumping and suppresses a specific persona WARN log (commented out).
MediaStreamContext.cpp Early-exits fragment injection when suppressDecode is enabled.
downloader/AampCurlDownloader.cpp Suppresses a personaActive WARN log (commented out).
AampConfig.h Adds new bool setting eAAMPConfig_MonitorMp4Integrity.
AampConfig.cpp Registers monitorMp4Integrity and locks log level for `log=warn
Comments suppressed due to low confidence (1)

test/aampcli/Aampcli.cpp:689

  • Same issue here: commented-out logging should be removed or controlled by a runtime verbosity/log-level toggle (e.g., trace) instead of being left in the codebase as dead code.
				{
					std::string manifest = mAampcli.mSingleton->GetManifest();
					//AAMPCLI_PRINTF("[AAMPCLI] Dash Manifest length [%zu]\n", manifest.length());
					break;

Comment thread test/aampcli/Aampcli.cpp
Comment thread priv_aamp.cpp
Comment thread priv_aamp.cpp
Comment thread priv_aamp.cpp Outdated
Comment thread downloader/AampCurlDownloader.cpp Outdated
Comment thread AampConfig.cpp
Comment on lines +1844 to +1852
else if(logString.compare("warn") == 0 || logString.compare("mil") == 0 || logString.compare("error") == 0)
{
// Explicit quiet-level config: lock level so tune-start INFO elevation is suppressed
AAMP_LogLevel lvl = (logString.compare("error") == 0) ? eLOGLEVEL_ERROR
: (logString.compare("mil") == 0) ? eLOGLEVEL_MIL
: eLOGLEVEL_WARN;
AampLogManager::setLogLevel(lvl);
AampLogManager::lockLogLevel(true);
}
Comment thread test/aampcli/Aampcli.cpp Outdated
trace/debug/info already locked the log level so that the tune-time
INFO elevation could not override an explicit verbosity setting.
warn/mil/error had no equivalent — log=warn in aamp.cfg set the string
config but ConfigureLogSettings never acted on it, so setLogLevel(INFO)
at tune start always overrode it.

Add an else-if branch that matches warn/mil/error strings, sets the
corresponding level, and calls lockLogLevel(true) so that tune-time
setLogLevel(INFO) calls are blocked.  This allows log=warn in aamp.cfg
to produce WARN/MIL/ERROR-only output regardless of player state,
which is useful when running in suppressDecode mode where the tune
never formally completes.
Three sources of log noise suppressed:

1. Aampcli.cpp - AAMP_EVENT_MANIFEST_REFRESH_NOTIFY handler printed the
   full manifest refresh details and Dash Manifest length on every MPD
   update cycle (every ~2s on a live stream).  There is already a
   structured event for this; the printf spew adds no diagnostic value
   and is removed.

2. AampCurlDownloader.cpp - AAMPLOG_WARN for personaActive printed on
   every single HTTP request.  This was introduced alongside network
   persona support and was clearly debug instrumentation that was never
   intended to ship at WARN level.  Commented out.

3. priv_aamp.cpp (GetFile) - matching AAMPLOG_WARN for download url /
   personaLoaded, same root cause as above.  Commented out.

The AampConfig.cpp log=warn locking change (already committed) addresses
the fourth item in the JIRA: tune-time INFO level elevation now
respects an explicit log=warn setting in aamp.cfg so that
suppressDecode test runs do not spew info-level logs indefinitely.
- AampCurlDownloader::Download personaActive log demoted from WARN to DEBUG
- MANIFEST_REFRESH_NOTIFY, Dash Manifest length, and TUNE_TIME_METRICS
  aampcli logs uncommented; suppressed when mQuiet is true
- Aampcli::mQuiet flag set/cleared by existing quiet/noisy commands
pstroffolino and others added 3 commits May 23, 2026 17:18
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@pstroffolino pstroffolino deleted the feature/VPAAMP-406 branch May 28, 2026 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants