VPAAMP-406 suppress noisy logging#1498
Closed
pstroffolino wants to merge 6 commits into
Closed
Conversation
Contributor
There was a problem hiding this comment.
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
aampclimanifest-refresh prints and comment out some accidental WARN-level persona download logs. - Add
monitorMp4Integrityconfig + per-track persistentMp4Demuxvalidators to detect and harvest corrupt MP4 segments. - Adjust
ConfigureLogSettings()solog=warn|mil|errorlocks 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 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); | ||
| } |
Vinish100
requested changes
May 22, 2026
64ca435 to
f7ca981
Compare
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
c2e2a96 to
94fdf7b
Compare
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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
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.
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