Skip to content

VPAAMP-410 linear cdai race condition that can cause a required init segment to not be injected#1500

Open
pstroffolino wants to merge 4 commits into
dev_sprint_25_2from
feature/VPAAMP-410
Open

VPAAMP-410 linear cdai race condition that can cause a required init segment to not be injected#1500
pstroffolino wants to merge 4 commits into
dev_sprint_25_2from
feature/VPAAMP-410

Conversation

@pstroffolino

Copy link
Copy Markdown
Contributor

Reason for Change: Add INFO-level diagnostic logs at four key points in the CDAI
clear-to-encrypted ad-to-content transition path:

  1. FetchAndInjectInitialization gate: log profileChanged and
    discontinuity per track before the init fetch condition is
    evaluated, to detect silent skip of init injection.

  2. FetchAndInjectInitialization fetch: log the resolved init URL
    and discontinuity flag when the gate passes, to detect use of
    a wrong (clear/ad) init segment URL.

  3. onAdEvent OUTSIDE_ADBREAK: log mCurrentPeriod, endPeriodId,
    endPeriodOffset and mCurrentPeriodIdx at the point mBasePeriodId
    is updated, to detect period index mismatch after ad completion.

  4. SelectSourceOrAdPeriod exit: log periodChanged, adStateChanged
    and requireStreamSelection before returning, to confirm
    StreamSelection will run for the content period.

  5. MediaStreamContext init cache: log HIT/MISS and URL for every
    init segment fetch, to detect stale cache entries returning
    a clear init segment for an encrypted period URL.

All new log lines use [CDAI-DBG] prefix for easy filtering.

When the soak test captures the failure, grep the INFO logs for [CDAI-DBG] around the timestamp of the crash. The sequence to look for in the bad case will be SelectSourceOrAdPeriod exit showing requireStreamSelection=0, followed immediately by FetchAndInjectInitialization showing profileChanged=0 discontinuity=0 for the video track — confirming the init fetch was silently skipped before the encrypted segments began flowing.

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 primarily adds INFO-level diagnostic logging to help investigate a linear CDAI (clear ad → encrypted content) transition issue where init segments may not be injected. It also introduces additional changes around MP4 integrity monitoring, log-level locking behavior, and macOS dependency installation.

Changes:

  • Add [CDAI-DBG] INFO logs at key CDAI transition points (init injection gate/fetch, ad state transitions, stream-selection decisions) and init-fragment cache HIT/MISS.
  • Add an optional MP4 integrity monitor that parses downloaded audio/video segments with persistent Mp4Demux validators and dumps corrupt segments.
  • Improve macOS install scripts to detect/install GStreamer pkg-config paths; comment out some noisy CLI/downloader logs.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
