Changeset 220414 in webkit

Timestamp:

Aug 8, 2017 12:03:57 PM (7 years ago)

Author:

commit-queue@webkit.org

Message:

Change fast seek logic to prevent ping-ponging.
​https://bugs.webkit.org/show_bug.cgi?id=175167
rdar://problem/32936116

Patch by Jeremy Jones <​jeremyj@apple.com> on 2017-08-08
Reviewed by Jer Noble.

Source/WebCore:

Updated test: media/video-fast-seek.html

Changed fast seek behavior to always seek early than seek time; and after current time when seeking forward.
This prevents ping-ponging, where a fast seek overshoots the target looking for a keyframe.
A subsequent seek will suddenly be on the other side of current time and
will search in the opposite direction for a key frame; repeat.

• html/HTMLMediaElement.cpp:

(WebCore::HTMLMediaElement::fastSeek):

LayoutTests:

Test now requires forward fast seek result to be between current time and the target time,
and a backward seek to be before the seek time.

• media/video-fast-seek.html:
• media/media-source/media-source-fastseek.html:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/media/media-source/media-source-fastseek-expected.txt (modified) (1 diff)
• LayoutTests/media/media-source/media-source-fastseek.html (modified) (1 diff)
• LayoutTests/media/video-fast-seek-expected.txt (modified) (1 diff)
• LayoutTests/media/video-fast-seek.html (modified) (3 diffs)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/html/HTMLMediaElement.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2017-08-08  Jeremy Jones  <jeremyj@apple.com>
+
+        Change fast seek logic to prevent ping-ponging.
+        https://bugs.webkit.org/show_bug.cgi?id=175167
+        rdar://problem/32936116
+
+        Reviewed by Jer Noble.
+
+        Test now requires forward fast seek result to be between current time and the target time,
+        and a backward seek to be before the seek time.
+
+        * media/video-fast-seek.html:
+        * media/media-source/media-source-fastseek.html:
+
 2017-08-08  Ms2ger  <Ms2ger@igalia.com>
 

trunk/LayoutTests/media/media-source/media-source-fastseek-expected.txt

@@ -12 +12 @@
 EVENT(seeked)
 EXPECTED (video.currentTime.toFixed(1) == '1') OK
-RUN(video.fastSeek(2))
+RUN(video.fastSeek(5))
 EVENT(seeked)
 EXPECTED (video.currentTime.toFixed(1) == '4') OK

trunk/LayoutTests/media/media-source/media-source-fastseek.html

@@ -60 +60 @@
         testExpected('video.currentTime.toFixed(1)', 1);
         waitForEventOnce('seeked', seeked2);
-        run('video.fastSeek(2)');
+        run('video.fastSeek(5)');
     }
 

trunk/LayoutTests/media/video-fast-seek-expected.txt

@@ -7 +7 @@
 EVENT(timeupdate)
 EXPECTED (video.currentTime.toFixed(1) == '2.5') OK
-Test that fastSeek() past the currentTime will not result in a seek before the currentTime:
+Test that fastSeek() past the currentTime will not result in a seek before the currentTime or past the seek time:
 RUN(video.fastSeek(2.6))
 EVENT(timeupdate)
-EXPECTED (video.currentTime >= '2.6') OK
+EXPECTED (video.currentTime <= '2.6') OK
+EXPECTED (video.currentTime >= '2.5') OK
 Seek before the 4th sync sample:
 RUN(video.currentTime = 2.3)
 EVENT(timeupdate)
 EXPECTED (video.currentTime.toFixed(1) == '2.3') OK
-Test that fastSeek() before the currentTime will not result in a seek past the currentTime:
+Test that fastSeek() before the currentTime will not result in a seek past the seek time:
 RUN(video.fastSeek(2.2))
 EVENT(timeupdate)

trunk/LayoutTests/media/video-fast-seek.html

@@ -29 +29 @@
         {
             testExpected('video.currentTime.toFixed(1)', 2.5);
-            consoleWrite('Test that fastSeek() past the currentTime will not result in a seek before the currentTime:');
+            consoleWrite('Test that fastSeek() past the currentTime will not result in a seek before the currentTime or past the seek time:');
             waitForEventOnce('timeupdate', seek2);
             run('video.fastSeek(2.6)');
@@ -36 +36 @@
         function seek2()
         {
-            testExpected('video.currentTime', 2.6, '>=');
+            testExpected('video.currentTime', 2.6, '<=');
+            testExpected('video.currentTime', 2.5, '>=');
             consoleWrite('Seek before the 4th sync sample:');
             waitForEventOnce('timeupdate', seek3);
@@ -45 +46 @@
         {
             testExpected('video.currentTime.toFixed(1)', 2.3);
-            consoleWrite('Test that fastSeek() before the currentTime will not result in a seek past the currentTime:');
+            consoleWrite('Test that fastSeek() before the currentTime will not result in a seek past the seek time:');
             waitForEventOnce('timeupdate', seek4);
             run('video.fastSeek(2.2)');

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-08-08  Jeremy Jones  <jeremyj@apple.com>
+
+        Change fast seek logic to prevent ping-ponging.
+        https://bugs.webkit.org/show_bug.cgi?id=175167
+        rdar://problem/32936116
+
+        Reviewed by Jer Noble.
+
+        Updated test: media/video-fast-seek.html
+
+        Changed fast seek behavior to always seek early than seek time; and after current time when seeking forward.
+        This prevents ping-ponging, where a fast seek overshoots the target looking for a keyframe.
+        A subsequent seek will suddenly be on the other side of current time and
+        will search in the opposite direction for a key frame; repeat.
+
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::fastSeek):
+
 2017-08-08  Ms2ger  <Ms2ger@igalia.com>
 

trunk/Source/WebCore/html/HTMLMediaElement.cpp

@@ -2628 +2628 @@
     // then the adjusted new playback position must also be after the current playback position.
     refreshCachedTime();
+
     MediaTime delta = time - currentMediaTime();
-    MediaTime negativeTolerance = delta >= MediaTime::zeroTime() ? delta : MediaTime::positiveInfiniteTime();
-    MediaTime positiveTolerance = delta < MediaTime::zeroTime() ? -delta : MediaTime::positiveInfiniteTime();
-
-    seekWithTolerance(time, negativeTolerance, positiveTolerance, true);
+    MediaTime negativeTolerance = delta < MediaTime::zeroTime() ? MediaTime::positiveInfiniteTime() : delta;
+    seekWithTolerance(time, negativeTolerance, MediaTime::zeroTime(), true);
 }