Handling pause-pause wedge race conditions

[rekhap2kandhavelan]

No description provided.

[Copilot]

Pull request overview

This PR adjusts GStreamer state-transition handling to reduce pause/resume race-condition wedges (notably pause-during-pause / resume-during-pause) by adding additional “wait for settle” logic during async transitions.

Changes:

• In Flush(), extend the “wait to settle” path for certain async state-change conditions (keepPausedSeek or current == pending while ASYNC).
• In Pause(), add retry/settle logic when an async state transition does not validate within the timeout (including a one-time re-issue of the state).

---

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

[Copilot]

Copilot wasn't able to review any files in this pull request.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (2)

InterfacePlayerRDK.cpp:1700

• The return value from the settle wait (settleRet) is only logged and not propagated. If the settle wait fails, ret remains whatever it was before and the later ret == GST_STATE_CHANGE_FAILURE check may miss a real failure from the second gst_element_get_state call.

		GstStateChangeReturn settleRet = gst_element_get_state(
			interfacePlayerPriv->gstPrivateContext->pipeline,
			&current, &pending, 500 * GST_MSECOND);
		MW_LOG_WARN("InterfacePlayerRDK: Flush keepPausedSeek settle wait "
			"complete: state=%s pending=%s ret=%d",

InterfacePlayerRDK.cpp:1693

• This comment says gst_element_get_state SUCCESS means ..., but this block is entered based on current == GST_STATE_PAUSED (and does not check the prior ret for SUCCESS). Either tighten the condition to match the comment or adjust the comment text to match the actual behavior.

		/* gst_element_get_state SUCCESS means the pipeline bin reached PAUSED,
		* but downstream elements (decoder, sinks) may still be completing
		* async preroll. Issuing gst_element_seek(FLUSH) during active preroll
		* causes PAUSED->PAUSED bus message and seek returning false on this
		* platform (RialtoSink/playbin3). Wait up to 500ms for full settle. */

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (3)

InterfacePlayerRDK.cpp:1709

• settleRet is logged/checked, but the function continues to use the older ret value afterwards (including the later ret == GST_STATE_CHANGE_FAILURE check and the seek retry condition). If the settle wait succeeds (or fails), ret should be updated so subsequent logic reflects the actual pipeline state, and GST_STATE_CHANGE_FAILURE should be handled immediately.

		GstStateChangeReturn settleRet = gst_element_get_state(
			interfacePlayerPriv->gstPrivateContext->pipeline,
			&current, &pending, 500 * GST_MSECOND);
		MW_LOG_WARN("InterfacePlayerRDK: Flush keepPausedSeek settle wait "
			"complete: state=%s pending=%s ret=%d",
			gst_element_state_get_name(current),
			gst_element_state_get_name(pending), settleRet);

		if (settleRet == GST_STATE_CHANGE_ASYNC)
		{
			MW_LOG_WARN("InterfacePlayerRDK: Flush settle timed out — deferring seek until ASYNC_DONE");
			SetPendingSeek(true);
			SetSeekPosition(position);
			return true;
		}
	}

InterfacePlayerRDK.cpp:1693

• The comment says gst_element_get_state SUCCESS means ..., but this block only runs when ret == GST_STATE_CHANGE_ASYNC. This is confusing/misleading and makes it hard to reason about why the extra settle wait is needed on this platform.

This issue also appears on line 1694 of the same file.

		/* gst_element_get_state SUCCESS means the pipeline bin reached PAUSED,
		* but downstream elements (decoder, sinks) may still be completing
		* async preroll. Issuing gst_element_seek(FLUSH) during active preroll
		* causes PAUSED->PAUSED bus message and seek returning false on this
		* platform (RialtoSink/playbin3). Wait up to 500ms for full settle. */

InterfacePlayerRDK.cpp:1804

• The seek retry path waits 300ms and then retries unconditionally, but it doesn't check whether the pipeline is still in GST_STATE_CHANGE_ASYNC after the wait. If preroll is still in flight, the retry can fail the same way; consider using the return value of gst_element_get_state to either defer instead of retrying, or only retry once the state change has completed.

		gst_element_get_state(
			interfacePlayerPriv->gstPrivateContext->pipeline,
			&current, &pending, 300 * GST_MSECOND);
		
		seekOk = gst_element_seek(	interfacePlayerPriv->gstPrivateContext->pipeline,
					playRate, GST_FORMAT_TIME,
					GST_SEEK_FLAG_FLUSH,
					GST_SEEK_TYPE_SET,
					position * GST_SECOND,
					GST_SEEK_TYPE_NONE, GST_CLOCK_TIME_NONE);