- Revision
- 206399
- Author
- [email protected]
- Date
- 2016-09-26 15:42:19 -0700 (Mon, 26 Sep 2016)
Log Message
Seeking video doesn't update seek position
https://bugs.webkit.org/show_bug.cgi?id=162575
<rdar://problem/28457219>
Reviewed by Jer Noble.
On ToT, seeking in a video causes the playhead to stutter, and does not actually update media remote's seek
position. This is partly due to how we do not update media remote with new information when beginning to respond
to remote seek commands, so media remote continues to think that a playing video is still playing despite the
user attempting to seek through it.
To fix this, we introduce timer-based guards around remote seek commands, such that a seek "gesture" begins when
we receive the first seek command and ends when no seek command has been received in a set amount of time (this
is 0.5 seconds, which is approximately what other clients around the platform use).
Also, when responding to a remote seek, perform the seek with no tolerance. This prevents the playhead from
stuttering at the end of a seek from the final requested destination of the seek to the last actually seeked
time in the video.
When beginning to seek, we must pause the media. Through existing mechanisms, this causes the media session
manager to update its Now Playing information, which informs media remote that we are no longer playing and
prevents us from stuttering. However, when ending a seek, we must also trigger an additional update to again
refresh media remote's view of the current time. This prevents a flicker when playing media after seeking.
Unit tests to be added in a follow-up due to time constraints.
* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::HTMLMediaElement):
(WebCore::HTMLMediaElement::handleSeekToPlaybackPosition):
(WebCore::HTMLMediaElement::seekToPlaybackPositionEndedTimerFired):
(WebCore::HTMLMediaElement::didReceiveRemoteControlCommand):
* html/HTMLMediaElement.h:
* platform/audio/PlatformMediaSessionManager.h:
(WebCore::PlatformMediaSessionManager::scheduleUpdateNowPlayingInfo):
(WebCore::PlatformMediaSessionManager::sessionDidEndRemoteScrubbing):
(WebCore::PlatformMediaSessionManager::sessions): Deleted.
* platform/audio/mac/MediaSessionManagerMac.h:
* platform/audio/mac/MediaSessionManagerMac.mm:
(WebCore::PlatformMediaSessionManager::updateNowPlayingInfoIfNecessary):
(WebCore::MediaSessionManagerMac::scheduleUpdateNowPlayingInfo):
(WebCore::MediaSessionManagerMac::sessionDidEndRemoteScrubbing):
(WebCore::MediaSessionManagerMac::updateNowPlayingInfo):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (206398 => 206399)
--- trunk/Source/WebCore/ChangeLog 2016-09-26 22:22:18 UTC (rev 206398)
+++ trunk/Source/WebCore/ChangeLog 2016-09-26 22:42:19 UTC (rev 206399)
@@ -1,3 +1,48 @@
+2016-09-26 Wenson Hsieh <[email protected]>
+
+ Seeking video doesn't update seek position
+ https://bugs.webkit.org/show_bug.cgi?id=162575
+ <rdar://problem/28457219>
+
+ Reviewed by Jer Noble.
+
+ On ToT, seeking in a video causes the playhead to stutter, and does not actually update media remote's seek
+ position. This is partly due to how we do not update media remote with new information when beginning to respond
+ to remote seek commands, so media remote continues to think that a playing video is still playing despite the
+ user attempting to seek through it.
+
+ To fix this, we introduce timer-based guards around remote seek commands, such that a seek "gesture" begins when
+ we receive the first seek command and ends when no seek command has been received in a set amount of time (this
+ is 0.5 seconds, which is approximately what other clients around the platform use).
+
+ Also, when responding to a remote seek, perform the seek with no tolerance. This prevents the playhead from
+ stuttering at the end of a seek from the final requested destination of the seek to the last actually seeked
+ time in the video.
+
+ When beginning to seek, we must pause the media. Through existing mechanisms, this causes the media session
+ manager to update its Now Playing information, which informs media remote that we are no longer playing and
+ prevents us from stuttering. However, when ending a seek, we must also trigger an additional update to again
+ refresh media remote's view of the current time. This prevents a flicker when playing media after seeking.
+
+ Unit tests to be added in a follow-up due to time constraints.
+
+ * html/HTMLMediaElement.cpp:
+ (WebCore::HTMLMediaElement::HTMLMediaElement):
+ (WebCore::HTMLMediaElement::handleSeekToPlaybackPosition):
+ (WebCore::HTMLMediaElement::seekToPlaybackPositionEndedTimerFired):
+ (WebCore::HTMLMediaElement::didReceiveRemoteControlCommand):
+ * html/HTMLMediaElement.h:
+ * platform/audio/PlatformMediaSessionManager.h:
+ (WebCore::PlatformMediaSessionManager::scheduleUpdateNowPlayingInfo):
+ (WebCore::PlatformMediaSessionManager::sessionDidEndRemoteScrubbing):
+ (WebCore::PlatformMediaSessionManager::sessions): Deleted.
+ * platform/audio/mac/MediaSessionManagerMac.h:
+ * platform/audio/mac/MediaSessionManagerMac.mm:
+ (WebCore::PlatformMediaSessionManager::updateNowPlayingInfoIfNecessary):
+ (WebCore::MediaSessionManagerMac::scheduleUpdateNowPlayingInfo):
+ (WebCore::MediaSessionManagerMac::sessionDidEndRemoteScrubbing):
+ (WebCore::MediaSessionManagerMac::updateNowPlayingInfo):
+
2016-09-26 Chris Dumez <[email protected]>
[WK2] BlobDownloadClient should use asynchronous IPC to decide destination path
Modified: trunk/Source/WebCore/html/HTMLMediaElement.cpp (206398 => 206399)
--- trunk/Source/WebCore/html/HTMLMediaElement.cpp 2016-09-26 22:22:18 UTC (rev 206398)
+++ trunk/Source/WebCore/html/HTMLMediaElement.cpp 2016-09-26 22:42:19 UTC (rev 206399)
@@ -405,6 +405,7 @@
, m_playbackProgressTimer(*this, &HTMLMediaElement::playbackProgressTimerFired)
, m_scanTimer(*this, &HTMLMediaElement::scanTimerFired)
, m_playbackControlsManagerBehaviorRestrictionsTimer(*this, &HTMLMediaElement::playbackControlsManagerBehaviorRestrictionsTimerFired)
+ , m_seekToPlaybackPositionEndedTimer(*this, &HTMLMediaElement::seekToPlaybackPositionEndedTimerFired)
, m_playedTimeRanges()
, m_asyncEventQueue(*this)
, m_requestedPlaybackRate(1)
@@ -467,6 +468,7 @@
, m_mediaControlsDependOnPageScaleFactor(false)
, m_haveSetUpCaptionContainer(false)
#endif
+ , m_isScrubbingRemotely(false)
#if ENABLE(VIDEO_TRACK)
, m_tracksAreReady(true)
, m_haveVisibleTextTrack(false)
@@ -4573,6 +4575,37 @@
m_playbackControlsManagerBehaviorRestrictionsTimer.startOneShot(HideMediaControlsAfterEndedDelay);
}
+void HTMLMediaElement::handleSeekToPlaybackPosition(double position)
+{
+#if PLATFORM(MAC)
+ // FIXME: This should ideally use faskSeek, but this causes MediaRemote's playhead to flicker upon release.
+ // Please see <rdar://problem/28457219> for more details.
+ seek(MediaTime::createWithDouble(position));
+ m_seekToPlaybackPositionEndedTimer.stop();
+ m_seekToPlaybackPositionEndedTimer.startOneShot(0.5);
+
+ if (!m_isScrubbingRemotely) {
+ m_isScrubbingRemotely = true;
+ if (!paused())
+ pauseInternal();
+ }
+#else
+ fastSeek(position);
+#endif
+}
+
+void HTMLMediaElement::seekToPlaybackPositionEndedTimerFired()
+{
+#if PLATFORM(MAC)
+ if (!m_isScrubbingRemotely)
+ return;
+
+ PlatformMediaSessionManager::sharedManager().sessionDidEndRemoteScrubbing(*m_mediaSession);
+ m_isScrubbingRemotely = false;
+ m_seekToPlaybackPositionEndedTimer.stop();
+#endif
+}
+
void HTMLMediaElement::mediaPlayerVolumeChanged(MediaPlayer*)
{
LOG(Media, "HTMLMediaElement::mediaPlayerVolumeChanged(%p)", this);
@@ -7015,7 +7048,7 @@
case PlatformMediaSession::SeekToPlaybackPositionCommand:
ASSERT(argument);
if (argument)
- fastSeek(argument->asDouble);
+ handleSeekToPlaybackPosition(argument->asDouble);
break;
default:
{ } // Do nothing
Modified: trunk/Source/WebCore/html/HTMLMediaElement.h (206398 => 206399)
--- trunk/Source/WebCore/html/HTMLMediaElement.h 2016-09-26 22:22:18 UTC (rev 206398)
+++ trunk/Source/WebCore/html/HTMLMediaElement.h 2016-09-26 22:42:19 UTC (rev 206399)
@@ -810,6 +810,8 @@
void setControllerJSProperty(const char*, JSC::JSValue);
void addBehaviorRestrictionsOnEndIfNecessary();
+ void handleSeekToPlaybackPosition(double);
+ void seekToPlaybackPositionEndedTimerFired();
Timer m_pendingActionTimer;
Timer m_progressEventTimer;
@@ -816,6 +818,7 @@
Timer m_playbackProgressTimer;
Timer m_scanTimer;
Timer m_playbackControlsManagerBehaviorRestrictionsTimer;
+ Timer m_seekToPlaybackPositionEndedTimer;
GenericTaskQueue<Timer> m_seekTaskQueue;
GenericTaskQueue<Timer> m_resizeTaskQueue;
GenericTaskQueue<Timer> m_shadowDOMTaskQueue;
@@ -968,6 +971,8 @@
bool m_haveSetUpCaptionContainer : 1;
#endif
+ bool m_isScrubbingRemotely : 1;
+
#if ENABLE(VIDEO_TRACK)
bool m_tracksAreReady : 1;
bool m_haveVisibleTextTrack : 1;
Modified: trunk/Source/WebCore/platform/audio/PlatformMediaSessionManager.h (206398 => 206399)
--- trunk/Source/WebCore/platform/audio/PlatformMediaSessionManager.h 2016-09-26 22:22:18 UTC (rev 206398)
+++ trunk/Source/WebCore/platform/audio/PlatformMediaSessionManager.h 2016-09-26 22:42:19 UTC (rev 206399)
@@ -51,6 +51,7 @@
virtual ~PlatformMediaSessionManager() { }
+ virtual void scheduleUpdateNowPlayingInfo() { }
bool has(PlatformMediaSession::MediaType) const;
int count(PlatformMediaSession::MediaType) const;
bool activeAudioSessionRequired() const;
@@ -85,6 +86,7 @@
virtual bool sessionWillBeginPlayback(PlatformMediaSession&);
virtual void sessionWillEndPlayback(PlatformMediaSession&);
virtual bool sessionCanLoadMedia(const PlatformMediaSession&) const;
+ virtual void sessionDidEndRemoteScrubbing(const PlatformMediaSession&) { };
virtual void clientCharacteristicsChanged(PlatformMediaSession&) { }
#if PLATFORM(IOS)
Modified: trunk/Source/WebCore/platform/audio/mac/MediaSessionManagerMac.h (206398 => 206399)
--- trunk/Source/WebCore/platform/audio/mac/MediaSessionManagerMac.h 2016-09-26 22:22:18 UTC (rev 206398)
+++ trunk/Source/WebCore/platform/audio/mac/MediaSessionManagerMac.h 2016-09-26 22:42:19 UTC (rev 206399)
@@ -27,6 +27,7 @@
#if PLATFORM(MAC)
+#include "GenericTaskQueue.h"
#include "PlatformMediaSessionManager.h"
namespace WebCore {
@@ -40,10 +41,12 @@
MediaSessionManagerMac();
+ void scheduleUpdateNowPlayingInfo() override;
void removeSession(PlatformMediaSession&) override;
bool sessionWillBeginPlayback(PlatformMediaSession&) override;
void sessionWillEndPlayback(PlatformMediaSession&) override;
+ void sessionDidEndRemoteScrubbing(const PlatformMediaSession&) override;
void clientCharacteristicsChanged(PlatformMediaSession&) override;
void updateNowPlayingInfo();
@@ -50,11 +53,9 @@
PlatformMediaSession* nowPlayingEligibleSession();
- double m_reportedRate { 0 };
- double m_reportedDuration { 0 };
- String m_reportedTitle;
bool m_nowPlayingActive { false };
bool m_isInBackground { false };
+ GenericTaskQueue<Timer> m_nowPlayingUpdateTaskQueue;
};
} // namespace WebCore
Modified: trunk/Source/WebCore/platform/audio/mac/MediaSessionManagerMac.mm (206398 => 206399)
--- trunk/Source/WebCore/platform/audio/mac/MediaSessionManagerMac.mm 2016-09-26 22:22:18 UTC (rev 206398)
+++ trunk/Source/WebCore/platform/audio/mac/MediaSessionManagerMac.mm 2016-09-26 22:42:19 UTC (rev 206399)
@@ -56,7 +56,7 @@
void PlatformMediaSessionManager::updateNowPlayingInfoIfNecessary()
{
if (auto existingManager = (MediaSessionManagerMac *)PlatformMediaSessionManager::sharedManagerIfExists())
- existingManager->updateNowPlayingInfo();
+ existingManager->scheduleUpdateNowPlayingInfo();
}
MediaSessionManagerMac::MediaSessionManagerMac()
@@ -69,6 +69,12 @@
{
}
+void MediaSessionManagerMac::scheduleUpdateNowPlayingInfo()
+{
+ if (!m_nowPlayingUpdateTaskQueue.hasPendingTasks())
+ m_nowPlayingUpdateTaskQueue.enqueueTask(std::bind(&MediaSessionManagerMac::updateNowPlayingInfo, this));
+}
+
bool MediaSessionManagerMac::sessionWillBeginPlayback(PlatformMediaSession& session)
{
if (!PlatformMediaSessionManager::sessionWillBeginPlayback(session))
@@ -79,6 +85,11 @@
return true;
}
+void MediaSessionManagerMac::sessionDidEndRemoteScrubbing(const PlatformMediaSession&)
+{
+ scheduleUpdateNowPlayingInfo();
+}
+
void MediaSessionManagerMac::removeSession(PlatformMediaSession& session)
{
PlatformMediaSessionManager::removeSession(session);
@@ -123,9 +134,6 @@
LOG(Media, "MediaSessionManagerMac::updateNowPlayingInfo - clearing now playing info");
MRMediaRemoteSetNowPlayingInfo(nullptr);
m_nowPlayingActive = false;
- m_reportedTitle = "";
- m_reportedRate = 0;
- m_reportedDuration = 0;
MRMediaRemoteSetNowPlayingApplicationPlaybackStateForOrigin(MRMediaRemoteGetLocalOrigin(), kMRPlaybackStateStopped, dispatch_get_main_queue(), ^(MRMediaRemoteError error) {
#if LOG_DISABLED
UNUSED_PARAM(error);
@@ -146,16 +154,6 @@
String title = currentSession->title();
double duration = currentSession->duration();
double rate = currentSession->state() == PlatformMediaSession::Playing ? 1 : 0;
- if (m_reportedTitle == title && m_reportedRate == rate && m_reportedDuration == duration) {
- m_nowPlayingActive = true;
- LOG(Media, "MediaSessionManagerMac::updateNowPlayingInfo - nothing new to show");
- return;
- }
-
- m_reportedRate = rate;
- m_reportedDuration = duration;
- m_reportedTitle = title;
-
auto info = adoptCF(CFDictionaryCreateMutable(kCFAllocatorDefault, 4, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks));
if (!title.isEmpty())
