Changeset 254565 in webkit

Timestamp:

Jan 15, 2020, 6:40:01 AM (7 years ago)

Author:

commit-queue@webkit.org

Message:

[GStreamer] Several buffering fixes
​https://bugs.webkit.org/show_bug.cgi?id=206234

Patch by Thibault Saunier <​tsaunier@igalia.com> on 2020-01-15
Reviewed by Xabier Rodriguez-Calvar.

No new tests as this is already tested.

• platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:

(WebCore::MediaPlayerPrivateGStreamer::durationChanged): Minor typo fix
(WebCore::MediaPlayerPrivateGStreamer::fillTimerFired): Query buffering on the pipeline not the source

otherwise GstBaseSrc returns some useless values before downloadbuffer actually gives us the
info about DOWNLOAD buffering status. Also ignores response if they are not in DOWNLOAD mode as those
will end up screwing our buffering management algorithm.

(WebCore::MediaPlayerPrivateGStreamer::handleMessage):

• Detect when DOWNLOAD is done by using the downloadbuffer GstCacheDownloadComplete element message which is what is supposed to be used for that purpose.
• Fix the way we detect that buffering is done (mostly when using a downloadbuffer) by relying on a buffering query to check if it is still buffering.

(WebCore::MediaPlayerPrivateGStreamer::updateBufferingStatus): Ensure that we properly pause the pipeline when

restarting buffering. There were cases when not using downloadbuffer where we didn't pause the pipeline
leading to pretty bad user experience.

(WebCore::MediaPlayerPrivateGStreamer::updateStates): Buffering should happen only on non live pipelines.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp (modified) (11 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-01-15  Thibault Saunier  <tsaunier@igalia.com>
+
+        [GStreamer] Several buffering fixes
+        https://bugs.webkit.org/show_bug.cgi?id=206234
+
+        Reviewed by Xabier Rodriguez-Calvar.
+
+        No new tests as this is already tested.
+
+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
+        (WebCore::MediaPlayerPrivateGStreamer::durationChanged): Minor typo fix
+        (WebCore::MediaPlayerPrivateGStreamer::fillTimerFired): Query buffering on the pipeline not the source
+            otherwise GstBaseSrc returns some useless values before `downloadbuffer` actually gives us the
+            info about DOWNLOAD buffering status. Also ignores response if they are not in DOWNLOAD mode as those
+            will end up screwing our buffering management algorithm.
+        (WebCore::MediaPlayerPrivateGStreamer::handleMessage):
+            - Detect when DOWNLOAD is done by using the `downloadbuffer` `GstCacheDownloadComplete`
+              element message which is what is supposed to be used for that purpose.
+            - Fix the way we detect that buffering is done (mostly when using a `downloadbuffer`) by relying on a
+              buffering query to check if it is still buffering.
+        (WebCore::MediaPlayerPrivateGStreamer::updateBufferingStatus): Ensure that we properly pause the pipeline when
+            restarting buffering. There were cases when not using `downloadbuffer` where we didn't pause the pipeline
+            leading to pretty bad user experience.
+        (WebCore::MediaPlayerPrivateGStreamer::updateStates): Buffering should happen only on **non live** pipelines.
+
 2020-01-15  youenn fablet  <youenn@apple.com>
 

trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp

@@ -1054 +1054 @@
     m_cachedDuration = MediaTime::invalidTime();
 
-    // Avoid emiting durationchanged in the case where the previous
+    // Avoid emitting durationChanged in the case where the previous
     // duration was 0 because that case is already handled by the
     // HTMLMediaElement.
@@ -1457 +1457 @@
     GstBufferingMode mode = GST_BUFFERING_DOWNLOAD;
 
-    if (gst_element_query(m_source.get(), query.get())) {
+    if (gst_element_query(pipeline(), query.get())) {
         gst_query_parse_buffering_stats(query.get(), &mode, nullptr, nullptr, nullptr);
 
@@ -1471 +1471 @@
     }
 
-    updateBufferingStatus(mode, fillStatus);
+    if (mode != GST_BUFFERING_DOWNLOAD)
+        GST_INFO_OBJECT(pipeline(), "Ignoring buffering in %s", enumToString(GST_TYPE_BUFFERING_MODE, mode).data());
+    else
+        updateBufferingStatus(mode, fillStatus);
 }
 
@@ -2199 +2202 @@
                     m_hasTaintedOrigin = webKitSrcWouldTaintOrigin(WEBKIT_WEB_SRC_CAST(m_source.get()), SecurityOrigin::create(URL(URL(), uri)));
             }
