Changeset 148840 in webkit

Timestamp:

Apr 21, 2013 11:36:58 AM (11 years ago)

Author:

kbalazs@webkit.org

Message:

[GStreamer] Media attribute preload="none" is not honored
​https://bugs.webkit.org/show_bug.cgi?id=114357

Reviewed by Philippe Normand.

Source/WebCore:

Fix the logic that prevents live streams from being buffered to not make preload="none" ignored.

Test: http/tests/media/video-preload.html
We need a http test because the bug does not triggered with local files.

• platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:

(WebCore::MediaPlayerPrivateGStreamer::MediaPlayerPrivateGStreamer):

• platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:

(MediaPlayerPrivateGStreamer):
Remove m_originalPreloadWasAutoAndWasOverridden because it is not necessary and it is causing this bug.
Currently if the tag has preload="none" attribute we set m_preload to Auto in the constructor. After that
MedaPlayer calls setPreload(None), so we set m_originalPreloadWasAutoAndWasOverridden to true and later
reset m_preload to Auto. The error prone factor here is that the m_preload member is repeated in the
private class and setPreload is also used there. This seems to be necessary because we need to be able
to ignore preloading if this is a live stream. Fortunately the original parsed value is available in the
constructor, so we can use that. This will give the correct value that we should override only in the case
of a live stream and that's it, we don't need to reset it later to Auto.
Furthermore, we should ignore setting preload to auto from js if it is a live stream. This patch also handles
this with an early return in setPreload.

LayoutTests:

• http/tests/media/video-preload-expected.txt: Added.
• http/tests/media/video-preload.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/http/tests/media/video-preload-expected.txt (added)
• LayoutTests/http/tests/media/video-preload.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp (modified) (4 diffs)
• Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-04-12  Balazs Kelemen  <kbalazs@webkit.org>
+
+        [GStreamer] Media attribute preload="none" is not honored
+        https://bugs.webkit.org/show_bug.cgi?id=114357
+
+        Reviewed by Philippe Normand.
+
+        * http/tests/media/video-preload-expected.txt: Added.
+        * http/tests/media/video-preload.html: Added.
+
 2013-04-21  Christophe Dumez  <ch.dumez@sisa.samsung.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-04-12  Balazs Kelemen  <kbalazs@webkit.org>
+
+        [GStreamer] Media attribute preload="none" is not honored
+        https://bugs.webkit.org/show_bug.cgi?id=114357
+
+        Reviewed by Philippe Normand.
+
+        Fix the logic that prevents live streams from being buffered to not make preload="none" ignored.
+
+        Test: http/tests/media/video-preload.html
+        We need a http test because the bug does not triggered with local files.
+
+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
+        (WebCore::MediaPlayerPrivateGStreamer::MediaPlayerPrivateGStreamer):
+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:
+        (MediaPlayerPrivateGStreamer):
+        Remove m_originalPreloadWasAutoAndWasOverridden because it is not necessary and it is causing this bug.
+        Currently if the tag has preload="none" attribute we set m_preload to Auto in the constructor. After that
+        MedaPlayer calls setPreload(None), so we set m_originalPreloadWasAutoAndWasOverridden to true and later
+        reset m_preload to Auto. The error prone factor here is that the m_preload member is repeated in the
+        private class and setPreload is also used there. This seems to be necessary because we need to be able
+        to ignore preloading if this is a live stream. Fortunately the original parsed value is available in the
+        constructor, so we can use that. This will give the correct value that we should override only in the case
+        of a live stream and that's it, we don't need to reset it later to Auto.
+        Furthermore, we should ignore setting preload to auto from js if it is a live stream. This patch also handles
+        this with an early return in setPreload.
+
 2013-04-21  Carlos Garcia Campos  <cgarcia@igalia.com>
 

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

@@ -208 +208 @@
     , m_maxTimeLoaded(0)
     , m_bufferingPercentage(0)
-    , m_preload(MediaPlayer::Auto)
+    , m_preload(player->preload())
     , m_delayingLoad(false)
     , m_mediaDurationKnown(true)
@@ -219 +219 @@
     , m_webkitAudioSink(0)
     , m_totalBytes(-1)
-    , m_originalPreloadWasAutoAndWasOverridden(false)
     , m_preservesPitch(false)
     , m_requestedState(GST_STATE_VOID_PENDING)
@@ -1391 +1390 @@
     if (previousDuration && m_mediaDuration != previousDuration)
         m_player->durationChanged();
-
-    if (m_preload == MediaPlayer::None && m_originalPreloadWasAutoAndWasOverridden) {
-        m_totalBytes = -1;
-        if (totalBytes() && !isLiveStream()) {
-            setPreload(MediaPlayer::Auto);
-            gst_element_set_state(m_playBin.get(), GST_STATE_NULL);
-            gst_element_set_state(m_playBin.get(), GST_STATE_PAUSED);
-        }
-    }
 }
 
@@ -1554 +1544 @@
 void MediaPlayerPrivateGStreamer::setPreload(MediaPlayer::Preload preload)
 {
-    m_originalPreloadWasAutoAndWasOverridden = m_preload != preload && m_preload == MediaPlayer::Auto;
+    if (preload == MediaPlayer::Auto && isLiveStream())
+        return;
 
     m_preload = preload;

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

@@ -161 +161 @@
     mutable long m_totalBytes;
     KURL m_url;
-    bool m_originalPreloadWasAutoAndWasOverridden;
     bool m_preservesPitch;
     GstState m_requestedState;