RDKEMW-20975 [Rack][RDKE][8.6][Llama G1/Cello]: Screen got stuck during VOD content playback

[rekhap2kandhavelan]

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
Propagate Pause() failure

Pause() detects the failure through validateStateWithMsTimeout(), but the failure was not being propagated to the middleware layer.
The fix ensures that the failure is propagated instead of being silently ignored.
Increase timeout budget

The existing retry budget of 5 × 100 ms (500 ms) was insufficient for TSB seek scenarios with concurrent Widevine DRM renegotiation.
Increased the retry count to 10 with adaptive sleep to provide sufficient time for recovery.
Maintain correct sink state

Previously, mSinkPaused was being set to false unconditionally, even when Pause() failed, resulting in a state mismatch that masked the playback freeze during subsequent SetRate() calls.
The state handling has been corrected to avoid this desynchronization.
AAMP Changes
Recover by re-tuning

Once the middleware propagates the Pause() failure, AAMP now schedules a recovery by performing a re-tune instead of continuing in a broken state.
This ensures playback recovery when the race condition is encountered.
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:
rdkcentral/aamp#1723

When Pause(false) fails, trigger recovery using TuneHelper(eTUNETYPE_SEEK), matching the existing Local-TSB recovery path.
Keep mSinkPaused = false and ResumeDownloads() unconditional as part of the recovery flow.
Result
VOD Playback: Successfully recovers without playback freeze (approximately 1-second recovery).
Linear Playback: Successfully recovers after applying the additional AAMP handling.
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.

[Copilot]

Pull request overview

This PR addresses playback freezes caused by race conditions by improving GStreamer state-transition handling in the middleware layer (and enabling higher layers to recover when Pause() fails).

Changes:

• Propagate Pause() failures back to the caller by returning false when state validation / state-set fails.
• Increase the pipeline state validation retry budget by raising GST_ELEMENT_GET_STATE_RETRY_CNT_MAX from 5 to 10.
• Extend InterfacePlayerRDK::Flush(...) with a new keepPausedSeek flag and add an extra “wait to settle” gst_element_get_state() when the pipeline is in an async transition.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
InterfacePlayerRDK.cpp	Updates Flush() signature/behavior and ensures Pause() returns failure on state-transition errors.
InterfacePlayerPriv.h	Increases the maximum retry count used by validateStateWithMsTimeout().

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

InterfacePlayerRDK.cpp:3462

• Pause() now returns false on state-change failure, but it still unconditionally updates gstPrivateContext->paused to the requested value. Since this flag is used to mean “paused by user” (e.g., GetDurationMilliseconds/GetPositionMilliseconds logic), updating it on failure can leave internal state inconsistent with the actual pipeline state and can impact subsequent queries/logic.

		interfacePlayerPriv->gstPrivateContext->buffering_target_state = nextState;
		interfacePlayerPriv->gstPrivateContext->paused = pause;
		interfacePlayerPriv->gstPrivateContext->pendingPlayState = false;

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

InterfacePlayerRDK.h:721

• The Flush signature change introduces a new required parameter, which will break existing callers that still pass 4 arguments (e.g., tests/CLI). Consider providing a default value for backward compatibility; also update the Doxygen @param list which currently references non-existent parameters (GstState, gstMediaFormat) and has misaligned indentation.

        	 * @param[in] GstState The desired GStreamer pipeline state.
        	 * @param[in] gstMediaFormat The media format for the pipeline.
			 * @param[in] keepPausedSeek true only for an explicit seek-with-keepPaused request
        	 */
        	bool Flush(double position, int rate, bool shouldTearDown, bool isAppSeek,  bool keepPausedSeek);

InterfacePlayerRDK.cpp:3462

• Pause() now correctly returns false on failure, but it still unconditionally updates gstPrivateContext->paused to the requested value. This can desynchronize internal state from the actual pipeline state and affect later logic that relies on paused (e.g., position/duration queries and buffer-control decisions). Update paused only when the state transition succeeds.

		interfacePlayerPriv->gstPrivateContext->buffering_target_state = nextState;
		interfacePlayerPriv->gstPrivateContext->paused = pause;
		interfacePlayerPriv->gstPrivateContext->pendingPlayState = false;

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

InterfacePlayerRDK.h:721

• Flush gained a new keepPausedSeek parameter, which breaks existing call sites (e.g., tests and pi-cli currently call Flush(position, rate, shouldTearDown, isAppSeek)). Also the parameter docs in this block don’t match the function signature (GstState/gstMediaFormat aren’t parameters). Consider keeping this backward-compatible by adding a default value and updating the doc lines to match the actual parameters.

        	 * @param[in] GstState The desired GStreamer pipeline state.
        	 * @param[in] gstMediaFormat The media format for the pipeline.
			 * @param[in] keepPausedSeek true only for an explicit seek-with-keepPaused request
        	 */
        	bool Flush(double position, int rate, bool shouldTearDown, bool isAppSeek,  bool keepPausedSeek);