Repository navigation
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved pause/resume races can lose resume requests or override the latest pause intent, while queued PLAYING messages can prematurely clear seek-preroll protection and re-enable stale position queries and blocking waits.
3 open issues
What changed in this PR
This PR addresses playback remaining blocked after seeking into a parental-controlled program and entering the PIN. It coordinates resume handling with first-frame delivery and suppresses potentially stale position queries during seek preroll.
Changes:
- Cancels pending first-frame pauses on resume while deferring the PLAYING state transition.
- Tracks seek preroll in middleware, skips blocking resume waits during preroll, and temporarily suppresses position queries.
- Adds focused unit tests and updates test doubles for the cancellation helper.
| File | Description |
|---|---|
test/utests/tests/PrivAampTests/PrivAampTests.cpp |
Tests cancellation of a pending first-frame pause. |
test/utests/fakes/FakePrivateInstanceAAMP.cpp |
Adds the cancellation helper stub. |
test/utests/drm/mocks/aampMocks.cpp |
Adds the cancellation helper to DRM test doubles. |
priv_aamp.h |
Declares and documents the cancellation helper. |
priv_aamp.cpp |
Implements cancellation and synchronizes first-frame flag capture. |
middleware/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp |
Tests preroll tracking, position suppression, timeout, and resume behavior. |
middleware/InterfacePlayerRDK.cpp |
Tracks preroll and adjusts position queries and resume waits. |
middleware/InterfacePlayerPriv.h |
Adds atomic preroll state and timestamp fields. |
main_aamp.cpp |
Integrates cancellation into resume and defers state notification. |
Review effort: Balanced
Vinish100
force-pushed
the
feature/2.11.1_8.4_vipa_VPAAMP-1521
branch
2 times, most recently
from
October 30, 2026 05:46
77694f5 to
2682520
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
An unqualified Ne matcher prevents unit-test compilation, while pause/resume changes can leave sidecar captions running during a pause or the stream marked paused after playback resumes.
3 open issues
3 resolved since last review
Review effort: Balanced
…ogram in the TSB and entering the PIN, the block screen stays and video never shows Reason for change: Multi-level fix 1. In the SetRate resume path, if the seek-while-paused first frame is pending or Pause(false) did not reach PLAYING, call NotifySpeedChanged with changeState=false and clear mPauseOnFirstVideoFrameDisp. NotifyFirstVideoFrameDisplayed then moves SEEKING → PLAYING at the right time. 2. Make InterfacePlayerRDK::Pause(false) return true when when pipeline is in preroll and does not reach the target. 3. Add a seekPrerollPending flag in MW to track the pipeline state. The flag is used to avoid querying stale position value from GStreamer during a in-flight preroll. Test Procedure: As mentioned in the ticket Risks: Medium Signed-off-by: Vinish100 <vinish.balan@gmail.com>
Vinish100
force-pushed
the
feature/2.11.1_8.4_vipa_VPAAMP-1521
branch
from
October 30, 2026 11:43
2682520 to
5b0d645
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The pause-after-seek sequence can block the first-frame callback and leave playback stuck in SEEKING, while a separate race can clear a newer seek’s pending flag and expose stale positions.
2 open issues
3 resolved since last review
Review effort: Balanced
| { | ||
| // A resume may have cancelled the pause on first frame of a seek while paused; keep the latest pause intent. | ||
| // The state stays SEEKING in that case and NotifyFirstVideoFrameDisplayed() will move it to PAUSED. | ||
| seekWhilePausedFramePending = aamp->RearmPauseOnFirstVideoFrame(); |
| return rc; | ||
| } | ||
| MW_LOG_WARN("Seek preroll not completed within %d ms, resuming position queries", GST_SEEK_PREROLL_TIMEOUT_MS); | ||
| interfacePlayerPriv->gstPrivateContext->seekPrerollPending = false; |
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.
in the TSB and entering the PIN, the block screen stays and video never shows
Reason for change: Multi-level fix
Test Procedure: As mentioned in the ticket
Risks: Medium
AI identified residual risks