Attachment 373239 Details for Bug 198376 – Patch

[patch] Patch

bug-198376-20190701172139.patch (text/plain), 16.53 KB, created by Philippe Normand on 2019-07-01 09:21:41 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Philippe Normand

Created: 2019-07-01 09:21:41 PDT

Size: 16.53 KB

patch

obsolete

>Subversion Revision: 247007
>diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
>index 6b5b8283847bb8d77d474cd9117d5bf11b188c82..c2f305e77004e1ea1a65c6b9cf0ceb352aaf78a2 100644
>--- a/Source/WTF/ChangeLog
>+++ b/Source/WTF/ChangeLog
>@@ -1,3 +1,13 @@
>+2019-07-01  Philippe Normand  <pnormand@igalia.com>
>+
>+        [GStreamer] Cannot play Bert's Bytes radio stream from http://radio.dos.nl/
>+        https://bugs.webkit.org/show_bug.cgi?id=198376
>+
>+        Reviewed by Xabier Rodriguez-Calvar.
>+
>+        * wtf/glib/GLibUtilities.h:
>+        (enumToString): Utility function to get a string representation of of a GLib enum.
>+
> 2019-06-22  Darin Adler  <darin@apple.com>
> 
>         Streamline some string code, focusing on functions that were using substringSharingImpl
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index ea5ad68f97dbbd8746f3809c6badd39c01bb3e0d..75890a7faa532e418411affa505875b9c787ab46 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,37 @@
>+2019-07-01  Philippe Normand  <pnormand@igalia.com>
>+
>+        [GStreamer] Cannot play Bert's Bytes radio stream from http://radio.dos.nl/
>+        https://bugs.webkit.org/show_bug.cgi?id=198376
>+
>+        Reviewed by Xabier Rodriguez-Calvar.
>+
>+        The delayed startup was due to a mix of buffering feedback
>+        messages not handled correctly by the player. We were handling
>+        download and streaming buffering metrics without distinction.
>+        Range requests (used for seeking) were also triggering on-disk
>+        buffering in some cases. The buffering percentage estimation based
>+        on network read position was not working either because uint64_t
>+        division doesn't return a floating point value.
>+
>+        No new tests, existing media tests cover this patch.
>+
>+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
>+        (WebCore::MediaPlayerPrivateGStreamer::commitLoad):
>+        (WebCore::MediaPlayerPrivateGStreamer::play):
>+        (WebCore::MediaPlayerPrivateGStreamer::handleMessage):
>+        (WebCore::MediaPlayerPrivateGStreamer::processBufferingStats):
>+        (WebCore::MediaPlayerPrivateGStreamer::updateBufferingStatus):
>+        (WebCore::MediaPlayerPrivateGStreamer::fillTimerFired):
>+        (WebCore::MediaPlayerPrivateGStreamer::maxTimeLoaded const):
>+        (WebCore::MediaPlayerPrivateGStreamer::didLoadingProgress const):
>+        (WebCore::MediaPlayerPrivateGStreamer::updateStates):
>+        (WebCore::MediaPlayerPrivateGStreamer::updateDownloadBufferingFlag):
>+        (WebCore::MediaPlayerPrivateGStreamer::setPreload):
>+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:
>+        * platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:
>+        (webkitWebSrcReset):
>+        * platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.h:
>+
> 2019-07-01  Miguel Gomez  <magomez@igalia.com>
> 
>         REGRESSION(r246963) GTK's debug build is broken
>diff --git a/Source/WTF/wtf/glib/GLibUtilities.cpp b/Source/WTF/wtf/glib/GLibUtilities.cpp
>index f44b9e5e7f48fd31d7617917f1da4b99bde91592..6ea4777b1423d35aa6d06cbf6732566b87a62961 100644
>--- a/Source/WTF/wtf/glib/GLibUtilities.cpp
>+++ b/Source/WTF/wtf/glib/GLibUtilities.cpp
>@@ -21,7 +21,6 @@
> #include <wtf/glib/GLibUtilities.h>
> 
> #include <glib.h>
>-#include <wtf/glib/GUniquePtr.h>
> 
> #if OS(WINDOWS)
> #include <windows.h>
>diff --git a/Source/WTF/wtf/glib/GLibUtilities.h b/Source/WTF/wtf/glib/GLibUtilities.h
>index 2993789c2baf588f1ee47912eaaa5f3eb7a4d922..67791841e825475091deb6f69f911294677306f6 100644
>--- a/Source/WTF/wtf/glib/GLibUtilities.h
>+++ b/Source/WTF/wtf/glib/GLibUtilities.h
>@@ -17,9 +17,9 @@
>  *  Boston, MA 02110-1301, USA.
>  */
> 
>-#ifndef GLibUtilities_h
>-#define GLibUtilities_h
>+#pragma once
> 
>+#include "GUniquePtr.h"
> #include <wtf/Assertions.h>
> #include <wtf/text/CString.h>
> 
>@@ -35,4 +35,15 @@ CString getCurrentExecutableName();
> #define GPOINTER_TO_ULONG(p) ((gulong) (p))
> #endif
> 
>+static inline GUniquePtr<char> enumToString(GType type, guint value)
>+{
>+#if GLIB_CHECK_VERSION(2, 54, 0)
>+    return GUniquePtr<char>(g_enum_to_string(type, value));
>+#else
>+    GEnumClass* enumClass = reinterpret_cast<GEnumClass*>(g_type_class_ref(type));
>+    GEnumValue* enumValue = g_enum_get_value(enumClass, value);
>+    char* representation = enumValue ? g_strdup(enumValue->value_nick) : nullptr;
>+    g_type_class_unref(enumClass);
>+    return GUniquePtr<char>(representation);
> #endif
>+}
>diff --git a/Source/WTF/wtf/glib/GUniquePtr.h b/Source/WTF/wtf/glib/GUniquePtr.h
>index e1cff8ddea44f6da72fdf0f00f11ca188396a6ed..2d0379332450e81618969700f5e774a32269369c 100644
>--- a/Source/WTF/wtf/glib/GUniquePtr.h
>+++ b/Source/WTF/wtf/glib/GUniquePtr.h
>@@ -24,6 +24,7 @@
> #if USE(GLIB)
> 
> #include <gio/gio.h>
>+#include <memory.h>
> #include <wtf/Noncopyable.h>
> 
> namespace WTF {
>diff --git a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp
>index 709a1b628466a12541bca8a142e96613f87adcde..97173f30615247678bab0ed3ce22ee6ccf70db59 100644
>--- a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp
>+++ b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp
>@@ -48,7 +48,7 @@
> #include <wtf/StringPrintStream.h>
> #include <wtf/URL.h>
> #include <wtf/WallTime.h>
>-#include <wtf/glib/GUniquePtr.h>
>+#include <wtf/glib/GLibUtilities.h>
> #include <wtf/glib/RunLoopSourcePriority.h>
> #include <wtf/text/CString.h>
> #include <wtf/text/StringConcatenateNumbers.h>
>@@ -349,7 +349,7 @@ void MediaPlayerPrivateGStreamer::commitLoad()
>     // start providing anything useful.
>     changePipelineState(GST_STATE_PAUSED);
> 
>-    setDownloadBuffering();
>+    updateDownloadBufferingFlag();
>     updateStates();
> }
> 
>@@ -458,7 +458,7 @@ void MediaPlayerPrivateGStreamer::play()
>         m_isEndReached = false;
>         m_delayingLoad = false;
>         m_preload = MediaPlayer::Auto;
>-        setDownloadBuffering();
>+        updateDownloadBufferingFlag();
>         GST_INFO_OBJECT(pipeline(), "Play");
>     } else
>         loadingFailed(MediaPlayer::Empty);
>@@ -1349,6 +1349,14 @@ void MediaPlayerPrivateGStreamer::handleMessage(GstMessage* message)
>                     break;
>                 }
>             }
>+
>+            bool isRangeRequest = false;
>+            GUniqueOutPtr<GstStructure> requestHeaders;
>+            if (gst_structure_get(structure, "request-headers", GST_TYPE_STRUCTURE, &requestHeaders.outPtr(), nullptr))
>+                isRangeRequest = gst_structure_has_field(requestHeaders.get(), "Range");
>+
>+            GST_DEBUG_OBJECT(pipeline(), "Is range request: %s", boolForPrinting(isRangeRequest));
>+
>             GUniqueOutPtr<GstStructure> responseHeaders;
>             if (gst_structure_get(structure, "response-headers", GST_TYPE_STRUCTURE, &responseHeaders.outPtr(), nullptr)) {
>                 const char* contentLengthHeaderName = httpHeaderNameString(HTTPHeaderName::ContentLength).utf8().data();
>@@ -1363,10 +1371,10 @@ void MediaPlayerPrivateGStreamer::handleMessage(GstMessage* message)
>                             contentLength = 0;
>                     }
>                 }
>-                GST_INFO_OBJECT(pipeline(), "%s stream detected", !contentLength ? "Live" : "Non-live");
>-                if (!contentLength) {
>-                    m_isStreaming = true;
>-                    setDownloadBuffering();
>+                if (!isRangeRequest) {
>+                    m_isStreaming = !contentLength;
>+                    GST_INFO_OBJECT(pipeline(), "%s stream detected", m_isStreaming ? "Live" : "Non-live");
>+                    updateDownloadBufferingFlag();
>                 }
>             }
>         } else if (gst_structure_has_name(structure, "webkit-network-statistics")) {
>@@ -1450,13 +1458,57 @@ void MediaPlayerPrivateGStreamer::handleMessage(GstMessage* message)
> 
> void MediaPlayerPrivateGStreamer::processBufferingStats(GstMessage* message)
> {
>-    m_buffering = true;
>-    gst_message_parse_buffering(message, &m_bufferingPercentage);
>+    GstBufferingMode mode;
>+    gst_message_parse_buffering_stats(message, &mode, nullptr, nullptr, nullptr);
>+
>+    int percentage;
>+    gst_message_parse_buffering(message, &percentage);
>+
>+    updateBufferingStatus(mode, percentage);
>+}
> 
>-    GST_DEBUG_OBJECT(pipeline(), "[Buffering] Buffering: %d%%.", m_bufferingPercentage);
>+void MediaPlayerPrivateGStreamer::updateMaxTimeLoaded(double percentage)
>+{
>+    MediaTime mediaDuration = durationMediaTime();
>+    if (!mediaDuration)
>+        return;
>+
>+    m_maxTimeLoaded = MediaTime(percentage * static_cast<double>(toGstUnsigned64Time(mediaDuration)) / 100, GST_SECOND);
>+    GST_DEBUG_OBJECT(pipeline(), "[Buffering] Updated maxTimeLoaded: %s", toString(m_maxTimeLoaded).utf8().data());
>+}
>+
>+void MediaPlayerPrivateGStreamer::updateBufferingStatus(GstBufferingMode mode, double percentage)
>+{
>+    GST_DEBUG_OBJECT(pipeline(), "[Buffering] mode: %s, status: %f%%", enumToString(GST_TYPE_BUFFERING_MODE, mode).get(), percentage);
>+
>+    m_downloadFinished = percentage == 100;
>+    m_buffering = !m_downloadFinished;
>+
>+    switch (mode) {
>+    case GST_BUFFERING_STREAM: {
>+        updateMaxTimeLoaded(percentage);
>+
>+        m_bufferingPercentage = percentage;
>+        if (m_downloadFinished)
>+            updateStates();
>+
>+        break;
>+    }
>+    case GST_BUFFERING_DOWNLOAD: {
>+        updateMaxTimeLoaded(percentage);
>+
>+        // Media is now fully loaded. It will play even if network connection is
>+        // cut. Buffering is done, remove the fill source from the main loop.
>+        if (m_downloadFinished)
>+            m_fillTimer.stop();
> 
>-    if (m_bufferingPercentage == 100)
>         updateStates();
>+        break;
>+    }
>+    default:
>+        GST_DEBUG_OBJECT(pipeline(), "Unhandled buffering mode: %s", enumToString(GST_TYPE_BUFFERING_MODE, mode).get());
>+        break;
>+    }
> }
> 
> #if ENABLE(VIDEO_TRACK) && USE(GSTREAMER_MPEGTS)
>@@ -1585,48 +1637,23 @@ void MediaPlayerPrivateGStreamer::fillTimerFired()
> {
>     GRefPtr<GstQuery> query = adoptGRef(gst_query_new_buffering(GST_FORMAT_PERCENT));
>     double fillStatus = 100.0;
>+    GstBufferingMode mode = GST_BUFFERING_DOWNLOAD;
> 
>-    if (gst_element_query(m_pipeline.get(), query.get())) {
>-        int64_t stop;
>-        GstFormat format;
>-        gst_query_parse_buffering_range(query.get(), &format, nullptr, &stop, nullptr);
>-        ASSERT(format == GST_FORMAT_PERCENT);
>+    if (gst_element_query(m_source.get(), query.get())) {
>+        gst_query_parse_buffering_stats(query.get(), &mode, nullptr, nullptr, nullptr);
> 
>-        if (stop != -1)
>-            fillStatus = 100.0 * stop / GST_FORMAT_PERCENT_MAX;
>+        int percentage;
>+        gst_query_parse_buffering_percent(query.get(), nullptr, &percentage);
>+        fillStatus = percentage;
>     } else if (m_httpResponseTotalSize) {
>         GST_DEBUG_OBJECT(pipeline(), "[Buffering] Query failed, falling back to network read position estimation");
>-        fillStatus = 100.0 * (m_networkReadPosition / m_httpResponseTotalSize);
>+        fillStatus = 100.0 * (static_cast<double>(m_networkReadPosition) / static_cast<double>(m_httpResponseTotalSize));
>     } else {
>         GST_DEBUG_OBJECT(pipeline(), "[Buffering] Unable to determine on-disk buffering status");
>         return;
>     }
> 
>-    GST_DEBUG_OBJECT(pipeline(), "[Buffering] Download buffer filled up to %f%%", fillStatus);
>-
>-    MediaTime mediaDuration = durationMediaTime();
>-
>-    // Update maxTimeLoaded only if the media duration is
>-    // available. Otherwise we can't compute it.
>-    if (mediaDuration) {
>-        if (fillStatus == 100.0)
>-            m_maxTimeLoaded = mediaDuration;
>-        else
>-            m_maxTimeLoaded = MediaTime(fillStatus * static_cast<double>(toGstUnsigned64Time(mediaDuration)) / 100, GST_SECOND);
>-        GST_DEBUG_OBJECT(pipeline(), "[Buffering] Updated maxTimeLoaded: %s", toString(m_maxTimeLoaded).utf8().data());
>-    }
>-
>-    m_downloadFinished = fillStatus == 100.0;
>-    if (!m_downloadFinished) {
>-        updateStates();
>-        return;
>-    }
>-
>-    // Media is now fully loaded. It will play even if network
>-    // connection is cut. Buffering is done, remove the fill source
>-    // from the main loop.
>-    m_fillTimer.stop();
>-    updateStates();
>+    updateBufferingStatus(mode, fillStatus);
> }
> 
> MediaTime MediaPlayerPrivateGStreamer::maxMediaTimeSeekable() const
>@@ -1655,7 +1682,7 @@ MediaTime MediaPlayerPrivateGStreamer::maxTimeLoaded() const
>     MediaTime loaded = m_maxTimeLoaded;
>     if (m_isEndReached)
>         loaded = durationMediaTime();
>-    GST_LOG("maxTimeLoaded: %s", toString(loaded).utf8().data());
>+    GST_LOG_OBJECT(pipeline(), "maxTimeLoaded: %s", toString(loaded).utf8().data());
>     return loaded;
> }
> 
>@@ -1666,8 +1693,9 @@ bool MediaPlayerPrivateGStreamer::didLoadingProgress() const
> 
>     if (WEBKIT_IS_WEB_SRC(m_source.get())) {
>         GST_LOG_OBJECT(pipeline(), "Last network read position: %" G_GUINT64_FORMAT ", current: %" G_GUINT64_FORMAT, m_readPositionAtLastDidLoadingProgress, m_networkReadPosition);
>-        bool didLoadingProgress = m_readPositionAtLastDidLoadingProgress != m_networkReadPosition;
>+        bool didLoadingProgress = m_readPositionAtLastDidLoadingProgress < m_networkReadPosition;
>         m_readPositionAtLastDidLoadingProgress = m_networkReadPosition;
>+        GST_LOG_OBJECT(pipeline(), "didLoadingProgress: %s", boolForPrinting(didLoadingProgress));
>         return didLoadingProgress;
>     }
> 
>@@ -2001,7 +2029,7 @@ void MediaPlayerPrivateGStreamer::updateStates()
> 
>         // Live pipelines go in PAUSED without prerolling.
>         m_isStreaming = true;
>-        setDownloadBuffering();
>+        updateDownloadBufferingFlag();
> 
>         if (m_currentState == GST_STATE_READY)
>             m_readyState = MediaPlayer::HaveNothing;
>@@ -2256,7 +2284,7 @@ MediaPlayer::SupportsType MediaPlayerPrivateGStreamer::supportsType(const MediaE
>     return finalResult;
> }
> 
>-void MediaPlayerPrivateGStreamer::setDownloadBuffering()
>+void MediaPlayerPrivateGStreamer::updateDownloadBufferingFlag()
> {
>     if (!m_pipeline)
>         return;
>@@ -2291,7 +2319,7 @@ void MediaPlayerPrivateGStreamer::setPreload(MediaPlayer::Preload preload)
>         return;
> 
>     m_preload = preload;
>-    setDownloadBuffering();
>+    updateDownloadBufferingFlag();
> 
>     if (m_delayingLoad && m_preload != MediaPlayer::None) {
>         m_delayingLoad = false;
>diff --git a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h
>index 8e257e6230320b93bbacdc826f90f11c325eb45c..511325e49fe33799dd0f550aa56d8e98d06785ad 100644
>--- a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h
>+++ b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h
>@@ -154,8 +154,11 @@ private:
>     bool loadNextLocation();
>     void mediaLocationChanged(GstMessage*);
> 
>-    virtual void setDownloadBuffering();
>+    virtual void updateDownloadBufferingFlag();
>     void processBufferingStats(GstMessage*);
>+    void updateBufferingStatus(GstBufferingMode, double percentage);
>+    void updateMaxTimeLoaded(double percentage);
>+
> #if ENABLE(VIDEO_TRACK)
> #if USE(GSTREAMER_MPEGTS)
>     void processMpegTsSection(GstMpegtsSection*);
>diff --git a/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp b/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp
>index cff19ab942864f054117941c2e5cf731875f8d20..121884a10ad51151f97ac07c550c236e95e6ded9 100644
>--- a/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp
>+++ b/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp
>@@ -237,6 +237,7 @@ static void webkitWebSrcReset(WebKitWebSrc* src)
> {
>     WebKitWebSrcPrivate* priv = WEBKIT_WEB_SRC_GET_PRIVATE(src);
> 
>+    GST_DEBUG_OBJECT(src, "Resetting internal state");
>     priv->haveSize = false;
>     priv->wereHeadersReceived = false;
>     priv->isSeekable = false;
>diff --git a/Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.h b/Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.h
>index 7d74057b9be9deb83d798ff88bb8953bdc6c1e00..3bcc0fdba3883aeec6e2e26585eac8cbbe0d4eae 100644
>--- a/Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.h
>+++ b/Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.h
>@@ -52,7 +52,7 @@ public:
>     void load(const String&) override;
>     void load(const String&, MediaSourcePrivateClient*) override;
> 
>-    void setDownloadBuffering() override { };
>+    void updateDownloadBufferingFlag() override { };
> 
>     bool isLiveStream() const override { return false; }
>     MediaTime currentMediaTime() const override;

Actions: View | Formatted Diff | Diff

Attachments on bug 198376: 373217 | 373236 | 373239 | 373242