Title: [148840] trunk
Revision
148840
Author
[email protected]
Date
2013-04-21 11:36:58 -0700 (Sun, 21 Apr 2013)

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

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (148839 => 148840)


--- trunk/LayoutTests/ChangeLog	2013-04-21 17:46:21 UTC (rev 148839)
+++ trunk/LayoutTests/ChangeLog	2013-04-21 18:36:58 UTC (rev 148840)
@@ -1,3 +1,13 @@
+2013-04-12  Balazs Kelemen  <[email protected]>
+
+        [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  <[email protected]>
 
         Unreviewed EFL gardening.

Added: trunk/LayoutTests/http/tests/media/video-preload-expected.txt (0 => 148840)


--- trunk/LayoutTests/http/tests/media/video-preload-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/media/video-preload-expected.txt	2013-04-21 18:36:58 UTC (rev 148840)
@@ -0,0 +1,4 @@
+Test for https://bugs.webkit.org/show_bug.cgi?id=114357 [GStreamer] Media attribute preload="none" is not honored
+
+Pass OK
+

Added: trunk/LayoutTests/http/tests/media/video-preload.html (0 => 148840)


--- trunk/LayoutTests/http/tests/media/video-preload.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/media/video-preload.html	2013-04-21 18:36:58 UTC (rev 148840)
@@ -0,0 +1,30 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+    <script src=""
+    <script>
+        function onLoadedMetaData()
+        {
+            logResult(false, "Fail");
+            if (testRunner)
+                testRunner.notifyDone();
+        }
+        function start()
+        {
+            setTimeout(function() {
+                logResult(true, "Pass");
+                if (testRunner)
+                    testRunner.notifyDone();
+            }, 200);
+        }
+    </script>
+</head>
+<body _onload_="start()">
+    <p>Test for https://bugs.webkit.org/show_bug.cgi?id=114357 [GStreamer] Media attribute preload="none" is not honored</p>
+    <!-- This test refers to the video content statically because otherwise the original bug could not have been triggered. -->
+    <video preload="none" _onloadedmetadata_="onLoadedMetaData()">
+        <source src="" type="video/mp4" />
+        <source src="" type="video/ogg" />
+    </video>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (148839 => 148840)


--- trunk/Source/WebCore/ChangeLog	2013-04-21 17:46:21 UTC (rev 148839)
+++ trunk/Source/WebCore/ChangeLog	2013-04-21 18:36:58 UTC (rev 148840)
@@ -1,3 +1,30 @@
+2013-04-12  Balazs Kelemen  <[email protected]>
+
+        [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  <[email protected]>
 
         Widget should not depend on AXObjectCache

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


--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp	2013-04-21 17:46:21 UTC (rev 148839)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp	2013-04-21 18:36:58 UTC (rev 148840)
@@ -207,7 +207,7 @@
     , m_fillTimer(this, &MediaPlayerPrivateGStreamer::fillTimerFired)
     , m_maxTimeLoaded(0)
     , m_bufferingPercentage(0)
-    , m_preload(MediaPlayer::Auto)
+    , m_preload(player->preload())
     , m_delayingLoad(false)
     , m_mediaDurationKnown(true)
     , m_maxTimeLoadedAtLastDidLoadingProgress(0)
@@ -218,7 +218,6 @@
     , m_videoTimerHandler(0)
     , m_webkitAudioSink(0)
     , m_totalBytes(-1)
-    , m_originalPreloadWasAutoAndWasOverridden(false)
     , m_preservesPitch(false)
     , m_requestedState(GST_STATE_VOID_PENDING)
     , m_missingPlugins(false)
@@ -1390,15 +1389,6 @@
     // HTMLMediaElement.
     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);
-        }
-    }
 }
 
 void MediaPlayerPrivateGStreamer::loadingFailed(MediaPlayer::NetworkState error)
@@ -1553,7 +1543,8 @@
 
 void MediaPlayerPrivateGStreamer::setPreload(MediaPlayer::Preload preload)
 {
-    m_originalPreloadWasAutoAndWasOverridden = m_preload != preload && m_preload == MediaPlayer::Auto;
+    if (preload == MediaPlayer::Auto && isLiveStream())
+        return;
 
     m_preload = preload;
 

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


--- trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h	2013-04-21 17:46:21 UTC (rev 148839)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h	2013-04-21 18:36:58 UTC (rev 148840)
@@ -160,7 +160,6 @@
     GRefPtr<GstElement> m_webkitAudioSink;
     mutable long m_totalBytes;
     KURL m_url;
-    bool m_originalPreloadWasAutoAndWasOverridden;
     bool m_preservesPitch;
     GstState m_requestedState;
     GRefPtr<GstElement> m_autoAudioSink;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to