RDKEMW-19440 [develop]- [Rack][RDKE][Llama G1 UK/Cello]: Screen got stuck during VOD series & Disney content playback#168
Open
rekhap2kandhavelan wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new seekPausedState flag intended to prevent “keepPaused seek” flows from racing into unintended PLAYING transitions (e.g., via buffering timeout, clock-lost recovery, or fragment caching completion), which can otherwise lead to stuck playback during VOD/series content.
Changes:
- Adds
seekPausedStateto the player private context and updates multiple PLAYING-transition paths to honor it. - Updates
Flush(),ConfigurePipeline(),buffering_timeout(), CLOCK_LOST handling, andSetPlayBackRate()to coordinate keepPaused seek vs. resume behavior. - Extends unit tests to validate new keepPaused/seekPausedState behaviors and adjusts existing Pause test expectations.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
InterfacePlayerRDK.cpp |
Implements seekPausedState gating across buffering/PLAYING transition paths, plus Pause retry logic and SetPlayBackRate resume behavior. |
InterfacePlayerPriv.h |
Adds the new seekPausedState field to GstPlayerPriv. |
test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp |
Updates Pause unit test behavior and adds new tests for seekPausedState/keepPaused flows. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+550
to
+554
| if (SetStateWithWarnings(interfacePlayerPriv->gstPrivateContext->pipeline, GST_STATE_PLAYING) == GST_STATE_CHANGE_FAILURE) | ||
| { | ||
| MW_LOG_ERR("InterfacePlayerRDK: GST_STATE_PLAYING failed"); | ||
| } | ||
| interfacePlayerPriv->gstPrivateContext->pendingPlayState = false; |
Comment on lines
+515
to
+520
| // buffering_timeout will handle the PLAYING transition, so seekPausedState must not block it. | ||
| if (interfacePlayerPriv->gstPrivateContext->seekPausedState) | ||
| { | ||
| MW_LOG_WARN("ConfigurePipeline: clearing seekPausedState — buffering will drive PLAYING transition"); | ||
| interfacePlayerPriv->gstPrivateContext->seekPausedState = false; | ||
| } |
Comment on lines
+3491
to
+3495
| else if (GST_STATE_CHANGE_SUCCESS != rcRetry) | ||
| { | ||
| MW_LOG_ERR("Retry failed immediately with rc %d — reporting error", rcRetry); | ||
| retValue = false; | ||
| } |
Comment on lines
4804
to
+4818
| else if (frames == -1 || frames >= pInterfacePlayerRDK->m_gstConfigParam->framesToQueue || (privatePlayer->gstPrivateContext->buffering_timeout_cnt > 0 && --privatePlayer->gstPrivateContext->buffering_timeout_cnt == 0)) | ||
| { | ||
| /* Do not set PLAYING if a seek-with-keepPaused is in progress. | ||
| * The buffering_timeout timer may fire after ConfigurePipeline restarts buffering | ||
| * but BEFORE the Pause(1) from keepPaused logic arrives — causing a race. */ | ||
| if (privatePlayer->gstPrivateContext->seekPausedState) | ||
| { | ||
| MW_LOG_WARN("buffering_timeout: skipping PLAYING — seekPausedState active (cnt %u, frames %d)", privatePlayer->gstPrivateContext->buffering_timeout_cnt, frames); | ||
| if (privatePlayer->gstPrivateContext->buffering_timeout_cnt == 0) | ||
| { | ||
| MW_LOG_ERR("buffering_timeout: seekPausedState still active after timeout exhausted — clearing to unblock"); | ||
| privatePlayer->gstPrivateContext->seekPausedState = false; | ||
| } | ||
| return privatePlayer->gstPrivateContext->buffering_in_progress; | ||
| } |
Comment on lines
+4676
to
+4685
| if (rate != 0.0 && interfacePlayerPriv->gstPrivateContext->seekPausedState && interfacePlayerPriv->gstPrivateContext->paused) | ||
| { | ||
| MW_LOG_WARN("InterfacePlayerRDK: SetPlayBackRate detected resume while seekPausedState active — forcing resume"); | ||
| /* Pause(false) clears seekPausedState in Pause implementation. */ | ||
| Pause(false, false); | ||
| interfacePlayerPriv->gstPrivateContext->seekPausedState = false; | ||
| interfacePlayerPriv->gstPrivateContext->pendingPlayState = false; | ||
| /* After explicit resume we consider operation successful */ | ||
| ret = true; | ||
| } |
Comment on lines
+3335
to
+3348
| // Arrange: Simulate a paused pipeline in seek-paused state | ||
| mPlayerContext->paused = true; | ||
| mPlayerContext->seekPausedState = true; | ||
| mPlayerContext->pendingPlayState = true; | ||
| mPlayerContext->pipeline = nullptr; | ||
|
|
||
| // Act: Request a resume rate | ||
| bool result = mInterfaceGstPlayer->SetPlayBackRate(1.0); | ||
|
|
||
| // Assert: Middleware should clear seekPausedState and pendingPlayState, and return true | ||
| EXPECT_TRUE(result); | ||
| EXPECT_EQ(mPlayerContext->seekPausedState, false); | ||
| EXPECT_EQ(mPlayerContext->pendingPlayState, false); | ||
| } |
| gboolean buffering_in_progress; /**< buffering is in progress */ | ||
| guint buffering_timeout_cnt; /**< make sure buffering_timeout doesn't get stuck */ | ||
| GstState buffering_target_state; /**< the target state after buffering */ | ||
| bool seekPausedState; /** < true when seek with keepPaused is active — guards buffering_timeout from setting PLAYING */ |
Comment on lines
+3475
to
+3479
| /* Recovery: retry the state change once before reporting failure */ | ||
| MW_LOG_INFO("InterfacePlayerRDK_Pause - retrying state change to GstState %d", nextState); | ||
|
|
||
| // Wait for any in-flight transition to settle | ||
| gst_element_get_state(interfacePlayerPriv->gstPrivateContext->pipeline, ¤t, &pending, 0); |
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.
RDKEMW-19484 : video stuck issue fix
Reason for change : The core problem is that there was no mechanism to signal between Flush() and the various PLAYING-transition paths that a keepPaused seek is in progress. The seekPausedState flag fills that gap.
Test Steps : mentioned in ticket
Signed-off by : [rekha_kandhavelan@comcast.com]