[WebM] MediaPlayerPrivateWebM can never enter in error state

https://bugs.webkit.org/show_bug.cgi?id=264931 rdar://118499989 Reviewed by Youenn Fablet. Parser return value wasn't read, therefore we couldn't detect errors. * LayoutTests/media/content/opus_variable_witherror.webm: Added. * LayoutTests/media/media-webm-opus-error-expected.txt: Added. * LayoutTests/media/media-webm-opus-error.html: Added. * LayoutTests/platform/mac-wk1/TestExpectations: * Source/WebCore/platform/graphics/cocoa/MediaPlayerPrivateWebM.h: * Source/WebCore/platform/graphics/cocoa/MediaPlayerPrivateWebM.mm: (WebCore::MediaPlayerPrivateWebM::appendCompleted): (WebCore::MediaPlayerPrivateWebM::append): Canonical link: https://commits.webkit.org/270938@main
WebKit · Nov 18, 2023 · 15b788c · 15b788c
1 parent d42df0c
commit 15b788c
Show file tree

Hide file tree

Showing 7 changed files with 50 additions and 6 deletions.
diff --git a/LayoutTests/media/content/opus_variable_witherror.webm b/LayoutTests/media/content/opus_variable_witherror.webm
diff --git a/LayoutTests/media/media-webm-opus-error-expected.txt b/LayoutTests/media/media-webm-opus-error-expected.txt
@@ -0,0 +1,10 @@
+
+Check that a webm with garbage content at the end generate an error.
+
+EXPECTED (audio.canPlayType('audio/webm; codecs=opus') == 'probably') OK
+RUN(audio.src = "content/opus_variable_witherror.webm")
+EVENT(loadedmetadata)
+EXPECTED (audio.duration >= '0.98') OK
+EVENT(error)
+END OF TEST
+
diff --git a/LayoutTests/media/media-webm-opus-error.html b/LayoutTests/media/media-webm-opus-error.html
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <script src="video-test.js"></script>
+        <script>
+            var audio = null;
+            async function start() {
+                audio = mediaElement = document.getElementsByTagName('audio')[0];
+
+                testExpected("audio.canPlayType('audio/webm; codecs=opus')", "probably");
+                waitForEvent('error', endTest);
+                run('audio.src = "content/opus_variable_witherror.webm"');
+                await waitFor(audio, 'loadedmetadata');
+                testExpected('audio.duration', '0.98', '>=');
+            }
+        </script>
+    </head>
+    <body onload="start()">
+        <audio controls></audio>
+        <p>Check that a webm with garbage content at the end generate an error.</p>
+    </body>
+</html>
diff --git a/LayoutTests/platform/glib/TestExpectations b/LayoutTests/platform/glib/TestExpectations
@@ -1105,6 +1105,7 @@ webkit.org/b/167108 imported/w3c/web-platform-tests/media-source/mediasource-tra
 webkit.org/b/146720 media/accessiblity-describes-video.html [ Failure ]
 
 webkit.org/b/227934 media/media-source/media-webm-vorbis-partial.html [ Failure ]
+webkit.org/b/265016 media/media-webm-opus-error.html [ Failure ]
 
 webkit.org/b/229973 media/track/in-band/track-in-band-kate-ogg-cues-added-once.html [ Failure Pass ]
 webkit.org/b/229973 media/track/in-band/track-in-band-srt-mkv-cues-added-once.html [ Failure Pass ]

diff --git a/LayoutTests/platform/mac-wk1/TestExpectations b/LayoutTests/platform/mac-wk1/TestExpectations
@@ -1023,6 +1023,9 @@ media/media-source/media-source-webm-configuration-vp9-header-color.html [ Failu
 
 # CRABS WebM isn't supported in WK1
 media/media-webm-opus-buffered.html [ Failure ]
+media/media-webm-opus-error.html [ Failure ]
+media/media-webm-opus-variable-length.html [ Failure ]
+media/media-webm-no-duration.html [ Failure ]
 
 # Screenshots timeline does not capture screenshots for changes in WebKitLegacy.
 webkit.org/b/251113 inspector/timeline/timeline-event-screenshots.html [ Skip ]

diff --git a/Source/WebCore/platform/graphics/cocoa/MediaPlayerPrivateWebM.h b/Source/WebCore/platform/graphics/cocoa/MediaPlayerPrivateWebM.h
@@ -199,7 +199,7 @@ class MediaPlayerPrivateWebM
 
     bool isReadyForMoreSamples(uint64_t);
     void didBecomeReadyForMoreSamples(uint64_t);
-    void appendCompleted();
+    void appendCompleted(bool);
     void provideMediaData(uint64_t);
     void provideMediaData(TrackBuffer&, uint64_t);
 
@@ -310,6 +310,7 @@ class MediaPlayerPrivateWebM
     mutable bool m_loadingProgressed { false };
     bool m_loadFinished { false };
     bool m_delayedIdle { false };
+    bool m_errored { false };
     bool m_processingInitializationSegment { false };
 };
 

diff --git a/Source/WebCore/platform/graphics/cocoa/MediaPlayerPrivateWebM.mm b/Source/WebCore/platform/graphics/cocoa/MediaPlayerPrivateWebM.mm
@@ -888,19 +888,26 @@ static bool isCopyDisplayedPixelBufferAvailable()
     provideMediaData(trackId);
 }
 
-void MediaPlayerPrivateWebM::appendCompleted()
+void MediaPlayerPrivateWebM::appendCompleted(bool success)
 {
     ASSERT(m_pendingAppends > 0);
     m_pendingAppends--;
     INFO_LOG(LOGIDENTIFIER, "pending appends = ", m_pendingAppends);
     setLoadingProgresssed(true);
-    updateBufferedFromTrackBuffers(m_loadFinished && !m_pendingAppends);
+    m_errored |= !success;
+    if (!m_errored)
+        updateBufferedFromTrackBuffers(m_loadFinished && !m_pendingAppends);
+
     if (m_loadFinished && !m_pendingAppends) {
         if (!m_hasVideo && !m_hasAudio) {
             ERROR_LOG(LOGIDENTIFIER, "could not load audio or video tracks");
             setNetworkState(MediaPlayer::NetworkState::FormatError);
             setReadyState(MediaPlayer::ReadyState::HaveNothing);
-            setNetworkState(MediaPlayer::NetworkState::Idle);
+            return;
+        }
+        if (m_errored) {
+            ERROR_LOG(LOGIDENTIFIER, "parsing error");
+            setNetworkState(m_readyState >= MediaPlayer::ReadyState::HaveMetadata ? MediaPlayer::NetworkState::DecodeError : MediaPlayer::NetworkState::FormatError);
             return;
         }
         if (m_readyState >= MediaPlayer::ReadyState::HaveMetadata)
@@ -1157,10 +1164,10 @@ static bool isCopyDisplayedPixelBufferAvailable()
     SourceBufferParser::Segment segment(Ref { buffer });
     invokeAsync(m_appendQueue, [segment = WTFMove(segment), parser = m_parser]() mutable {
         return MediaPromise::createAndSettle(parser->appendData(WTFMove(segment)));
-    })->whenSettled(RunLoop::current(), [weakThis = WeakPtr { *this }] {
+    })->whenSettled(RunLoop::current(), [weakThis = WeakPtr { *this }](auto&& result) {
         if (!weakThis)
             return;
-        weakThis->appendCompleted();
+        weakThis->appendCompleted(!!result);
     });
 }