Title: [247010] trunk/Source
Revision
247010
Author
ph...@webkit.org
Date
2019-07-01 09:57:03 -0700 (Mon, 01 Jul 2019)

Log Message

[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.

Source/WebCore:

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:

Source/WTF:

* wtf/glib/GLibUtilities.h:
(enumToString): Utility function to get a string representation of of a GLib enum.

Modified Paths

Diff

Modified: trunk/Source/WTF/ChangeLog (247009 => 247010)


--- trunk/Source/WTF/ChangeLog	2019-07-01 15:34:16 UTC (rev 247009)
+++ trunk/Source/WTF/ChangeLog	2019-07-01 16:57:03 UTC (rev 247010)
@@ -1,3 +1,13 @@
+2019-07-01  Philippe Normand  <pnorm...@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  <da...@apple.com>
 
         Streamline some string code, focusing on functions that were using substringSharingImpl

Modified: trunk/Source/WTF/wtf/glib/GLibUtilities.cpp (247009 => 247010)


--- trunk/Source/WTF/wtf/glib/GLibUtilities.cpp	2019-07-01 15:34:16 UTC (rev 247009)
+++ trunk/Source/WTF/wtf/glib/GLibUtilities.cpp	2019-07-01 16:57:03 UTC (rev 247010)
@@ -77,3 +77,18 @@
 
     return g_get_prgname();
 }
+
+CString enumToString(GType type, guint value)
+{
+#if GLIB_CHECK_VERSION(2, 54, 0)
+    GUniquePtr<char> result(g_enum_to_string(type, value));
+    return result.get();
+#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);
+    GUniquePtr<char> result(representation);
+    return result.get();
+#endif
+}

Modified: trunk/Source/WTF/wtf/glib/GLibUtilities.h (247009 => 247010)


--- trunk/Source/WTF/wtf/glib/GLibUtilities.h	2019-07-01 15:34:16 UTC (rev 247009)
+++ trunk/Source/WTF/wtf/glib/GLibUtilities.h	2019-07-01 16:57:03 UTC (rev 247010)
@@ -20,11 +20,13 @@
 #ifndef GLibUtilities_h
 #define GLibUtilities_h
 
+#include <glib-object.h>
 #include <wtf/Assertions.h>
 #include <wtf/text/CString.h>
 
 CString getCurrentExecutablePath();
 CString getCurrentExecutableName();
+CString enumToString(GType, guint value);
 
 // These might be added to glib in the future, but in the meantime they're defined here.
 #ifndef GULONG_TO_POINTER

Modified: trunk/Source/WebCore/ChangeLog (247009 => 247010)


--- trunk/Source/WebCore/ChangeLog	2019-07-01 15:34:16 UTC (rev 247009)
+++ trunk/Source/WebCore/ChangeLog	2019-07-01 16:57:03 UTC (rev 247010)
@@ -1,3 +1,37 @@
+2019-07-01  Philippe Normand  <pnorm...@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  <mago...@igalia.com>
 
         REGRESSION(r246963) GTK's debug build is broken

Modified: trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp (247009 => 247010)


--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp	2019-07-01 15:34:16 UTC (rev 247009)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp	2019-07-01 16:57:03 UTC (rev 247010)
@@ -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 @@
     // start providing anything useful.
     changePipelineState(GST_STATE_PAUSED);
 
-    setDownloadBuffering();
+    updateDownloadBufferingFlag();
     updateStates();
 }
 
@@ -458,7 +458,7 @@
         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 @@
                     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 @@
                             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::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);
 
-    GST_DEBUG_OBJECT(pipeline(), "[Buffering] Buffering: %d%%.", m_bufferingPercentage);
+    int percentage;
+    gst_message_parse_buffering(message, &percentage);
 
-    if (m_bufferingPercentage == 100)
+    updateBufferingStatus(mode, percentage);
+}
+
+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).data(), 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();
+
         updateStates();
+        break;
+    }
+    default:
+        GST_DEBUG_OBJECT(pipeline(), "Unhandled buffering mode: %s", enumToString(GST_TYPE_BUFFERING_MODE, mode).data());
+        break;
+    }
 }
 
 #if ENABLE(VIDEO_TRACK) && USE(GSTREAMER_MPEGTS)
@@ -1585,48 +1637,23 @@
 {
     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 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 @@
 
     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 @@
 
         // 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 @@
     return finalResult;
 }
 
-void MediaPlayerPrivateGStreamer::setDownloadBuffering()
+void MediaPlayerPrivateGStreamer::updateDownloadBufferingFlag()
 {
     if (!m_pipeline)
         return;
@@ -2291,7 +2319,7 @@
         return;
 
     m_preload = preload;
-    setDownloadBuffering();
+    updateDownloadBufferingFlag();
 
     if (m_delayingLoad && m_preload != MediaPlayer::None) {
         m_delayingLoad = false;

Modified: trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h (247009 => 247010)


--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h	2019-07-01 15:34:16 UTC (rev 247009)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h	2019-07-01 16:57:03 UTC (rev 247010)
@@ -154,8 +154,11 @@
     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*);

Modified: trunk/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp (247009 => 247010)


--- trunk/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp	2019-07-01 15:34:16 UTC (rev 247009)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp	2019-07-01 16:57:03 UTC (rev 247010)
@@ -237,6 +237,7 @@
 {
     WebKitWebSrcPrivate* priv = WEBKIT_WEB_SRC_GET_PRIVATE(src);
 
+    GST_DEBUG_OBJECT(src, "Resetting internal state");
     priv->haveSize = false;
     priv->wereHeadersReceived = false;
     priv->isSeekable = false;

Modified: trunk/Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.h (247009 => 247010)


--- trunk/Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.h	2019-07-01 15:34:16 UTC (rev 247009)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.h	2019-07-01 16:57:03 UTC (rev 247010)
@@ -52,7 +52,7 @@
     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;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to