Diff
Modified: trunk/LayoutTests/ChangeLog (188389 => 188390)
--- trunk/LayoutTests/ChangeLog 2015-08-13 19:08:42 UTC (rev 188389)
+++ trunk/LayoutTests/ChangeLog 2015-08-13 19:16:09 UTC (rev 188390)
@@ -1,3 +1,21 @@
+2015-08-13 Eric Carlson <eric.carl...@apple.com>
+
+ Don't short circuit seeking
+ https://bugs.webkit.org/show_bug.cgi?id=147892
+
+ Reviewed by Jer Noble.
+
+ * media/event-attributes-expected.txt: Update for test change.
+ * media/event-attributes.html: There is no reason to expect that a 'timeupdate' will have
+ been sent before 'canplaythrough'.
+ * media/video-seek-to-current-time-expected.txt: Added.
+ * media/video-seek-to-current-time.html: Added.
+ * platform/efl/TestExpectations: Skip new test.
+ * platform/gtk/TestExpectations: Ditto.
+ * platform/mac/TestExpectations: Mark the new test as sometimes failing because of
+ webkit.org/b/147944.
+ * platform/win/TestExpectations: Skip new test.
+
2015-08-13 Alexey Proskuryakov <a...@apple.com>
[Cocoa] [CJK-configured device] System font has vertical punctuation
Modified: trunk/LayoutTests/media/event-attributes-expected.txt (188389 => 188390)
--- trunk/LayoutTests/media/event-attributes-expected.txt 2015-08-13 19:08:42 UTC (rev 188389)
+++ trunk/LayoutTests/media/event-attributes-expected.txt 2015-08-13 19:16:09 UTC (rev 188390)
@@ -5,7 +5,6 @@
EVENT(loadeddata)
EVENT(canplay)
EVENT(canplaythrough)
-EXPECTED (progressEventCount >= '1') OK
*** starting playback
RUN(video.play())
Modified: trunk/LayoutTests/media/event-attributes.html (188389 => 188390)
--- trunk/LayoutTests/media/event-attributes.html 2015-08-13 19:08:42 UTC (rev 188389)
+++ trunk/LayoutTests/media/event-attributes.html 2015-08-13 19:16:09 UTC (rev 188390)
@@ -18,7 +18,6 @@
switch (event.type)
{
case "canplaythrough":
- testExpected('progressEventCount', 1, '>=');
consoleWrite("<br>*** starting playback");
run("video.play()");
break;
Added: trunk/LayoutTests/media/video-seek-to-current-time-expected.txt (0 => 188390)
--- trunk/LayoutTests/media/video-seek-to-current-time-expected.txt (rev 0)
+++ trunk/LayoutTests/media/video-seek-to-current-time-expected.txt 2015-08-13 19:16:09 UTC (rev 188390)
@@ -0,0 +1,20 @@
+
+Test that setting currentTime immediately after fastSeek() works correctly.
+
+EVENT(canplaythrough)
+
+Seek to a specific time:
+RUN(video.currentTime = 2.5)
+EVENT(seeked)
+EXPECTED (video.currentTime == '2.5') OK
+
+Set currentTime shortly after calling fastSeek():
+RUN(video.fastSeek(4.6))
+RUN(setTimeout(function() { video.currentTime = 2.5; }, 10))
+
+EVENT(seeked)
+EVENT(seeked)
+EXPECTED (video.currentTime == '2.5') OK
+
+END OF TEST
+
Added: trunk/LayoutTests/media/video-seek-to-current-time.html (0 => 188390)
--- trunk/LayoutTests/media/video-seek-to-current-time.html (rev 0)
+++ trunk/LayoutTests/media/video-seek-to-current-time.html 2015-08-13 19:16:09 UTC (rev 188390)
@@ -0,0 +1,63 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <script src=""
+ <script src=""
+ <script>
+
+ // The test.mp4 file has sync samples at the following presentation time stamps:
+ // 0.0000, 0.7968, 1.5936, 2.3904, 3.1872, 3.9840, 4.7808, 5.5776
+ seekedCount = 0;
+
+ function setup()
+ {
+ findMediaElement();
+ waitForEvent('canplaythrough', canplaythrough);
+
+ // Other media files may have sync samples at completely different points, so
+ // explicitly use the .mp4 here.
+ video.src = ""
+ }
+
+ function canplaythrough()
+ {
+ consoleWrite('<br><em>Seek to a specific time:</em>');
+ run('video.currentTime = 2.5');
+ waitForEventOnce('seeked', prepareTest);
+ }
+
+ function prepareTest()
+ {
+ testExpected('video.currentTime', 2.5);
+
+ consoleWrite('<br><em>Set currentTime shortly after calling fastSeek():</em>');
+ run('video.fastSeek(4.6)');
+
+ run('setTimeout(function() { video.currentTime = 2.5; }, 10)');
+ waitForEvent('seeked', runTest);
+ seekedCount = 0;
+ consoleWrite('');
+
+ // The second 'seeked' event is occasionally not sent, so fail the test it we don't
+ // get it in 1.5 seconds instead of waiting for the full timeout.
+ // https://bugs.webkit.org/show_bug.cgi?id=147944
+ failTestIn(1500);
+ }
+
+ function runTest()
+ {
+ if (++seekedCount == 1)
+ return;
+
+ testExpected('video.currentTime', 2.5);
+ consoleWrite('');
+ endTest();
+ }
+
+ </script>
+</head>
+<body _onload_="setup()">
+ <video controls></video>
+ <p>Test that setting currentTime immediately after fastSeek() works correctly.</p>
+</body>
+</html>
Modified: trunk/LayoutTests/platform/efl/TestExpectations (188389 => 188390)
--- trunk/LayoutTests/platform/efl/TestExpectations 2015-08-13 19:08:42 UTC (rev 188389)
+++ trunk/LayoutTests/platform/efl/TestExpectations 2015-08-13 19:16:09 UTC (rev 188390)
@@ -2256,3 +2256,6 @@
# This test relies on iOS-specific font fallback.
fast/text/arabic-glyph-cache-fill-combine.html [ ImageOnlyFailure ]
+
+# This test uses an MPEG-4 video
+media/video-seek-to-current-time.html [ Skip ]
Modified: trunk/LayoutTests/platform/gtk/TestExpectations (188389 => 188390)
--- trunk/LayoutTests/platform/gtk/TestExpectations 2015-08-13 19:08:42 UTC (rev 188389)
+++ trunk/LayoutTests/platform/gtk/TestExpectations 2015-08-13 19:16:09 UTC (rev 188390)
@@ -2423,3 +2423,6 @@
# This test relies on iOS-specific font fallback.
fast/text/arabic-glyph-cache-fill-combine.html [ ImageOnlyFailure ]
+
+# This test uses an MPEG-4 video
+media/video-seek-to-current-time.html [ Skip ]
Modified: trunk/LayoutTests/platform/mac/TestExpectations (188389 => 188390)
--- trunk/LayoutTests/platform/mac/TestExpectations 2015-08-13 19:08:42 UTC (rev 188389)
+++ trunk/LayoutTests/platform/mac/TestExpectations 2015-08-13 19:16:09 UTC (rev 188390)
@@ -989,6 +989,7 @@
webkit.org/b/141084 [ Yosemite ] http/tests/media/video-preload.html [ Pass Timeout ]
webkit.org/b/141294 compositing/reflections/masked-reflection-on-composited.html [ Pass Crash ]
webkit.org/b/142152 media/track/track-in-band-cues-added-once.html [ Pass Failure ]
+webkit.org/b/147944 media/video-seek-to-current-time.html [ Pass Failure ]
## --- End flaky media tests
# Skipped while Eric Carlson works on a fix.
Modified: trunk/LayoutTests/platform/win/TestExpectations (188389 => 188390)
--- trunk/LayoutTests/platform/win/TestExpectations 2015-08-13 19:08:42 UTC (rev 188389)
+++ trunk/LayoutTests/platform/win/TestExpectations 2015-08-13 19:16:09 UTC (rev 188390)
@@ -3170,3 +3170,6 @@
fast/forms/indeterminate-progress-inline-height.html [ Failure ]
fast/forms/listbox-scrollbar-hit-test.html [ Failure ]
+
+# This test uses an MPEG-4 video
+media/video-seek-to-current-time.html [ Skip ]
Modified: trunk/Source/WebCore/ChangeLog (188389 => 188390)
--- trunk/Source/WebCore/ChangeLog 2015-08-13 19:08:42 UTC (rev 188389)
+++ trunk/Source/WebCore/ChangeLog 2015-08-13 19:16:09 UTC (rev 188390)
@@ -1,3 +1,36 @@
+2015-08-13 Eric Carlson <eric.carl...@apple.com>
+
+ Don't short circuit seeking
+ https://bugs.webkit.org/show_bug.cgi?id=147892
+
+ Reviewed by Jer Noble.
+
+ Test: media/video-seek-to-current-time.html
+
+ * html/HTMLMediaElement.cpp:
+ (WebCore::HTMLMediaElement::prepareForLoad): Call clearSeeking.
+ (WebCore::HTMLMediaElement::fastSeek): Add logging.
+ (WebCore::HTMLMediaElement::seekWithTolerance): Add logging. Set m_pendingSeekType.
+ (WebCore::HTMLMediaElement::seekTask): Call clearSeeking. Don't short circuit a
+ if the current or pending seek is a fast seek. Set m_seeking to true immediately
+ before calling media engine as it may have been cleared before the seek task
+ queue ran.
+ (WebCore::HTMLMediaElement::clearSeeking): New.
+ * html/HTMLMediaElement.h:
+ * html/HTMLMediaElementEnums.h:
+
+ * platform/GenericTaskQueue.h:
+ (WebCore::GenericTaskQueue::enqueueTask): Clear m_pendingTasks.
+
+ * platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:
+ (WebCore::MediaPlayerPrivateAVFoundation::seekWithTolerance): Don't return early
+ when asked to seek to the current time.
+ (WebCore::MediaPlayerPrivateAVFoundation::invalidateCachedDuration): Remove some
+ extremely noisy logging.
+
+ * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
+ (WebCore::MediaPlayerPrivateAVFoundationObjC::seekToTime): Add logging.
+
2015-08-13 Simon Fraser <simon.fra...@apple.com>
FilterOperation.h should not include FilterEffect.h
Modified: trunk/Source/WebCore/html/HTMLMediaElement.cpp (188389 => 188390)
--- trunk/Source/WebCore/html/HTMLMediaElement.cpp 2015-08-13 19:08:42 UTC (rev 188389)
+++ trunk/Source/WebCore/html/HTMLMediaElement.cpp 2015-08-13 19:16:09 UTC (rev 188390)
@@ -1017,7 +1017,7 @@
m_paused = true;
// 4.6 - If seeking is true, set it to false.
- m_seeking = false;
+ clearSeeking();
// 4.7 - Set the current playback position to 0.
// Set the official playback position to 0.
@@ -2493,8 +2493,10 @@
// already running. Abort that other instance of the algorithm without waiting for the step that
// it is running to complete.
if (m_seekTaskQueue.hasPendingTasks()) {
+ LOG(Media, "HTMLMediaElement::seekWithTolerance(%p) - cancelling pending seeks", this);
m_seekTaskQueue.cancelAllTasks();
m_pendingSeek = nullptr;
+ m_pendingSeekType = NoSeek;
}
// 4 - Set the seeking IDL attribute to true.
@@ -2509,16 +2511,19 @@
// 5 - If the seek was in response to a DOM method call or setting of an IDL attribute, then continue
// the script. The remainder of these steps must be run asynchronously.
m_pendingSeek = std::make_unique<PendingSeek>(now, time, negativeTolerance, positiveTolerance);
- if (fromDOM)
+ if (fromDOM) {
+ LOG(Media, "HTMLMediaElement::seekWithTolerance(%p) - enqueuing seek from %s to %s", this, toString(now).utf8().data(), toString(time).utf8().data());
m_seekTaskQueue.enqueueTask(std::bind(&HTMLMediaElement::seekTask, this));
- else
+ } else
seekTask();
}
void HTMLMediaElement::seekTask()
{
+ LOG(Media, "HTMLMediaElement::seekTask(%p)", this);
+
if (!m_player) {
- m_seeking = false;
+ clearSeeking();
return;
}
@@ -2545,7 +2550,7 @@
#if !LOG_DISABLED
MediaTime mediaTime = m_player->mediaTimeForTimeValue(time);
if (time != mediaTime)
- LOG(Media, "HTMLMediaElement::seekTimerFired(%p) - %s - media timeline equivalent is %s", this, toString(time).utf8().data(), toString(mediaTime).utf8().data());
+ LOG(Media, "HTMLMediaElement::seekTask(%p) - %s - media timeline equivalent is %s", this, toString(time).utf8().data(), toString(mediaTime).utf8().data());
#endif
time = m_player->mediaTimeForTimeValue(time);
@@ -2554,11 +2559,14 @@
// that is the nearest to the new playback position. ... If there are no ranges given in the seekable
// attribute then set the seeking IDL attribute to false and abort these steps.
RefPtr<TimeRanges> seekableRanges = seekable();
+ bool noSeekRequired = !seekableRanges->length();
// Short circuit seeking to the current time by just firing the events if no seek is required.
- // Don't skip calling the media engine if we are in poster mode because a seek should always
- // cancel poster display.
- bool noSeekRequired = !seekableRanges->length() || (time == now && displayMode() != Poster);
+ // Don't skip calling the media engine if 1) we are in poster mode (because a seek should always cancel
+ // poster display), or 2) if there is a pending fast seek, or 3) if this seek is not an exact seek
+ SeekType thisSeekType = (negativeTolerance == MediaTime::zeroTime() && positiveTolerance == MediaTime::zeroTime()) ? Precise : Fast;
+ if (!noSeekRequired && time == now && thisSeekType == Precise && m_pendingSeekType != Fast && displayMode() != Poster)
+ noSeekRequired = true;
#if ENABLE(MEDIA_SOURCE)
// Always notify the media engine of a seek if the source is not closed. This ensures that the source is
@@ -2568,18 +2576,21 @@
#endif
if (noSeekRequired) {
+ LOG(Media, "HTMLMediaElement::seekTask(%p) - seek to %s ignored", this, toString(time).utf8().data());
if (time == now) {
scheduleEvent(eventNames().seekingEvent);
scheduleTimeupdateEvent(false);
scheduleEvent(eventNames().seekedEvent);
}
- m_seeking = false;
+ clearSeeking();
return;
}
time = seekableRanges->ranges().nearest(time);
m_sentEndEvent = false;
m_lastSeekTime = time;
+ m_pendingSeekType = thisSeekType;
+ m_seeking = true;
// 10 - Queue a task to fire a simple event named seeking at the element.
scheduleEvent(eventNames().seekingEvent);
@@ -2592,14 +2603,21 @@
// 13 - Await a stable state. The synchronous section consists of all the remaining steps of this algorithm.
}
+void HTMLMediaElement::clearSeeking()
+{
+ m_seeking = false;
+ m_pendingSeekType = NoSeek;
+ invalidateCachedTime();
+}
+
void HTMLMediaElement::finishSeek()
{
- LOG(Media, "HTMLMediaElement::finishSeek(%p)", this);
-
// 4.8.10.9 Seeking
// 14 - Set the seeking IDL attribute to false.
- m_seeking = false;
+ clearSeeking();
+ LOG(Media, "HTMLMediaElement::finishSeek(%p) - current time = %s", this, toString(currentMediaTime()).utf8().data());
+
// 15 - Run the time maches on steps.
// Handled by mediaPlayerTimeChanged().
Modified: trunk/Source/WebCore/html/HTMLMediaElement.h (188389 => 188390)
--- trunk/Source/WebCore/html/HTMLMediaElement.h 2015-08-13 19:08:42 UTC (rev 188389)
+++ trunk/Source/WebCore/html/HTMLMediaElement.h 2015-08-13 19:16:09 UTC (rev 188390)
@@ -635,7 +635,7 @@
void seekInternal(const MediaTime&);
void seekWithTolerance(const MediaTime&, const MediaTime& negativeTolerance, const MediaTime& positiveTolerance, bool fromDOM);
void finishSeek();
- void checkIfSeekNeeded();
+ void clearSeeking();
void addPlayedRange(const MediaTime& start, const MediaTime& end);
void scheduleTimeupdateEvent(bool periodicEvent);
@@ -797,6 +797,7 @@
MediaTime positiveTolerance;
};
std::unique_ptr<PendingSeek> m_pendingSeek;
+ SeekType m_pendingSeekType { NoSeek };
double m_volume;
bool m_volumeInitialized;
Modified: trunk/Source/WebCore/html/HTMLMediaElementEnums.h (188389 => 188390)
--- trunk/Source/WebCore/html/HTMLMediaElementEnums.h 2015-08-13 19:08:42 UTC (rev 188389)
+++ trunk/Source/WebCore/html/HTMLMediaElementEnums.h 2015-08-13 19:16:09 UTC (rev 188390)
@@ -49,6 +49,12 @@
enum NetworkState { NETWORK_EMPTY, NETWORK_IDLE, NETWORK_LOADING, NETWORK_NO_SOURCE };
enum TextTrackVisibilityCheckType { CheckTextTrackVisibility, AssumeTextTrackVisibilityChanged };
enum InvalidURLAction { DoNothing, Complain };
+
+ typedef enum {
+ NoSeek,
+ Fast,
+ Precise
+ } SeekType;
};
}
Modified: trunk/Source/WebCore/platform/GenericTaskQueue.h (188389 => 188390)
--- trunk/Source/WebCore/platform/GenericTaskQueue.h 2015-08-13 19:08:42 UTC (rev 188389)
+++ trunk/Source/WebCore/platform/GenericTaskQueue.h 2015-08-13 19:16:09 UTC (rev 188390)
@@ -102,6 +102,8 @@
m_dispatcher.postTask([weakThis, task] {
if (!weakThis)
return;
+ ASSERT(weakThis->m_pendingTasks);
+ --weakThis->m_pendingTasks;
task();
});
}
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp (188389 => 188390)
--- trunk/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp 2015-08-13 19:08:42 UTC (rev 188389)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp 2015-08-13 19:16:09 UTC (rev 188390)
@@ -274,9 +274,6 @@
if (time > durationMediaTime())
time = durationMediaTime();
- if (currentMediaTime() == time)
- return;
-
if (currentTextTrack())
currentTextTrack()->beginSeeking();
@@ -684,8 +681,6 @@
void MediaPlayerPrivateAVFoundation::invalidateCachedDuration()
{
- LOG(Media, "MediaPlayerPrivateAVFoundation::invalidateCachedDuration(%p)", this);
-
m_cachedDuration = MediaTime::invalidTime();
// For some media files, reported duration is estimated and updated as media is loaded
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm (188389 => 188390)
--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm 2015-08-13 19:08:42 UTC (rev 188389)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm 2015-08-13 19:16:09 UTC (rev 188390)
@@ -1305,9 +1305,14 @@
auto weakThis = createWeakPtr();
+ LOG(Media, "MediaPlayerPrivateAVFoundationObjC::seekToTime(%p) - calling seekToTime", this);
+
[m_avPlayerItem.get() seekToTime:cmTime toleranceBefore:cmBefore toleranceAfter:cmAfter completionHandler:^(BOOL finished) {
- callOnMainThread([weakThis, finished] {
+ double currentTime = CMTimeGetSeconds([m_avPlayerItem currentTime]);
+ callOnMainThread([weakThis, finished, currentTime] {
+ UNUSED_PARAM(currentTime);
auto _this = weakThis.get();
+ LOG(Media, "MediaPlayerPrivateAVFoundationObjC::seekToTime(%p) - completion handler called, currentTime = %f", _this, currentTime);
if (!_this)
return;
