VPAAMP-473:Place CDAI ads for cold CDVR and iVOD#1592
Open
varshnie wants to merge 1 commit into
Open
Conversation
d905e95 to
1395c6f
Compare
Vinish100
reviewed
Jun 11, 2026
| lock.unlock(); | ||
| AAMPLOG_INFO("[CDAI] Cold CDVR/IVOD: calling PlaceAds with base MPD helper %p", baseMPDHelper.get()); | ||
| PlaceAds(baseMPDHelper); | ||
| lock.lock(); |
Contributor
There was a problem hiding this comment.
Any reason to remove lock?
We can make mDaiMtx recursive mutex if there is lock up issues by calling lock multiple times
f67159c to
9f2da17
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses CDAI ad placement for cold CDVR / iVOD (static MPDs) where manifests do not refresh, by enabling ad placement to be triggered immediately after ad fulfillment rather than waiting for a manifest update.
Changes:
- Update MPD stream logic to run CDAI placement for static manifests and keep the CDAI object in sync with the latest base MPD parse helper.
- Extend
PrivateCDAIObjectMPDto store a “base” MPD parse helper and use it to trigger immediatePlaceAds()after an ad is fulfilled. - Add additional CDAI logging and introduce conditional “cold CDAI” event handling in
ProcessEventStream().
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| fragmentcollector_mpd.cpp | Triggers CDAI placement for static manifests and adds cold-CDVR/iVOD event handling/logging. |
| admanager_mpd.h | Adds storage + mutex for a base-stream MPD parse helper and exposes SetBaseMPDParseHelper(). |
| admanager_mpd.cpp | Implements base-helper storage and immediate PlaceAds() after fulfillment; adds placement logging. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1336
to
+1341
| // Release mDaiMtx while calling PlaceAds so that the | ||
| // fetcher thread can also run onAdEvent concurrently. | ||
| lock.unlock(); | ||
| PlaceAds(baseMPDHelper); | ||
| lock.lock(); | ||
| } |
e3053cf to
7708dcc
Compare
Reason for change:Cold CDVR/static manifests never refresh, so ad placement was never triggered after fulfillment, causing playback to freeze waiting for ads that were never mapped.Fixed by immediately calling ad placement as soon as an ad is resolved, instead of waiting for a manifest refresh that never comes. Risks: p1 Signed-off-by: varshnie <varshniblue14@gmail.com>
7708dcc to
dbc171b
Compare
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:Cold CDVR/static manifests never refresh, so ad placement was never triggered after fulfillment, causing playback to freeze waiting for ads that were never mapped.Fixed by immediately calling ad placement as soon as an ad is resolved, instead of waiting for a manifest refresh that never comes.
Risks: p1
Signed-off-by: varshnie varshniblue14@gmail.com