Title: [243489] trunk
Revision
243489
Author
ph...@webkit.org
Date
2019-03-26 02:34:07 -0700 (Tue, 26 Mar 2019)

Log Message

[GStreamer] Sound loop with Google Hangouts and WhatsApp notifications
https://bugs.webkit.org/show_bug.cgi?id=189471

Reviewed by Xabier Rodriguez-Calvar.

Source/WebCore:

The media duration is now cached (again). The loop issue was
triggered by the previous version of the code returning positive
infinite duration in didEnd(), followed by the timeupdate event
propagation that would trick the HTMLMediaElement into a new call
to play(). Now the cached duration is updated to current position
at EOS (for forward playback direction only), so the media element
no longer triggers a new play call for those cases.

* platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
(WebCore::MediaPlayerPrivateGStreamer::MediaPlayerPrivateGStreamer):
(WebCore::MediaPlayerPrivateGStreamer::loadFull):
(WebCore::MediaPlayerPrivateGStreamer::playbackPosition const):
(WebCore::MediaPlayerPrivateGStreamer::platformDuration const):
(WebCore::MediaPlayerPrivateGStreamer::durationMediaTime const):
(WebCore::MediaPlayerPrivateGStreamer::currentMediaTime const):
(WebCore::MediaPlayerPrivateGStreamer::didEnd):
(WebCore::MediaPlayerPrivateGStreamer::durationChanged):
* platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:
* platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp:
(WebCore::MediaPlayerPrivateGStreamerMSE::currentMediaTime const):

LayoutTests:

* platform/gtk/TestExpectations:
* platform/gtk/media/video-playing-and-pause-expected.txt:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (243488 => 243489)


--- trunk/LayoutTests/ChangeLog	2019-03-26 07:12:47 UTC (rev 243488)
+++ trunk/LayoutTests/ChangeLog	2019-03-26 09:34:07 UTC (rev 243489)
@@ -1,3 +1,13 @@
+2019-03-26  Philippe Normand  <pnorm...@igalia.com>
+
+        [GStreamer] Sound loop with Google Hangouts and WhatsApp notifications
+        https://bugs.webkit.org/show_bug.cgi?id=189471
+
+        Reviewed by Xabier Rodriguez-Calvar.
+
+        * platform/gtk/TestExpectations:
+        * platform/gtk/media/video-playing-and-pause-expected.txt:
+
 2019-03-26  Antti Koivisto  <an...@apple.com>
 
         Hit-testing on layers overlapping scrollers should hit-test on text boxes

Modified: trunk/LayoutTests/platform/gtk/TestExpectations (243488 => 243489)


--- trunk/LayoutTests/platform/gtk/TestExpectations	2019-03-26 07:12:47 UTC (rev 243488)
+++ trunk/LayoutTests/platform/gtk/TestExpectations	2019-03-26 09:34:07 UTC (rev 243489)
@@ -1460,14 +1460,8 @@
 
 webkit.org/b/116976 media/video-played-collapse.html [ Failure Pass Crash ]
 
-webkit.org/b/116977 media/event-attributes.html [ Crash Failure Timeout Pass ]
-
 webkit.org/b/103443 fast/parser/parser-yield-timing.html [ Failure Pass ]
 
-webkit.org/b/105191 media/video-playing-and-pause.html [ Failure Pass ]
-
-webkit.org/b/118460 media/media-element-play-after-eos.html [ Timeout Pass ]
-
 webkit.org/b/119009 http/tests/cache/subresource-failover-to-network.html [ Failure Pass ]
 
 webkit.org/b/119040 perf/nested-combined-selectors.html [ Failure Pass ]
@@ -1678,8 +1672,6 @@
 
 webkit.org/b/134981 fast/parser/document-write-during-load.html [ Timeout Pass Crash ]
 
-webkit.org/b/81604 media/video-loop.html [ Timeout Pass ]
-
 webkit.org/b/134998 plugins/change-widget-and-click-crash.html [ Timeout Pass Crash ]
 
 webkit.org/b/135003 jquery/attributes.html [ Timeout Pass ]
@@ -3817,6 +3809,8 @@
 
 webkit.org/b/196201 fast/text/ja-sans-serif.html [ ImageOnlyFailure ]
 
+webkit.org/b/116977 media/event-attributes.html [ Failure ]
+
 #////////////////////////////////////////////////////////////////////////////////////////
 # End of non-crashing, non-flaky tests failing
 #////////////////////////////////////////////////////////////////////////////////////////

Modified: trunk/LayoutTests/platform/gtk/media/video-playing-and-pause-expected.txt (243488 => 243489)


