- Revision
- 252398
- Author
- [email protected]
- Date
- 2019-11-13 02:43:58 -0800 (Wed, 13 Nov 2019)
Log Message
[GStreamer] Several issues while trying to play a video on NextCloud
https://bugs.webkit.org/show_bug.cgi?id=203194
Reviewed by Philippe Normand.
First problem was the loader not being restarted in PAUSED, so
sometimes playback never started since buffering never reached
100%.
Then, after investigating blocksizes and not being a viable
solution, reducing the 200_ms to 100_ms wait for new data was the
trick to avoid choppyness.
During investigation several issues were fixed like turning
GstQuery* into GRefPtr for MediaPlayerPrivateGStreamer::buffered,
making blocksize unsigned instead of uint64_t as it is in
GStreamer and creating and using WEBKIT_WEB_SRC_CAST since many
uses of WEBKIT_WEB_SRC cast were already protected by
WEBKIT_IS_WEB_SRC.
* platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
(WebCore::MediaPlayerPrivateGStreamer::buffered const): GRefPtr<GstQuery>.
(WebCore::MediaPlayerPrivateGStreamer::handleMessage):
(WebCore::MediaPlayerPrivateGStreamer::sourceSetup):
(WebCore::MediaPlayerPrivateGStreamer::didPassCORSAccessCheck const):
Use WEBKIT_WEB_SRC_CAST.
* platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:
(webKitWebSrcCreate): Reduce wait for data down to 100_ms and
request download restart also in PAUSED.
(CachedResourceStreamingClient::checkUpdateBlocksize): Turn
blocksize to unsigned.
* platform/graphics/gstreamer/WebKitWebSourceGStreamer.h: Add
WEBKIT_WEB_SRC_CAST.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (252397 => 252398)
--- trunk/Source/WebCore/ChangeLog 2019-11-13 10:22:04 UTC (rev 252397)
+++ trunk/Source/WebCore/ChangeLog 2019-11-13 10:43:58 UTC (rev 252398)
@@ -1,3 +1,39 @@
+2019-11-13 Xabier Rodriguez Calvar <[email protected]>
+
+ [GStreamer] Several issues while trying to play a video on NextCloud
+ https://bugs.webkit.org/show_bug.cgi?id=203194
+
+ Reviewed by Philippe Normand.
+
+ First problem was the loader not being restarted in PAUSED, so
+ sometimes playback never started since buffering never reached
+ 100%.
+
+ Then, after investigating blocksizes and not being a viable
+ solution, reducing the 200_ms to 100_ms wait for new data was the
+ trick to avoid choppyness.
+
+ During investigation several issues were fixed like turning
+ GstQuery* into GRefPtr for MediaPlayerPrivateGStreamer::buffered,
+ making blocksize unsigned instead of uint64_t as it is in
+ GStreamer and creating and using WEBKIT_WEB_SRC_CAST since many
+ uses of WEBKIT_WEB_SRC cast were already protected by
+ WEBKIT_IS_WEB_SRC.
+
+ * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
+ (WebCore::MediaPlayerPrivateGStreamer::buffered const): GRefPtr<GstQuery>.
+ (WebCore::MediaPlayerPrivateGStreamer::handleMessage):
+ (WebCore::MediaPlayerPrivateGStreamer::sourceSetup):
+ (WebCore::MediaPlayerPrivateGStreamer::didPassCORSAccessCheck const):
+ Use WEBKIT_WEB_SRC_CAST.
+ * platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:
+ (webKitWebSrcCreate): Reduce wait for data down to 100_ms and
+ request download restart also in PAUSED.
+ (CachedResourceStreamingClient::checkUpdateBlocksize): Turn
+ blocksize to unsigned.
+ * platform/graphics/gstreamer/WebKitWebSourceGStreamer.h: Add
+ WEBKIT_WEB_SRC_CAST.
+
2019-11-13 Rob Buis <[email protected]>
Support stale-while-revalidate cache strategy
Modified: trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp (252397 => 252398)
--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp 2019-11-13 10:22:04 UTC (rev 252397)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp 2019-11-13 10:43:58 UTC (rev 252398)
@@ -1146,17 +1146,15 @@
if (!mediaDuration || mediaDuration.isPositiveInfinite())
return timeRanges;
- GstQuery* query = gst_query_new_buffering(GST_FORMAT_PERCENT);
+ GRefPtr<GstQuery> query = adoptGRef(gst_query_new_buffering(GST_FORMAT_PERCENT));
- if (!gst_element_query(m_pipeline.get(), query)) {
- gst_query_unref(query);
+ if (!gst_element_query(m_pipeline.get(), query.get()))
return timeRanges;
- }
- guint numBufferingRanges = gst_query_get_n_buffering_ranges(query);
+ guint numBufferingRanges = gst_query_get_n_buffering_ranges(query.get());
for (guint index = 0; index < numBufferingRanges; index++) {
gint64 rangeStart = 0, rangeStop = 0;
- if (gst_query_parse_nth_buffering_range(query, index, &rangeStart, &rangeStop)) {
+ if (gst_query_parse_nth_buffering_range(query.get(), index, &rangeStart, &rangeStop)) {
uint64_t startTime = gst_util_uint64_scale_int_round(toGstUnsigned64Time(mediaDuration), rangeStart, GST_FORMAT_PERCENT_MAX);
uint64_t stopTime = gst_util_uint64_scale_int_round(toGstUnsigned64Time(mediaDuration), rangeStop, GST_FORMAT_PERCENT_MAX);
timeRanges->add(MediaTime(startTime, GST_SECOND), MediaTime(stopTime, GST_SECOND));
@@ -1171,8 +1169,6 @@
timeRanges->add(MediaTime::zeroTime(), loaded);
}
- gst_query_unref(query);
-
return timeRanges;
}
@@ -1393,7 +1389,7 @@
} else if (gst_structure_has_name(structure, "adaptive-streaming-statistics")) {
if (WEBKIT_IS_WEB_SRC(m_source.get()) && !webkitGstCheckVersion(1, 12, 0)) {
if (const char* uri = gst_structure_get_string(structure, "uri"))
- m_hasTaintedOrigin = webKitSrcWouldTaintOrigin(WEBKIT_WEB_SRC(m_source.get()), SecurityOrigin::create(URL(URL(), uri)));
+ m_hasTaintedOrigin = webKitSrcWouldTaintOrigin(WEBKIT_WEB_SRC_CAST(m_source.get()), SecurityOrigin::create(URL(URL(), uri)));
}
} else
GST_DEBUG_OBJECT(pipeline(), "Unhandled element message: %" GST_PTR_FORMAT, structure);
@@ -1854,7 +1850,7 @@
m_source = sourceElement;
if (WEBKIT_IS_WEB_SRC(m_source.get())) {
- webKitWebSrcSetMediaPlayer(WEBKIT_WEB_SRC(m_source.get()), m_player);
+ webKitWebSrcSetMediaPlayer(WEBKIT_WEB_SRC_CAST(m_source.get()), m_player);
g_signal_connect(GST_ELEMENT_PARENT(m_source.get()), "element-added", G_CALLBACK(uriDecodeBinElementAddedCallback), this);
#if ENABLE(MEDIA_STREAM)
} else if (WEBKIT_IS_MEDIA_STREAM_SRC(sourceElement)) {
@@ -2522,7 +2518,7 @@
bool MediaPlayerPrivateGStreamer::didPassCORSAccessCheck() const
{
if (WEBKIT_IS_WEB_SRC(m_source.get()))
- return webKitSrcPassedCORSAccessCheck(WEBKIT_WEB_SRC(m_source.get()));
+ return webKitSrcPassedCORSAccessCheck(WEBKIT_WEB_SRC_CAST(m_source.get()));
return false;
}
Modified: trunk/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp (252397 => 252398)
--- trunk/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp 2019-11-13 10:22:04 UTC (rev 252397)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp 2019-11-13 10:43:58 UTC (rev 252398)
@@ -61,7 +61,7 @@
void setSourceElement(WebKitWebSrc* src) { m_src = GST_ELEMENT_CAST(src); }
private:
- void checkUpdateBlocksize(uint64_t bytesRead);
+ void checkUpdateBlocksize(unsigned bytesRead);
// PlatformMediaResourceClient virtual methods.
void responseReceived(PlatformMediaResource&, const ResourceResponse&, CompletionHandler<void(ShouldContinue)>&&) override;
@@ -137,7 +137,7 @@
bool isSeekable;
bool isSeeking;
bool wasSeeking { false };
- uint64_t minimumBlocksize;
+ unsigned minimumBlocksize;
Lock adapterLock;
Condition adapterCondition;
uint64_t queueSize { 0 };
@@ -412,20 +412,21 @@
unsigned retries = 0;
size_t available = gst_adapter_available_fast(priv->adapter.get());
while (available < size && !isAdapterDrained) {
- priv->adapterCondition.waitFor(priv->adapterLock, 200_ms, [&] {
+ priv->adapterCondition.waitFor(priv->adapterLock, 100_ms, [&] {
return gst_adapter_available_fast(priv->adapter.get()) >= size;
});
retries++;
available = gst_adapter_available_fast(priv->adapter.get());
- if (available && available < size)
+ if (available && available < size) {
+ GST_TRACE_OBJECT(src, "did not get the %u blocksize bytes, let's push the %" G_GSIZE_FORMAT " bytes we got", size, available);
size = available;
- else if (retries > 3)
+ } else if (retries > 3)
isAdapterDrained = true;
}
}
if (isAdapterDrained) {
- GST_DEBUG_OBJECT(src, "Adapter still empty after 800 milli-seconds of waiting, assuming EOS");
+ GST_DEBUG_OBJECT(src, "Adapter still empty after 400 milli-seconds of waiting, assuming EOS");
return GST_FLOW_EOS;
}
@@ -464,7 +465,7 @@
&& (priv->size > SMALL_MEDIA_RESOURCE_MAX_SIZE) && priv->readPosition
&& (priv->readPosition != priv->size)
&& (priv->queueSize < (priv->size * HIGH_QUEUE_FACTOR_THRESHOLD * LOW_QUEUE_FACTOR_THRESHOLD))
- && (GST_STATE(src) == GST_STATE_PLAYING) && priv->isDownloadSuspended) {
+ && GST_STATE(src) >= GST_STATE_PAUSED && priv->isDownloadSuspended) {
GST_DEBUG_OBJECT(src, "[Buffering] Adapter running out of data, restarting download");
priv->isDownloadSuspended = false;
webKitWebSrcMakeRequest(baseSrc, false);
@@ -920,14 +921,14 @@
CachedResourceStreamingClient::~CachedResourceStreamingClient() = default;
-void CachedResourceStreamingClient::checkUpdateBlocksize(uint64_t bytesRead)
+void CachedResourceStreamingClient::checkUpdateBlocksize(unsigned bytesRead)
{
WebKitWebSrc* src = ""
GstBaseSrc* baseSrc = GST_BASE_SRC_CAST(src);
WebKitWebSrcPrivate* priv = src->priv;
- uint64_t blocksize = gst_base_src_get_blocksize(baseSrc);
- GST_LOG_OBJECT(src, "Checking to update blocksize. Read: %" PRIu64 ", current blocksize: %" PRIu64, bytesRead, blocksize);
+ unsigned blocksize = gst_base_src_get_blocksize(baseSrc);
+ GST_LOG_OBJECT(src, "Checking to update blocksize. Read: %u, current blocksize: %u", bytesRead, blocksize);
if (bytesRead >= blocksize * s_growBlocksizeLimit) {
m_reduceBlocksizeCount = 0;
@@ -935,7 +936,7 @@
if (m_increaseBlocksizeCount >= s_growBlocksizeCount) {
blocksize *= s_growBlocksizeFactor;
- GST_DEBUG_OBJECT(src, "Increased blocksize to %" PRIu64, blocksize);
+ GST_DEBUG_OBJECT(src, "Increased blocksize to %u", blocksize);
gst_base_src_set_blocksize(baseSrc, blocksize);
m_increaseBlocksizeCount = 0;
}
@@ -946,7 +947,7 @@
if (m_reduceBlocksizeCount >= s_reduceBlocksizeCount) {
blocksize *= s_reduceBlocksizeFactor;
blocksize = std::max(blocksize, priv->minimumBlocksize);
- GST_DEBUG_OBJECT(src, "Decreased blocksize to %" PRIu64, blocksize);
+ GST_DEBUG_OBJECT(src, "Decreased blocksize to %u", blocksize);
gst_base_src_set_blocksize(baseSrc, blocksize);
m_reduceBlocksizeCount = 0;
}
Modified: trunk/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.h (252397 => 252398)
--- trunk/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.h 2019-11-13 10:22:04 UTC (rev 252397)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.h 2019-11-13 10:43:58 UTC (rev 252398)
@@ -35,6 +35,7 @@
#define WEBKIT_WEB_SRC_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), WEBKIT_TYPE_WEB_SRC, WebKitWebSrcClass))
#define WEBKIT_IS_WEB_SRC(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), WEBKIT_TYPE_WEB_SRC))
#define WEBKIT_IS_WEB_SRC_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), WEBKIT_TYPE_WEB_SRC))
+#define WEBKIT_WEB_SRC_CAST(obj) ((WebKitWebSrc*)(obj))
#define WEBKIT_WEB_SRC_PLAYER_CONTEXT_TYPE_NAME "webkit.media-player"
