- Revision
- 278988
- Author
- [email protected]
- Date
- 2021-06-17 08:57:59 -0700 (Thu, 17 Jun 2021)
Log Message
[GTK] Unexpected timeout in http/tests/media/video-play-stall-seek.html
https://bugs.webkit.org/show_bug.cgi?id=196198
Reviewed by Philippe Normand.
Source/WebCore:
Increased default GstDowloadBuffer size to prevent a race condition. To do that, we need
to set the buffer-size of GstUriDecodebin (that will set the GstMultiQueue size, so we use
the default value that would be used if no changes had been made, and also leaves
GstDownloadBuffer size untouched, which wouldn't happen if no buffer-size had been set)
and then set the desired max-size-bytes on GstDownloadBuffer.
The race condition was that some times the player private readyState went up to
HAVE_ENOUGH_DATA and then back to HAVE_CURRENT_DATA (triggering the expected waiting event),
and some others times went directly to HAVE_CURRENT_DATA (no waiting event, test stalled).
Increasing the buffer size gave more time for the double transition to happen.
Still, these changes weren't enough to get the test passing, as with these changes the
multiqueue sucked all the data, downloadbuffer fell to a low percentage and updateStates()
paused the pipeline for rebuffering. The pipeline won't ever be unpaused because at that
point WebKitWebSrc has reached EOS (no more file to download, a side effect of the current
libsoup behaviour) and the buffering won't ever go up again (and trigger the unpause).
This was solved by setting the downloadbuffer high-percent property to 0 when a seek
is done after EOS has been received by WebKitWebSrc. This effectively forces the
downloadbuffer to report 100% buffered, which unpauses the pipeline and lets the playback
continue after seek.
* platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
(WebCore::MediaPlayerPrivateGStreamer::doSeek): Set high-percent when seeking after WebKitWebSrc EOS.
(WebCore::MediaPlayerPrivateGStreamer::sourceSetup): Manually set buffer-size to its default value.
(WebCore::MediaPlayerPrivateGStreamer::handleMessage): Receive the custom EOS message from WebKitWebSrc and remember it.
(WebCore::MediaPlayerPrivateGStreamer::uriDecodeBinElementAddedCallback): Manully set max-size-bytes to a higher than default value.
(WebCore::MediaPlayerPrivateGStreamer::downloadBufferFileCreatedCallback): Don't clean the reference to the downloadbuffer, as it'll be needed later.
* platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h: Added m_hasWebKitWebSrcSentEOS to remember the EOS condition.
* platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:
(webKitWebSrcCreate): Notify the EOS condition using the bus, so the player private can handle it.
(CachedResourceStreamingClient::loadFailed): Extra useful logs.
(CachedResourceStreamingClient::loadFinished): Ditto.
LayoutTests:
Make test more stable and don't expect NETWORK_LOADING, because some network implementations,
like libsoup, just report end-of-stream and finish the download.
* http/tests/media/video-play-stall-seek-expected.txt: Removed NETWORK_LOADING expectation.
* http/tests/media/video-play-stall-seek.html: Stabilize the test by not listening to waiting events once the relevant one has been handled, and by pausing the video after the test has finished. Also re
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (278987 => 278988)
--- trunk/LayoutTests/ChangeLog 2021-06-17 15:53:44 UTC (rev 278987)
+++ trunk/LayoutTests/ChangeLog 2021-06-17 15:57:59 UTC (rev 278988)
@@ -1,3 +1,19 @@
+2021-06-17 Enrique Ocaña González <[email protected]>
+
+ [GTK] Unexpected timeout in http/tests/media/video-play-stall-seek.html
+ https://bugs.webkit.org/show_bug.cgi?id=196198
+
+ Reviewed by Philippe Normand.
+
+ Make test more stable and don't expect NETWORK_LOADING, because some network implementations,
+ like libsoup, just report end-of-stream and finish the download.
+
+ * http/tests/media/video-play-stall-seek-expected.txt: Removed NETWORK_LOADING expectation.
+ * http/tests/media/video-play-stall-seek.html: Stabilize the test by not listening to waiting events once the relevant one has been handled, and by pausing the video after the test has finished. Also re
+move the NETWORK_LOADING check.
+ * platform/glib/TestExpectations: Unskipped test.
+ * platform/gtk-wayland/TestExpectations: Ditto.
+
2021-06-17 Philippe Normand <[email protected]>
Unreviewed GStreamer mediastream gardening
Modified: trunk/LayoutTests/http/tests/media/video-play-stall-seek-expected.txt (278987 => 278988)
--- trunk/LayoutTests/http/tests/media/video-play-stall-seek-expected.txt 2021-06-17 15:53:44 UTC (rev 278987)
+++ trunk/LayoutTests/http/tests/media/video-play-stall-seek-expected.txt 2021-06-17 15:57:59 UTC (rev 278988)
@@ -7,6 +7,5 @@
RUN(video.currentTime = 0.1)
EVENT(canplay)
EXPECTED (video.readyState >= '2') OK
-EXPECTED (video.networkState == '2') OK
END OF TEST
Modified: trunk/LayoutTests/http/tests/media/video-play-stall-seek.html (278987 => 278988)
--- trunk/LayoutTests/http/tests/media/video-play-stall-seek.html 2021-06-17 15:53:44 UTC (rev 278987)
+++ trunk/LayoutTests/http/tests/media/video-play-stall-seek.html 2021-06-17 15:57:59 UTC (rev 278988)
@@ -6,8 +6,7 @@
var waitingCount = 0;
- video.addEventListener('waiting', function () {
-
+ video.addEventListener('waiting', function onWaiting() {
// Skip the first 'waiting' event because it is fired when we call play()
// and there isn't enough media to begin playback immediately.
if (++waitingCount == 1)
@@ -14,12 +13,13 @@
return;
consoleWrite("EVENT(waiting)");
+ video.removeEventListener('waiting', onWaiting);
// now that playback has paused to wait for data, seek back and verify that we
// get a 'canplay' event
waitForEvent('canplay' , function () {
testExpected("video.readyState", HTMLMediaElement.HAVE_CURRENT_DATA, ">=");
- testExpected("video.networkState", HTMLMediaElement.NETWORK_LOADING);
+ setTimeout(1, function() { video.pause(); });
endTest();
} );
testExpected("video.readyState", HTMLMediaElement.HAVE_CURRENT_DATA);
Modified: trunk/LayoutTests/platform/glib/TestExpectations (278987 => 278988)
--- trunk/LayoutTests/platform/glib/TestExpectations 2021-06-17 15:53:44 UTC (rev 278987)
+++ trunk/LayoutTests/platform/glib/TestExpectations 2021-06-17 15:57:59 UTC (rev 278988)
@@ -2276,8 +2276,6 @@
webkit.org/b/174242 media/media-fullscreen-pause-inline.html [ Skip ]
webkit.org/b/182108 http/tests/media/hls/hls-webvtt-tracks.html [ Timeout ]
webkit.org/b/137311 media/video-fullscreen-only-playback.html [ Timeout Crash ]
-webkit.org/b/196198 [ Release ] http/tests/media/video-play-stall-seek.html [ Timeout Failure Pass ]
-webkit.org/b/196198 [ Debug ] http/tests/media/video-play-stall-seek.html [ Timeout Failure ]
webkit.org/b/198118 media/playlist-inherits-user-gesture.html [ Timeout ]
webkit.org/b/198113 fast/mediastream/media-stream-page-muted.html [ Crash Timeout ]
Modified: trunk/LayoutTests/platform/gtk-wayland/TestExpectations (278987 => 278988)
--- trunk/LayoutTests/platform/gtk-wayland/TestExpectations 2021-06-17 15:53:44 UTC (rev 278987)
+++ trunk/LayoutTests/platform/gtk-wayland/TestExpectations 2021-06-17 15:57:59 UTC (rev 278988)
@@ -254,8 +254,6 @@
webkit.org/b/158923 media/airplay-autoplay.html [ Timeout ]
webkit.org/b/163523 media/track/track-manual-mode.html [ Failure Timeout ]
-webkit.org/b/196198 http/tests/media/video-play-stall-seek.html [ Timeout Failure ]
-
#////////////////////////////////////////////////////////////////////////////////////////
# End of Tests timing out
#////////////////////////////////////////////////////////////////////////////////////////
Modified: trunk/Source/WebCore/ChangeLog (278987 => 278988)
--- trunk/Source/WebCore/ChangeLog 2021-06-17 15:53:44 UTC (rev 278987)
+++ trunk/Source/WebCore/ChangeLog 2021-06-17 15:57:59 UTC (rev 278988)
@@ -1,3 +1,44 @@
+2021-06-17 Enrique Ocaña González <[email protected]>
+
+ [GTK] Unexpected timeout in http/tests/media/video-play-stall-seek.html
+ https://bugs.webkit.org/show_bug.cgi?id=196198
+
+ Reviewed by Philippe Normand.
+
+ Increased default GstDowloadBuffer size to prevent a race condition. To do that, we need
+ to set the buffer-size of GstUriDecodebin (that will set the GstMultiQueue size, so we use
+ the default value that would be used if no changes had been made, and also leaves
+ GstDownloadBuffer size untouched, which wouldn't happen if no buffer-size had been set)
+ and then set the desired max-size-bytes on GstDownloadBuffer.
+
+ The race condition was that some times the player private readyState went up to
+ HAVE_ENOUGH_DATA and then back to HAVE_CURRENT_DATA (triggering the expected waiting event),
+ and some others times went directly to HAVE_CURRENT_DATA (no waiting event, test stalled).
+ Increasing the buffer size gave more time for the double transition to happen.
+
+ Still, these changes weren't enough to get the test passing, as with these changes the
+ multiqueue sucked all the data, downloadbuffer fell to a low percentage and updateStates()
+ paused the pipeline for rebuffering. The pipeline won't ever be unpaused because at that
+ point WebKitWebSrc has reached EOS (no more file to download, a side effect of the current
+ libsoup behaviour) and the buffering won't ever go up again (and trigger the unpause).
+
+ This was solved by setting the downloadbuffer high-percent property to 0 when a seek
+ is done after EOS has been received by WebKitWebSrc. This effectively forces the
+ downloadbuffer to report 100% buffered, which unpauses the pipeline and lets the playback
+ continue after seek.
+
+ * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
+ (WebCore::MediaPlayerPrivateGStreamer::doSeek): Set high-percent when seeking after WebKitWebSrc EOS.
+ (WebCore::MediaPlayerPrivateGStreamer::sourceSetup): Manually set buffer-size to its default value.
+ (WebCore::MediaPlayerPrivateGStreamer::handleMessage): Receive the custom EOS message from WebKitWebSrc and remember it.
+ (WebCore::MediaPlayerPrivateGStreamer::uriDecodeBinElementAddedCallback): Manully set max-size-bytes to a higher than default value.
+ (WebCore::MediaPlayerPrivateGStreamer::downloadBufferFileCreatedCallback): Don't clean the reference to the downloadbuffer, as it'll be needed later.
+ * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h: Added m_hasWebKitWebSrcSentEOS to remember the EOS condition.
+ * platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:
+ (webKitWebSrcCreate): Notify the EOS condition using the bus, so the player private can handle it.
+ (CachedResourceStreamingClient::loadFailed): Extra useful logs.
+ (CachedResourceStreamingClient::loadFinished): Ditto.
+
2021-06-17 Oriol Brufau <[email protected]>
[css-logical] Implement logical property groups
Modified: trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp (278987 => 278988)
--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp 2021-06-17 15:53:44 UTC (rev 278987)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp 2021-06-17 15:57:59 UTC (rev 278988)
@@ -88,6 +88,7 @@
#include <wtf/MathExtras.h>
#include <wtf/MediaTime.h>
#include <wtf/NeverDestroyed.h>
+#include <wtf/StdLibExtras.h>
#include <wtf/StringPrintStream.h>
#include <wtf/text/AtomString.h>
#include <wtf/text/CString.h>
@@ -451,6 +452,11 @@
if (!rate)
rate = 1.0;
+ if (m_hasWebKitWebSrcSentEOS && m_downloadBuffer) {
+ GST_DEBUG_OBJECT(pipeline(), "Setting high-percent=0 on GstDownloadBuffer to force 100%% buffered reporting");
+ g_object_set(m_downloadBuffer.get(), "high-percent", 0, nullptr);
+ }
+
GST_DEBUG_OBJECT(pipeline(), "[Seek] Performing actual seek to %" GST_TIME_FORMAT " (endTime: %" GST_TIME_FORMAT ") at rate %f", GST_TIME_ARGS(toGstClockTime(startTime)), GST_TIME_ARGS(toGstClockTime(endTime)), rate);
return gst_element_seek(m_pipeline.get(), rate, GST_FORMAT_TIME, seekFlags,
GST_SEEK_TYPE_SET, toGstClockTime(startTime), GST_SEEK_TYPE_SET, toGstClockTime(endTime));
@@ -861,6 +867,8 @@
if (WEBKIT_IS_WEB_SRC(m_source.get())) {
webKitWebSrcSetMediaPlayer(WEBKIT_WEB_SRC_CAST(m_source.get()), m_player, m_referrer);
g_signal_connect(GST_ELEMENT_PARENT(m_source.get()), "element-added", G_CALLBACK(uriDecodeBinElementAddedCallback), this);
+ // This will set the multiqueue size to the default value.
+ g_object_set(GST_ELEMENT_PARENT(m_source.get()), "buffer-size", 2 * MB, nullptr);
#if ENABLE(MEDIA_STREAM)
} else if (WEBKIT_IS_MEDIA_STREAM_SRC(sourceElement)) {
auto stream = m_streamPrivate.get();
@@ -1893,6 +1901,9 @@
m_fillTimer.stop();
m_bufferingPercentage = 100;
updateStates();
+ } else if (gst_structure_has_name(structure, "webkit-web-src-has-eos")) {
+ GST_DEBUG_OBJECT(pipeline(), "WebKitWebSrc has EOS");
+ m_hasWebKitWebSrcSentEOS = true;
} else
GST_DEBUG_OBJECT(pipeline(), "Unhandled element message: %" GST_PTR_FORMAT, structure);
break;
@@ -2099,6 +2110,9 @@
g_signal_handlers_disconnect_by_func(bin, reinterpret_cast<gpointer>(uriDecodeBinElementAddedCallback), player);
g_signal_connect_swapped(element, "notify::temp-location", G_CALLBACK(downloadBufferFileCreatedCallback), player);
+ // Set the GstDownloadBuffer size to our preferred value controls the thresholds for buffering events.
+ g_object_set(element, "max-size-bytes", 100 * KB, nullptr);
+
GUniqueOutPtr<char> oldDownloadTemplate;
g_object_get(element, "temp-template", &oldDownloadTemplate.outPtr(), nullptr);
@@ -2118,7 +2132,6 @@
GUniqueOutPtr<char> downloadFile;
g_object_get(player->m_downloadBuffer.get(), "temp-location", &downloadFile.outPtr(), nullptr);
- player->m_downloadBuffer = nullptr;
if (UNLIKELY(!FileSystem::deleteFile(downloadFile.get()))) {
GST_WARNING("Couldn't unlink media temporary file %s after creation", downloadFile.get());
Modified: trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h (278987 => 278988)
--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h 2021-06-17 15:53:44 UTC (rev 278987)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h 2021-06-17 15:57:59 UTC (rev 278988)
@@ -512,6 +512,7 @@
#endif
bool m_isBuffering { false };
int m_bufferingPercentage { 0 };
+ bool m_hasWebKitWebSrcSentEOS { false };
mutable unsigned long long m_totalBytes { 0 };
URL m_url;
bool m_shouldPreservePitch { false };
Modified: trunk/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp (278987 => 278988)
--- trunk/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp 2021-06-17 15:53:44 UTC (rev 278987)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp 2021-06-17 15:57:59 UTC (rev 278988)
@@ -565,6 +565,10 @@
// If the queue is empty reached this point, the only other option is that we are in EOS.
ASSERT(members->doesHaveEOS);
GST_DEBUG_OBJECT(src, "Reached the end of the response, signalling EOS");
+
+ gst_element_post_message(GST_ELEMENT_CAST(src), gst_message_new_element(GST_OBJECT_CAST(src),
+ gst_structure_new_empty("webkit-web-src-has-eos")));
+
return GST_FLOW_EOS;
}
@@ -1149,7 +1153,8 @@
if (!error.isCancellation()) {
GST_ERROR_OBJECT(src, "R%u: Have failure: %s", m_requestNumber, error.localizedDescription().utf8().data());
GST_ELEMENT_ERROR(src, RESOURCE, FAILED, ("R%u: %s", m_requestNumber, error.localizedDescription().utf8().data()), (nullptr));
- }
+ } else
+ GST_LOG_OBJECT(src, "R%u: Request cancelled: %s", m_requestNumber, error.localizedDescription().utf8().data());
members->doesHaveEOS = true;
members->responseCondition.notifyOne();
@@ -1163,6 +1168,8 @@
if (members->requestNumber != m_requestNumber)
return;
+ GST_LOG_OBJECT(src, "R%u: Load finished. Read position: %" G_GUINT64_FORMAT, m_requestNumber, members->readPosition);
+
members->doesHaveEOS = true;
members->responseCondition.notifyOne();
}