--- trunk/LayoutTests/platform/gtk/media/video-playing-and-pause-expected.txt	2019-03-26 07:12:47 UTC (rev 243488)
+++ trunk/LayoutTests/platform/gtk/media/video-playing-and-pause-expected.txt	2019-03-26 09:34:07 UTC (rev 243489)
@@ -1,28 +1,29 @@
 layer at (0,0) size 800x600
   RenderView at (0,0) size 800x600
-layer at (0,0) size 800x317
-  RenderBlock {HTML} at (0,0) size 800x317
-    RenderBody {BODY} at (8,16) size 784x293
-      RenderBlock {P} at (0,0) size 784x34
-        RenderText {#text} at (0,0) size 766x34
-          text run at (0,0) width 766: "Test that pausing the media element in \"playing\" event handler pauses the media immediately. The video should show the"
-          text run at (0,17) width 68: "first frame."
-      RenderBlock (anonymous) at (0,50) size 784x243
+layer at (0,0) size 800x320
+  RenderBlock {HTML} at (0,0) size 800x320
+    RenderBody {BODY} at (8,16) size 784x296
+      RenderBlock {P} at (0,0) size 784x36
+        RenderText {#text} at (0,0) size 764x35
+          text run at (0,0) width 764: "Test that pausing the media element in \"playing\" event handler pauses the media immediately. The video should show the"
+          text run at (0,18) width 68: "first frame."
+      RenderBlock (anonymous) at (0,52) size 784x244
         RenderText {#text} at (0,0) size 0x0
-layer at (8,66) size 320x240
+layer at (8,68) size 320x240
   RenderVideo {VIDEO} at (0,0) size 320x240
-layer at (8,66) size 320x240
+layer at (8,68) size 320x240
   RenderFlexibleBox {DIV} at (0,0) size 320x240
     RenderBlock {DIV} at (0,200) size 320x40
-layer at (13,271) size 310x30
+layer at (13,273) size 310x30
   RenderFlexibleBox {DIV} at (5,5) size 310x30 [bgcolor=#141414CC]
-    RenderButton {INPUT} at (9,0) size 30x30
+    RenderButton {BUTTON} at (9,0) size 30x30
     RenderSlider {INPUT} at (49,11) size 93x8 [color=#E6E6E659]
       RenderFlexibleBox {DIV} at (0,0) size 93x8 [border: (1px solid #E6E6E659)]
         RenderBlock {DIV} at (1,-2) size 105x12
-          RenderBlock {DIV} at (-7,0) size 13x12 [color=#FFFFFF]
+          RenderBlock {DIV} at (-7,0) size 12x12 [color=#FFFFFF]
     RenderBlock {DIV} at (157,0) size 74x30 [color=#FFFFFF]
       RenderText {#text} at (0,7) size 74x15
         text run at (0,7) width 74: "00:00 / 00:06"
-    RenderButton {INPUT} at (239,0) size 30x30
-    RenderButton {INPUT} at (271,0) size 30x30
+    RenderButton {BUTTON} at (239,0) size 30x30
+    RenderFlexibleBox {DIV} at (271,0) size 30x30
+      RenderButton {BUTTON} at (0,0) size 30x30

Modified: trunk/Source/WebCore/ChangeLog (243488 => 243489)


--- trunk/Source/WebCore/ChangeLog	2019-03-26 07:12:47 UTC (rev 243488)
+++ trunk/Source/WebCore/ChangeLog	2019-03-26 09:34:07 UTC (rev 243489)
@@ -1,3 +1,31 @@
+2019-03-26  Philippe Normand  <pnorm...@igalia.com>
+
+        [GStreamer] Sound loop with Google Hangouts and WhatsApp notifications
+        https://bugs.webkit.org/show_bug.cgi?id=189471
+
+        Reviewed by Xabier Rodriguez-Calvar.
+
+        The media duration is now cached (again). The loop issue was
+        triggered by the previous version of the code returning positive
+        infinite duration in didEnd(), followed by the timeupdate event
+        propagation that would trick the HTMLMediaElement into a new call
+        to play(). Now the cached duration is updated to current position
+        at EOS (for forward playback direction only), so the media element
+        no longer triggers a new play call for those cases.
+
+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
+        (WebCore::MediaPlayerPrivateGStreamer::MediaPlayerPrivateGStreamer):
+        (WebCore::MediaPlayerPrivateGStreamer::loadFull):
+        (WebCore::MediaPlayerPrivateGStreamer::playbackPosition const):
+        (WebCore::MediaPlayerPrivateGStreamer::platformDuration const):
+        (WebCore::MediaPlayerPrivateGStreamer::durationMediaTime const):
+        (WebCore::MediaPlayerPrivateGStreamer::currentMediaTime const):
+        (WebCore::MediaPlayerPrivateGStreamer::didEnd):
+        (WebCore::MediaPlayerPrivateGStreamer::durationChanged):
+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:
+        * platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp:
+        (WebCore::MediaPlayerPrivateGStreamerMSE::currentMediaTime const):
+
 2019-03-26  Antti Koivisto  <an...@apple.com>
 
         Hit-testing on layers overlapping scrollers should hit-test on text boxes

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


--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp	2019-03-26 07:12:47 UTC (rev 243488)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp	2019-03-26 09:34:07 UTC (rev 243489)
@@ -136,6 +136,7 @@
     , m_buffering(false)
     , m_bufferingPercentage(0)
     , m_cachedPosition(MediaTime::invalidTime())
+    , m_cachedDuration(MediaTime::invalidTime())
     , m_canFallBackToLastFinishedSeekPosition(false)
     , m_changingRate(false)
     , m_downloadFinished(false)
@@ -142,7 +143,6 @@
     , m_errorOccured(false)
     , m_isEndReached(false)
     , m_isStreaming(false)
-    , m_durationAtEOS(MediaTime::invalidTime())
     , m_paused(true)
     , m_playbackRate(1)
     , m_requestedState(GST_STATE_VOID_PENDING)
@@ -302,7 +302,6 @@
     m_readyState = MediaPlayer::HaveNothing;
     m_player->readyStateChanged();
     m_volumeAndMuteInitialized = false;
-    m_durationAtEOS = MediaTime::invalidTime();
 
     if (!m_delayingLoad)
         commitLoad();
@@ -357,6 +356,7 @@
 
 MediaTime MediaPlayerPrivateGStreamer::playbackPosition() const
 {
+    GST_TRACE_OBJECT(pipeline(), "isEndReached: %s, seeking: %s, seekTime: %s", boolForPrinting(m_isEndReached), boolForPrinting(m_seeking), m_seekTime.toString().utf8().data());
     if (m_isEndReached && m_seeking)
         return m_seekTime;
 
@@ -363,8 +363,10 @@
     // This constant should remain lower than HTMLMediaElement's maxTimeupdateEventFrequency.
     static const Seconds positionCacheThreshold = 200_ms;
     Seconds now = WTF::WallTime::now().secondsSinceEpoch();
-    if (m_lastQueryTime && (now - m_lastQueryTime.value()) < positionCacheThreshold && m_cachedPosition.isValid())
+    if (m_lastQueryTime && (now - m_lastQueryTime.value()) < positionCacheThreshold && m_cachedPosition.isValid()) {
+        GST_TRACE_OBJECT(pipeline(), "Returning cached position: %s", m_cachedPosition.toString().utf8().data());
         return m_cachedPosition;
+    }
 
     m_lastQueryTime = now;
 
@@ -375,7 +377,7 @@
         gst_query_parse_position(query, 0, &position);
     gst_query_unref(query);
 
-    GST_TRACE_OBJECT(pipeline(), "Position %" GST_TIME_FORMAT, GST_TIME_ARGS(position));
+    GST_TRACE_OBJECT(pipeline(), "Position %" GST_TIME_FORMAT ", canFallBackToLastFinishedSeekPosition: %s", GST_TIME_ARGS(position), boolForPrinting(m_canFallBackToLastFinishedSeekPosition));
 
     MediaTime playbackPosition = MediaTime::zeroTime();
     GstClockTime gstreamerPosition = static_cast<GstClockTime>(position);
@@ -477,29 +479,39 @@
         loadingFailed(MediaPlayer::Empty);
 }
 
-MediaTime MediaPlayerPrivateGStreamer::durationMediaTime() const
+MediaTime MediaPlayerPrivateGStreamer::platformDuration() const
 {
-    if (!m_pipeline || m_errorOccured)
+    GST_TRACE_OBJECT(pipeline(), "errorOccured: %s, pipeline state: %s", boolForPrinting(m_errorOccured), gst_element_state_get_name(GST_STATE(m_pipeline.get())));
+    if (m_errorOccured)
         return MediaTime::invalidTime();
 
-    if (m_durationAtEOS.isValid())
-        return m_durationAtEOS;
-
     // The duration query would fail on a not-prerolled pipeline.
     if (GST_STATE(m_pipeline.get()) < GST_STATE_PAUSED)
-        return MediaTime::positiveInfiniteTime();
+        return MediaTime::invalidTime();
 
-    gint64 timeLength = 0;
-
-    if (!gst_element_query_duration(m_pipeline.get(), GST_FORMAT_TIME, &timeLength) || !GST_CLOCK_TIME_IS_VALID(timeLength)) {
+    int64_t duration = 0;
+    if (!gst_element_query_duration(m_pipeline.get(), GST_FORMAT_TIME, &duration) || !GST_CLOCK_TIME_IS_VALID(duration)) {
         GST_DEBUG_OBJECT(pipeline(), "Time duration query failed for %s", m_url.string().utf8().data());
         return MediaTime::positiveInfiniteTime();
     }
 
-    GST_LOG("Duration: %" GST_TIME_FORMAT, GST_TIME_ARGS(timeLength));
+    GST_LOG_OBJECT(pipeline(), "Duration: %" GST_TIME_FORMAT, GST_TIME_ARGS(duration));
+    return MediaTime(duration, GST_SECOND);
+}
 
-    return MediaTime(timeLength, GST_SECOND);
-    // FIXME: handle 3.14.9.5 properly
+MediaTime MediaPlayerPrivateGStreamer::durationMediaTime() const
+{
+    GST_TRACE_OBJECT(pipeline(), "Cached duration: %s", m_cachedDuration.toString().utf8().data());
+    if (m_cachedDuration.isValid())
+        return m_cachedDuration;
+
+    MediaTime duration = platformDuration();
+    if (!duration || duration.isInvalid())
+        return MediaTime::zeroTime();
+
+    m_cachedDuration = duration;
+
+    return m_cachedDuration;
 }
 
 MediaTime MediaPlayerPrivateGStreamer::currentMediaTime() const
@@ -507,6 +519,7 @@
     if (!m_pipeline || m_errorOccured)
         return MediaTime::invalidTime();
 
+    GST_TRACE_OBJECT(pipeline(), "seeking: %s, seekTime: %s", boolForPrinting(m_seeking), m_seekTime.toString().utf8().data());
     if (m_seeking)
         return m_seekTime;
 
@@ -2200,27 +2213,26 @@
     // position is not always reported as 0 for instance.
     m_cachedPosition = MediaTime::invalidTime();
     MediaTime now = currentMediaTime();
-    if (now > MediaTime { } && now <= durationMediaTime())
+    if (now > MediaTime::zeroTime() && !m_seeking) {
+        m_cachedDuration = now;
         m_player->durationChanged();
+    }
 
     m_isEndReached = true;
-    timeChanged();
 
     if (!m_player->client().mediaPlayerIsLooping()) {
         m_paused = true;
-        m_durationAtEOS = durationMediaTime();
         changePipelineState(GST_STATE_READY);
         m_downloadFinished = false;
     }
+    timeChanged();
 }
 
 void MediaPlayerPrivateGStreamer::durationChanged()
 {
     MediaTime previousDuration = durationMediaTime();
+    m_cachedDuration = MediaTime::invalidTime();
 
-    // FIXME: Check if this method is still useful, because it's not doing its work at all
-    // since bug #159458 removed a cacheDuration() call here.
-
     // Avoid emiting durationchanged in the case where the previous
     // duration was 0 because that case is already handled by the
     // HTMLMediaElement.

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


--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h	2019-03-26 07:12:47 UTC (rev 243488)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h	2019-03-26 09:34:07 UTC (rev 243489)
@@ -91,6 +91,7 @@
     bool paused() const override;
     bool seeking() const override;
 
+    MediaTime platformDuration() const;
     MediaTime durationMediaTime() const override;
     MediaTime currentMediaTime() const override;
     void seek(const MediaTime&) override;
@@ -187,11 +188,10 @@
 #endif
 
 protected:
-    void cacheDuration();
-
     bool m_buffering;
     int m_bufferingPercentage;
     mutable MediaTime m_cachedPosition;
+    mutable MediaTime m_cachedDuration;
     bool m_canFallBackToLastFinishedSeekPosition;
     bool m_changingRate;
     bool m_downloadFinished;
@@ -198,7 +198,6 @@
     bool m_errorOccured;
     mutable bool m_isEndReached;
     mutable bool m_isStreaming;
-    mutable MediaTime m_durationAtEOS;
     bool m_paused;
     float m_playbackRate;
     GstState m_currentState;

Modified: trunk/Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp (243488 => 243489)


--- trunk/Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp	2019-03-26 07:12:47 UTC (rev 243488)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp	2019-03-26 09:34:07 UTC (rev 243489)
@@ -782,7 +782,6 @@
         m_eosPending = false;
         m_isEndReached = true;
         m_cachedPosition = m_mediaTimeDuration;
-        m_durationAtEOS = m_mediaTimeDuration;
         m_player->timeChanged();
     }
     return position;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to