Skip to content

Disable synchronous play in RialtoServer as it caused some race conditions#507

Open
Koky2701 wants to merge 1 commit into
release/v0.19.0from
feature/RDKEMW-18286
Open

Disable synchronous play in RialtoServer as it caused some race conditions#507
Koky2701 wants to merge 1 commit into
release/v0.19.0from
feature/RDKEMW-18286

Conversation

@Koky2701
Copy link
Copy Markdown
Contributor

Summary: Disable synchronous play in RialtoServer as it caused some race conditions
Type: Fix
Test Plan: UT/CT, Fullstack
Jira: RDKEMW-18286

…tions (#494)

Summary: Disable synchronous play in RialtoServer as it caused some race
conditions
Type: Fix
Test Plan: UT/CT, Fullstack
Jira: RDKEMW-18286
@github-actions
Copy link
Copy Markdown

Pull request must be merged with a description containing the required fields,

Summary:
Type: Feature/Fix/Cleanup
Test Plan:
Jira:

If there is no jira releated to this change, please put 'Jira: NO-JIRA'.

Description can be changed by editing the top comment on your pull request and making a new commit.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to eliminate “synchronous play” behavior in the Rialto server path to reduce race conditions by ensuring play requests are dispatched via the normal main-thread queue and executed asynchronously on the player worker thread.

Changes:

  • Switch MediaPipelineServerInternal::play() from enqueuePriorityTaskAndWait to enqueueTaskAndWait.
  • Make GstGenericPlayer::play() always asynchronous (enqueue play task on the worker thread and set async=true).
  • Update/remove unit tests and test helpers that relied on synchronous/priority play behavior.

Reviewed changes

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

Show a summary per file
File Description
tests/unittests/media/server/main/mediaPipeline/MiscellaneousFunctionsTest.cpp Updates play-related expectations to use non-priority main-thread enqueue behavior.
tests/unittests/media/server/main/mediaPipeline/base/MediaPipelineTestBase.h Removes the priority-enqueue test helper declaration.
tests/unittests/media/server/main/mediaPipeline/base/MediaPipelineTestBase.cpp Removes the priority-enqueue test helper implementation.
tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerTest.cpp Removes tests covering immediate synchronous/asynchronous play; keeps worker-thread play test with async=true.
media/server/main/source/MediaPipelineServerInternal.cpp Routes play via enqueueTaskAndWait instead of priority enqueue.
media/server/gstplayer/source/GstGenericPlayer.cpp Forces play to be worker-thread queued and always asynchronous; removes state-change counter usage.
media/server/gstplayer/include/GstGenericPlayer.h Removes the m_ongoingStateChangesNumber atomic member.

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

Comment on lines 341 to 345
bool result;
auto task = [&]() { result = playInternal(async); };

m_mainThread->enqueuePriorityTaskAndWait(m_mainThreadClientId, task);
m_mainThread->enqueueTaskAndWait(m_mainThreadClientId, task);
return result;
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants