[GStreamer][MSE] Take playbin's states lock when sending seek event #29897
+13
−2
Conversation
This file contains 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
|
EWS run on previous version of this PR (hash 29c0e21) |
ntrrgc
approved these changes
Jun 18, 2024
| gst_element_seek(m_source.get(), rate, GST_FORMAT_TIME, m_seekFlags, | ||
| GST_SEEK_TYPE_SET, toGstClockTime(target.time), GST_SEEK_TYPE_NONE, 0); | ||
| { | ||
| auto locker = GstStateLocker(pipeline()); |
There was a problem hiding this comment.
Needs an explanation. For instance:
// Take the STATE_LOCK of the __pipeline__.
//
// gst_element_send_event() [which is called by gst_element_seek()] already takes the
// STATE_LOCK of the element in order to delay any state change attempts from other
// threads while the event is travelling the pipeline.
//
// Normally that would happen to both the pipeline and then recursively to the
// elements inside as they handle the seek event, but since we're sending the event
// directly to the source element we need to take the STATE_LOCK on the pipeline ourselves.|
EWS run on current version of this PR (hash a5b17e9) |
https://bugs.webkit.org/show_bug.cgi?id=275566 Reviewed by Alicia Boya Garcia. This fixes possible race between application (triggering another seek from 'seeked' event) and 'state chagne' continuation (triggered by playbin). Top level bin element does change the state as result of 'async-done' handling from previous seek request (see gst_bin_continue_func in gstbin.c). Which may race with handling of 'async-start' posted by sinks on flushing seek. And may leave the pipeline in inconsistent state and 'hanging' seek that never finishes. By taking the states lock of top level bin element player will wait for possible state change continuation to complete before sending next seek event. Based on a patch by Eugene Mutavchi <[email protected]>. * Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp: (WebCore::MediaPlayerPrivateGStreamerMSE::doSeek): Canonical link: https://commits.webkit.org/280168@main
a5b17e9
to
81376ca
Compare
|
Committed 280168@main (81376ca): https://commits.webkit.org/280168@main Reviewed commits have been landed. Closing PR #29897 and removing active labels. |
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.
81376ca
a5b17e9
🧪 api-ios