Under some circumstances, GStreamer deadlocks completely during MSE seeks on OpenGL ES systems. This is because the video sink still holds samples. Proper DRAIN support would fix this issue.
Created attachment 290497 [details] Patch
Comment on attachment 290497 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290497&action=review These things can be fixed at landing time. No need to upload the patch again. > Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:374 > + WebKitVideoSinkPrivate* priv = sink->priv; > + priv->scheduler.drain(); Since you are using priv just once, you can collapse these two lines. > Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:432 > + 0, // Class offset > + 0, // Accumulator > + 0, // Accumulator data > + g_cclosure_marshal_generic, > + G_TYPE_NONE, // Return type > + 0); // No parameters Add period at the end of the comments.
We don't need to wait for other MSE patches to land this one, do we?
Comment on attachment 290497 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290497&action=review > Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:376 > + LOG_MEDIA_MESSAGE("Flush-start, emitting DRAIN signal and releasing EGL samples"); This shouldn't build -- LOG_MEDIA_MESSAGE stuff was removed.
(In reply to comment #4) > > Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:376 > > + LOG_MEDIA_MESSAGE("Flush-start, emitting DRAIN signal and releasing EGL samples"); > > This shouldn't build -- LOG_MEDIA_MESSAGE stuff was removed. Yes, I missed that, sorry. Still, this can be fixed at landing time.
Comment on attachment 290497 [details] Patch Adding the signal without any handler connected from the player makes little sense
Created attachment 290713 [details] Patch
Addressed all the suggestions, added VideoRenderRequestScheduler::drain() to release the internal m_sample and subscribed to the drain signal from the base player to release the other m_sample held by the player. Now it really does a proper drain, particularly on flushes. The changes are relevant enough for a review.
Comment on attachment 290713 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290713&action=review > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:615 > + m_videoSize.setWidth(0); > + m_videoSize.setHeight(0); Just assign a new FloatSize() to m_videoSize, essentially resetting it to (0, 0). > Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:389 > + break; Unnecessary.
Comment on attachment 290713 [details] Patch Address the comments please.
Created attachment 291755 [details] Patch
Created attachment 292887 [details] Patch
Comment on attachment 292887 [details] Patch Clearing flags on attachment: 292887 Committed r207874: <http://trac.webkit.org/changeset/207874>
All reviewed patches have been landed. Closing bug.