fragmentcollector_mpd.cpp Adds [CDAI-DBG] logs for init injection gating/fetching, adbreak completion, and period/stream-selection decisions.
MediaStreamContext.cpp Adds [CDAI-DBG] init-fragment cache HIT/MISS logging; short-circuits injection when eAAMPConfig_SuppressDecode is set.
priv_aamp.h Adds CheckSegmentIntegrity() declaration and persistent Mp4Demux validator members; includes MP4Demux.h.
priv_aamp.cpp Implements MP4 segment integrity checking and hooks it into GetFile() under config control; comments out a WARN log.
AampConfig.h Adds eAAMPConfig_MonitorMp4Integrity boolean config.
AampConfig.cpp Registers monitorMp4Integrity config key; changes ConfigureLogSettings() to lock log level for `"warn"
scripts/install_dependencies.sh Adds macOS GStreamer framework/homebrew detection and installs via brew if missing.
scripts/install_aampcli.sh Adds macOS GStreamer framework/homebrew pkg-config path discovery and clearer error on missing GStreamer.
downloader/AampCurlDownloader.cpp Comments out a WARN log line related to network persona.
test/aampcli/Aampcli.cpp Comments out manifest-refresh related CLI prints.
Comments suppressed due to low confidence (1)

test/aampcli/Aampcli.cpp:689

  • Same issue here: leaving a commented-out AAMPCLI_PRINTF makes the intent unclear and risks bitrot. Prefer removing it entirely or controlling it via an explicit verbosity/log-level setting.
				default:
				{
					std::string manifest = mAampcli.mSingleton->GetManifest();
					//AAMPCLI_PRINTF("[AAMPCLI] Dash Manifest length [%zu]\n", manifest.length());
					break;

Comment thread fragmentcollector_mpd.cpp
Comment on lines +12427 to +12431
AAMPLOG_INFO("[CDAI-DBG] onAdEvent OUTSIDE_ADBREAK: mCurrentPeriod=%s endPeriodId=%s endPeriodOffset=%u mCurrentPeriodIdx=%d",
mCurrentPeriod ? mCurrentPeriod->GetId().c_str() : "null",
mCdaiObject->mAdBreaks[mCdaiObject->mCurPlayingBreakId].endPeriodId.c_str(),
mCdaiObject->mAdBreaks[mCdaiObject->mCurPlayingBreakId].endPeriodOffset,
mCurrentPeriodIdx);
Comment thread priv_aamp.h
Comment on lines 66 to 71
#include "AampDefine.h"
#include "AampCurlDefine.h"
#include "AampLLDASHData.h"
#include "mp4demux/MP4Demux.h"
#include "AampMPDPeriodInfo.h"
#include "TsbApi.h"
Comment thread priv_aamp.cpp
Comment on lines +5466 to +5473
if (ISCONFIGSET_PRIV(eAAMPConfig_MonitorMp4Integrity) &&
ISCONFIGSET_PRIV(eAAMPConfig_UseMp4Demux) &&
!buffer.empty() &&
(mediaType == eMEDIATYPE_VIDEO || mediaType == eMEDIATYPE_INIT_VIDEO ||
mediaType == eMEDIATYPE_AUDIO || mediaType == eMEDIATYPE_INIT_AUDIO))
{
CheckSegmentIntegrity(buffer, mediaType, remoteUrl);
}
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
Comment on lines 670 to 674
case AAMP_EVENT_MANIFEST_REFRESH_NOTIFY:
{
ManifestRefreshEventPtr ev = std::dynamic_pointer_cast<ManifestRefreshEvent>(e);
AAMPCLI_PRINTF("[AAMPCLI] AAMP_EVENT_MANIFEST_REFRESH_NOTIFY received Dur[%u]:NoPeriods[%u]:PubTime[%u] manifestType[%s]\n",ev->getManifestDuration(),ev->getNoOfPeriods(),ev->getManifestPublishedTime(),ev->getManifestType().c_str());
//AAMPCLI_PRINTF("[AAMPCLI] AAMP_EVENT_MANIFEST_REFRESH_NOTIFY received Dur[%u]:NoPeriods[%u]:PubTime[%u] manifestType[%s]\n",ev->getManifestDuration(),ev->getNoOfPeriods(),ev->getManifestPublishedTime(),ev->getManifestType().c_str());
AAMPPlayerState state = mAampcli.mSingleton->GetState();
Comment thread priv_aamp.cpp
AampNetworkPersona::Instance().LoadFromFile(personaPath);
}
AAMPLOG_WARN("priv_aamp: download url=%s personaLoaded=%d", remoteUrl.c_str(), AampNetworkPersona::Instance().IsLoaded() ? 1 : 0);
// AAMPLOG_WARN("priv_aamp: download url=%s personaLoaded=%d", remoteUrl.c_str(), AampNetworkPersona::Instance().IsLoaded() ? 1 : 0);
// acquire-load (~1 ns) with no mutex, no clock call, nothing else.
const bool personaActive = AampNetworkPersona::Instance().IsLoaded();
AAMPLOG_WARN("AampCurlDownloader::Download personaActive=%d url=%s", personaActive ? 1 : 0, urlStr.c_str());
// AAMPLOG_WARN("AampCurlDownloader::Download personaActive=%d url=%s", personaActive ? 1 : 0, urlStr.c_str());
Introduces eAAMPConfig_MonitorMp4Integrity (bool, default false).
When enabled alongside useMp4Demux=true, every downloaded video/audio
segment (including init segments) is parsed by a persistent per-track
Mp4Demux instance.  The per-track validator preserves init-segment
state so encrypted streams do not produce false-positive
MP4_PARSE_ERROR_INVALID_IV_SIZE errors on media segments.

Each processed segment is logged at INFO level.  On parse failure the
segment is logged at WARN level with the Mp4ParseError code and written
to harvestPath (or /tmp as fallback) using the server-side filename.

Files changed:
  AampConfig.h    - add eAAMPConfig_MonitorMp4Integrity enum value
  AampConfig.cpp  - add bool lookup table entry for monitorMp4Integrity
  priv_aamp.h     - add CheckSegmentIntegrity() declaration and
                    mVideoIntegrityValidator / mAudioIntegrityValidator
                    unique_ptr<Mp4Demux> members
  priv_aamp.cpp   - implement CheckSegmentIntegrity(); hook into GetFile()
                    after harvest block for VIDEO/AUDIO/INIT media types
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.
Add INFO-level diagnostic logs at four key points in the CDAI
clear-to-encrypted ad-to-content transition path:

1. FetchAndInjectInitialization gate: log profileChanged and
   discontinuity per track before the init fetch condition is
   evaluated, to detect silent skip of init injection.

2. FetchAndInjectInitialization fetch: log the resolved init URL
   and discontinuity flag when the gate passes, to detect use of
   a wrong (clear/ad) init segment URL.

3. onAdEvent OUTSIDE_ADBREAK: log mCurrentPeriod, endPeriodId,
   endPeriodOffset and mCurrentPeriodIdx at the point mBasePeriodId
   is updated, to detect period index mismatch after ad completion.

4. SelectSourceOrAdPeriod exit: log periodChanged, adStateChanged
   and requireStreamSelection before returning, to confirm
   StreamSelection will run for the content period.

5. MediaStreamContext init cache: log HIT/MISS and URL for every
   init segment fetch, to detect stale cache entries returning
   a clear init segment for an encrypted period URL.

All new log lines use [CDAI-DBG] prefix for easy filtering.

VPAAMP-410
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.

2 participants