[GStreamer][MSE] Crash in webKitMediaSrcStreamFlush

https://bugs.webkit.org/show_bug.cgi?id=260455 Reviewed by Alicia Boya Garcia. Fix crash and playback of video on https://www.apple.com/apple-watch-ultra-2/. The video starts outside of the viewport and is played via MSE. The issue was caused due to failing to transition the pipeline to PAUSED in MediaPlayerPrivateGStreamer::setVisibleInViewport when an MSE video starts initially outside of the viewport. That, in turn, is caused because we try to set playbin to PAUSED without an URL set. Then, the media player pipeline gets stuck in READY, while webkitmediasrc is in the PAUSED state. With this, the video never starts playing back. Moreover, scrolling the page for the video to be in and out of the viewport, it triggers a downgrade state in webkitmediasrc to READY, which if coupled with a seek attempt then causes a crash in WebKitMediaSourceGStreamer when flushing. The fix is two-fold: 1. Avoid setting the state to PAUSED if no playbin URL has been set. 2. Avoid setting the state of the pipeline to READY in MediaPlayerPrivateGStreamer::setVisibleInViewport if the media player is MSE. Added a test that triggers the described behavior and crashes without the fix. It scrolls the page to move a video in and out of the viewport, while also seeking to the start on every scroll. Currently, part of the code to trigger the crash isn't executed due to WEBKIT_GST_ALLOW_PLAYBACK_OF_INVISIBLE_VIDEOS=1 being set in the test driver in Tools/Scripts/webkitpy/port/glib.py (264017@main), which needs to be commented manually. We should find a way to add a test preference for this code path to be enabled. * LayoutTests/media/media-source/media-source-muted-scroll-and-seek-crash-expected.txt: Added. * LayoutTests/media/media-source/media-source-muted-scroll-and-seek-crash.html: Added. * Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp: (WebCore::MediaPlayerPrivateGStreamer::load): Set playbin URL before calling setVisibleInViewport. (WebCore::MediaPlayerPrivateGStreamer::setVisibleInViewport): Avoid state transitions to READY if MSE and avoid setting the pipeline to PAUSED if playbin URL is not set. Canonical link: https://commits.webkit.org/276798@main
WebKit · Mar 28, 2024 · f91aeb9 · f91aeb9
1 parent 85696a6
commit f91aeb9
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 3 deletions.
diff --git a/LayoutTests/media/media-source/media-source-muted-scroll-and-seek-crash-expected.txt b/LayoutTests/media/media-source/media-source-muted-scroll-and-seek-crash-expected.txt
@@ -0,0 +1,4 @@
+
+No crash OK
+END OF TEST
+
diff --git a/LayoutTests/media/media-source/media-source-muted-scroll-and-seek-crash.html b/LayoutTests/media/media-source/media-source-muted-scroll-and-seek-crash.html
@@ -0,0 +1,65 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ MediaSourceEnabled=true ] -->
+<html>
+<head>
+    <title>Test scrolling and seeking in an initially hidden an muted MSE video - it should not crash</title>
+    <style>
+        .fullheight {
+            height: 100vh;
+        }
+    </style>
+    <script src="../../media/media-source/media-source-loader.js"></script>
+    <script src="../../media/video-test.js"></script>
+    <script>
+    function createObserver() {
+        let handleIntersect = (entries, observer) => {
+            entries.forEach((entry) => {
+                video.currentTime = 0;
+                video.play();
+            });
+        }
+        let options = {
+            root: null,
+            rootMargin: "0px",
+            threshold: [0],
+        };
+        let observer = new IntersectionObserver(handleIntersect, options);
+        observer.observe(video);
+    }
+
+    async function runTest() {
+        findMediaElement();
+        createObserver();
+
+        let source = new MediaSource();
+        video.src = URL.createObjectURL(source);
+        await waitFor(source, 'sourceopen', true);
+
+        let loader = new MediaSourceLoader('content/test-fragmented-video-manifest.json');
+        await loader.load();
+        await loader.loadMediaData();
+
+        let sourceBuffer = source.addSourceBuffer(loader.type());
+        sourceBuffer.appendBuffer(loader.initSegment());
+        await waitFor(sourceBuffer, 'update', true);
+        sourceBuffer.appendBuffer(loader.mediaSegment(0));
+        await waitFor(sourceBuffer, 'update', true);
+        source.endOfStream();
+        video.play();
+
+        for (let numScrolled = 0; numScrolled < 10; ++numScrolled) {
+            await sleepFor(50);
+            const isVideoVisible = (numScrolled % 2 != 0);
+            const scrollTargetY = isVideoVisible ? 0 : window.innerHeight;
+            window.scrollTo(0, scrollTargetY);
+        }
+        source.removeSourceBuffer(sourceBuffer);
+        logResult(Success, 'No crash');
+        endTest();
+    }
+    </script>
+</head>
+<body onload="runTest()">
+    <div class="fullheight"></div>
+    <video muted></video>
+</body>
+</html>
diff --git a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp
@@ -331,8 +331,8 @@ void MediaPlayerPrivateGStreamer::load(const String& urlString)
         m_fillTimer.stop();
 
     ASSERT(m_pipeline);
-    setVisibleInViewport(player->isVisibleInViewport());
     setPlaybinURL(url);
+    setVisibleInViewport(player->isVisibleInViewport());
 
     GST_DEBUG_OBJECT(pipeline(), "preload: %s", convertEnumerationToString(m_preload).utf8().data());
     if (m_preload == MediaPlayer::Preload::None && !isMediaSource()) {
@@ -3955,10 +3955,18 @@ void MediaPlayerPrivateGStreamer::setVisibleInViewport(bool isVisible)
     if (!isVisible) {
         GstState currentState;
         gst_element_get_state(m_pipeline.get(), &currentState, nullptr, 0);
-        if (currentState > GST_STATE_NULL)
+        // WebKitMediaSrc cannot properly handle PAUSED -> READY -> PAUSED currently, so we have to avoid transitioning
+        // back to READY when the player becomes visible.
+        GstState minimumState = isMediaSource() ? GST_STATE_PAUSED : GST_STATE_READY;
+        if (currentState >= minimumState)
             m_invisiblePlayerState = currentState;
         m_isVisibleInViewport = false;
-        gst_element_set_state(m_pipeline.get(), GST_STATE_PAUSED);
+        // Avoid setting the pipeline to PAUSED unless the playbin URL has already been set,
+        // otherwise it will fail, and may leave the pipeline stuck on READY with PAUSE pending.
+        if (!m_url.isValid())
+            return;
+        [[maybe_unused]] auto setStateResult = gst_element_set_state(m_pipeline.get(), GST_STATE_PAUSED);
+        ASSERT(setStateResult != GST_STATE_CHANGE_FAILURE);
     } else {
         m_isVisibleInViewport = true;
         if (m_invisiblePlayerState != GST_STATE_VOID_PENDING)