+        } else if (gst_structure_has_name(structure, "GstCacheDownloadComplete")) {
+            GST_INFO_OBJECT(pipeline(), "Stream is fully downloaded, stopping monitoring downloading progress.");
+            m_fillTimer.stop();
+            m_bufferingPercentage = 100;
+            updateStates();
         } else
             GST_DEBUG_OBJECT(pipeline(), "Unhandled element message: %" GST_PTR_FORMAT, structure);
@@ -2293 +2301 @@
 void MediaPlayerPrivateGStreamer::updateBufferingStatus(GstBufferingMode mode, double percentage)
 {
+    bool wasBuffering = m_isBuffering;
+
     GST_DEBUG_OBJECT(pipeline(), "[Buffering] mode: %s, status: %f%%", enumToString(GST_TYPE_BUFFERING_MODE, mode).data(), percentage);
 
@@ -2298 +2308 @@
     m_isBuffering = !m_didDownloadFinish;
 
+    if (!m_didDownloadFinish)
+        m_isBuffering = true;
+
+    m_bufferingPercentage = percentage;
     switch (mode) {
     case GST_BUFFERING_STREAM: {
@@ -2303 +2317 @@
 
         m_bufferingPercentage = percentage;
-        if (m_didDownloadFinish)
+        if (m_didDownloadFinish || (!wasBuffering && m_isBuffering))
             updateStates();
 
@@ -2310 +2324 @@
     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_didDownloadFinish)
-            m_fillTimer.stop();
-
         updateStates();
         break;
@@ -2578 +2586 @@
         case GST_STATE_PLAYING:
             if (m_isBuffering) {
-                if (m_bufferingPercentage == 100) {
-                    GST_DEBUG_OBJECT(pipeline(), "[Buffering] Complete.");
-                    m_isBuffering = false;
+                GRefPtr<GstQuery> query = adoptGRef(gst_query_new_buffering(GST_FORMAT_PERCENT));
+
+                m_isBuffering = m_bufferingPercentage == 100;
+                if (gst_element_query(m_pipeline.get(), query.get())) {
+                    gboolean isBuffering = m_isBuffering;
+                    gst_query_parse_buffering_percent(query.get(), &isBuffering, nullptr);
+                    m_isBuffering = isBuffering;
+                }
+
+                if (!m_isBuffering) {
+                    GST_INFO_OBJECT(pipeline(), "[Buffering] Complete.");
                     m_readyState = MediaPlayer::ReadyState::HaveEnoughData;
                     m_networkState = m_didDownloadFinish ? MediaPlayer::NetworkState::Idle : MediaPlayer::NetworkState::Loading;
@@ -2610 +2626 @@
 
             if (didBuffering && !m_isBuffering && !m_isPaused && m_playbackRate) {
-                GST_DEBUG_OBJECT(pipeline(), "[Buffering] Restarting playback.");
+                GST_INFO_OBJECT(pipeline(), "[Buffering] Restarting playback.");
                 changePipelineState(GST_STATE_PLAYING);
             }
@@ -2617 +2633 @@
 
             if ((m_isBuffering && !m_isLiveStream) || !m_playbackRate) {
-                GST_DEBUG_OBJECT(pipeline(), "[Buffering] Pausing stream for buffering.");
+                GST_INFO_OBJECT(pipeline(), "[Buffering] Pausing stream for buffering.");
                 changePipelineState(GST_STATE_PAUSED);
             }