Log Message
[GStreamer] Several buffering fixes https://bugs.webkit.org/show_bug.cgi?id=206234
Patch by Thibault Saunier <tsaun...@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.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (254564 => 254565)
--- trunk/Source/WebCore/ChangeLog 2020-01-15 14:24:12 UTC (rev 254564)
+++ trunk/Source/WebCore/ChangeLog 2020-01-15 14:40:01 UTC (rev 254565)
@@ -1,3 +1,28 @@
+2020-01-15 Thibault Saunier <tsaun...@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 <you...@apple.com>
Introduce an abstract SampleBufferDisplayLayer
Modified: trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp (254564 => 254565)
--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp 2020-01-15 14:24:12 UTC (rev 254564)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp 2020-01-15 14:40:01 UTC (rev 254565)
@@ -1053,7 +1053,7 @@
MediaTime previousDuration = durationMediaTime();
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.
if (previousDuration && durationMediaTime() != previousDuration)
@@ -1456,7 +1456,7 @@
double fillStatus = 100.0;
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);
int percentage;
@@ -1470,7 +1470,10 @@
return;
}
- 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);
}
void MediaPlayerPrivateGStreamer::loadStateChanged()
@@ -2198,6 +2201,11 @@
if (const char* uri = gst_structure_get_string(structure, "uri"))
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);
break;
@@ -2292,17 +2300,23 @@
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);
m_didDownloadFinish = percentage == 100;
m_isBuffering = !m_didDownloadFinish;
+ if (!m_didDownloadFinish)
+ m_isBuffering = true;
+
+ m_bufferingPercentage = percentage;
switch (mode) {
case GST_BUFFERING_STREAM: {
updateMaxTimeLoaded(percentage);
m_bufferingPercentage = percentage;
- if (m_didDownloadFinish)
+ if (m_didDownloadFinish || (!wasBuffering && m_isBuffering))
updateStates();
break;
@@ -2309,12 +2323,6 @@
}
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;
}
@@ -2577,9 +2585,17 @@
FALLTHROUGH;
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;
} else {
@@ -2609,7 +2625,7 @@
}
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);
}
} else if (m_currentState == GST_STATE_PLAYING) {
@@ -2616,7 +2632,7 @@
m_isPaused = false;
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);
}
} else
_______________________________________________ webkit-changes mailing list webkit-changes@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-changes
