RDKEMW-20975 [Rack][RDKE][8.6][Llama G1/Cello]: Screen got stuck during VOD content playback#1723
Open
rekhap2kandhavelan wants to merge 7 commits into
Open
RDKEMW-20975 [Rack][RDKE][8.6][Llama G1/Cello]: Screen got stuck during VOD content playback#1723rekhap2kandhavelan wants to merge 7 commits into
rekhap2kandhavelan wants to merge 7 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent playback freezes caused by a race during resume/unpause by (a) adding an explicit “seek-with-keepPaused” signal through the Flush path and (b) adding an AAMP-side recovery path that re-seeks (re-tunes) when an unpause fails.
Changes:
- Extend
StreamSink::Flush/AAMPGstPlayer::Flush/InterfacePlayerRDK::Flushwith akeepPausedSeekboolean to distinguish explicit keep-paused seeks from incidental paused states. - Propagate
seekWhilePausedfromPrivateInstanceAAMP::TuneHelperintoFlush(..., keepPausedSeek)calls. - Add SetRate resume recovery: if
sink->Pause(false,false)fails, switch to SEEKING and recover viaTuneHelper(eTUNETYPE_SEEK).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
StreamSink.h |
Adds keepPausedSeek parameter to Flush API and documents intended semantics. |
priv_aamp.cpp |
Passes seekWhilePaused through to StreamSink::Flush(..., keepPausedSeek). |
middleware/InterfacePlayerRDK.h |
Extends middleware Flush signature to accept keepPausedSeek. |
middleware/InterfacePlayerRDK.cpp |
Updates middleware Flush signature to accept keepPausedSeek. |
main_aamp.cpp |
Adds resume failure recovery via re-seek/re-tune in SetRateInternal. |
AampStreamSinkManager.cpp |
Updates mGstPlayer->Flush call to include the new argument. |
aampgstplayer.h |
Updates AAMPGstPlayer::Flush override signature with keepPausedSeek. |
aampgstplayer.cpp |
Propagates keepPausedSeek down to InterfacePlayerRDK::Flush; updates internal call sites. |
| * @brief Flush cached GstBuffers and set seek position & rate | ||
| */ | ||
| bool InterfacePlayerRDK::Flush(double position, int rate, bool shouldTearDown, bool isAppSeek) | ||
| bool InterfacePlayerRDK::Flush(double position, int rate, bool shouldTearDown, bool isAppSeek, bool keepPausedSeek) |
Comment on lines
717
to
+720
| * @param[in] GstState The desired GStreamer pipeline state. | ||
| * @param[in] gstMediaFormat The media format for the pipeline. | ||
| */ | ||
| bool Flush(double position, int rate, bool shouldTearDown, bool isAppSeek); | ||
| bool Flush(double position, int rate, bool shouldTearDown, bool isAppSeek, bool keepPausedSeek); |
Comment on lines
167
to
+172
| * @param[in] position playback seek position | ||
| * @param[in] rate playback rate | ||
| * @param[in] shouldTearDown flag indicates if pipeline should be destroyed if in invalid state | ||
| * @param[in] keepPausedSeek true only for an explicit seek-with-keepPaused request | ||
| */ | ||
| void Flush(double position, int rate, bool shouldTearDown) override; | ||
| void Flush(double position, int rate, bool shouldTearDown, bool keepPausedSeek) override; |
Comment on lines
+139
to
+143
| * fragment caching), since that conflation can permanently block later | ||
| * PLAYING transitions when no explicit resume ever arrives. | ||
| * @return void | ||
| */ | ||
| virtual void Flush(double position = 0, int rate = AAMP_NORMAL_PLAY_RATE, bool shouldTearDown = true){} | ||
| virtual void Flush(double position = 0, int rate = AAMP_NORMAL_PLAY_RATE, bool shouldTearDown = true, bool keepPausedSeek = false){} |
Comment on lines
+931
to
+934
| { | ||
| std::lock_guard<std::recursive_mutex> lock(aamp->GetStreamLock()); | ||
| aamp->TuneHelper(eTUNETYPE_SEEK, false); | ||
| } |
Comment on lines
498
to
501
| mGstPlayer->ChangeAamp(aamp, mInactiveGstPlayersMap[aamp]->GetID3MetadataHandler()); | ||
| aamp->mIsFlushOperationInProgress = true; | ||
| mGstPlayer->Flush(position, aamp->rate, true); | ||
| mGstPlayer->Flush(position, aamp->rate, true,false); | ||
| aamp->mIsFlushOperationInProgress = 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.
Race Condition Recovery Handling Summary
To address the playback freeze caused by race conditions, the recovery handling has been implemented across both the Middleware and AAMP layers.
Middleware Changes
rdkcentral/middleware-player-interface#200
Propagate Pause() failure
Pause()detects the failure throughvalidateStateWithMsTimeout(), but the failure was not being propagated to the middleware layer.Increase timeout budget
Maintain correct sink state
mSinkPausedwas being set tofalseunconditionally, even whenPause()failed, resulting in a state mismatch that masked the playback freeze during subsequentSetRate()calls.AAMP Changes
Recover by re-tuning
Pause()failure, AAMP now schedules a recovery by performing a re-tune instead of continuing in a broken state.Additional Linear Playback Handling
After implementing the above changes, VOD playback recovered successfully with approximately 1 second recovery time. However, linear playback still exhibited issues.
To address this, the AAMP handling was updated in the pipeline-unpause branch:
Pause(false)fails, trigger recovery usingTuneHelper(eTUNETYPE_SEEK), matching the existing Local-TSB recovery path.mSinkPaused = falseandResumeDownloads()unconditional as part of the recovery flow.Result
Note: The recovery logic is implemented collaboratively across both the Middleware and AAMP layers. When the race condition is detected and
Pause()fails, the failure is propagated and appropriate recovery actions are triggered, allowing both VOD and Linear playback to recover automatically.