<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[161613] trunk/Source/WebCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/161613">161613</a></dd>
<dt>Author</dt> <dd>jer.noble@apple.com</dd>
<dt>Date</dt> <dd>2014-01-09 17:57:54 -0800 (Thu, 09 Jan 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>[Mac] Scrubbing performance of HD content with software decoding is poor.
https://bugs.webkit.org/show_bug.cgi?id=126705
Reviewed by Eric Carlson.
Instead of issuing a new seek before the previous one completes, wait until that seek's
completion handler is called, and then issue the new seek. This has the added benefit of
coalescing multiple incoming seeks so that only the last one is acted upon.
Save the parameters passed into seekToTime and bind them together in a std::function
to be replayed once the in-flight seek completes. To handle the case where a completion
handler fires after the media player is destroyed, add a weakPtrFactory and pass a
WeakPtr into the completion handler.
Clean up some ivars which are no longer necessary: remove m_seekCount and m_seekTime.
* platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:
(WebCore::MediaPlayerPrivateAVFoundation::MediaPlayerPrivateAVFoundation): Initialize the
WeakPtrFactory.
(WebCore::MediaPlayerPrivateAVFoundation::seekWithTolerance):
(WebCore::MediaPlayerPrivateAVFoundation::seeking): m_seekTime -> m_seeking.
(WebCore::MediaPlayerPrivateAVFoundation::seekCompleted):
* platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h:
* platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:
(WebCore::MediaPlayerPrivateAVFoundationObjC::createWeakPtr):
* platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
(WebCore::MediaPlayerPrivateAVFoundationObjC::MediaPlayerPrivateAVFoundationObjC):
(WebCore::MediaPlayerPrivateAVFoundationObjC::seekToTime):
(WebCore::MediaPlayerPrivateAVFoundationObjC::finishSeek):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsavfoundationMediaPlayerPrivateAVFoundationcpp">trunk/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsavfoundationMediaPlayerPrivateAVFoundationh">trunk/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsavfoundationobjcMediaPlayerPrivateAVFoundationObjCmm">trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsavfoundationobjcMediaPlayerPrivateMediaSourceAVFObjCmm">trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (161612 => 161613)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2014-01-10 01:49:21 UTC (rev 161612)
+++ trunk/Source/WebCore/ChangeLog 2014-01-10 01:57:54 UTC (rev 161613)
</span><span class="lines">@@ -1,3 +1,35 @@
</span><ins>+2014-01-09 Jer Noble <jer.noble@apple.com>
+
+ [Mac] Scrubbing performance of HD content with software decoding is poor.
+ https://bugs.webkit.org/show_bug.cgi?id=126705
+
+ Reviewed by Eric Carlson.
+
+ Instead of issuing a new seek before the previous one completes, wait until that seek's
+ completion handler is called, and then issue the new seek. This has the added benefit of
+ coalescing multiple incoming seeks so that only the last one is acted upon.
+
+ Save the parameters passed into seekToTime and bind them together in a std::function
+ to be replayed once the in-flight seek completes. To handle the case where a completion
+ handler fires after the media player is destroyed, add a weakPtrFactory and pass a
+ WeakPtr into the completion handler.
+
+ Clean up some ivars which are no longer necessary: remove m_seekCount and m_seekTime.
+
+ * platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:
+ (WebCore::MediaPlayerPrivateAVFoundation::MediaPlayerPrivateAVFoundation): Initialize the
+ WeakPtrFactory.
+ (WebCore::MediaPlayerPrivateAVFoundation::seekWithTolerance):
+ (WebCore::MediaPlayerPrivateAVFoundation::seeking): m_seekTime -> m_seeking.
+ (WebCore::MediaPlayerPrivateAVFoundation::seekCompleted):
+ * platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h:
+ * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:
+ (WebCore::MediaPlayerPrivateAVFoundationObjC::createWeakPtr):
+ * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
+ (WebCore::MediaPlayerPrivateAVFoundationObjC::MediaPlayerPrivateAVFoundationObjC):
+ (WebCore::MediaPlayerPrivateAVFoundationObjC::seekToTime):
+ (WebCore::MediaPlayerPrivateAVFoundationObjC::finishSeek):
+
</ins><span class="cx"> 2014-01-08 Jer Noble <jer.noble@apple.com>
</span><span class="cx">
</span><span class="cx"> [MSE][Mac] Report the intrinsic size of the media element
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsavfoundationMediaPlayerPrivateAVFoundationcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp (161612 => 161613)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp 2014-01-10 01:49:21 UTC (rev 161612)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp 2014-01-10 01:57:54 UTC (rev 161613)
</span><span class="lines">@@ -47,6 +47,7 @@
</span><span class="cx">
</span><span class="cx"> MediaPlayerPrivateAVFoundation::MediaPlayerPrivateAVFoundation(MediaPlayer* player)
</span><span class="cx"> : m_player(player)
</span><ins>+ , m_weakPtrFactory(this)
</ins><span class="cx"> , m_queuedNotifications()
</span><span class="cx"> , m_queueMutex()
</span><span class="cx"> , m_networkState(MediaPlayer::Empty)
</span><span class="lines">@@ -58,7 +59,6 @@
</span><span class="cx"> , m_cachedDuration(MediaPlayer::invalidTime())
</span><span class="cx"> , m_reportedDuration(MediaPlayer::invalidTime())
</span><span class="cx"> , m_maxTimeLoadedAtLastDidLoadingProgress(MediaPlayer::invalidTime())
</span><del>- , m_seekTo(MediaPlayer::invalidTime())
</del><span class="cx"> , m_requestedRate(1)
</span><span class="cx"> , m_delayCallbacks(0)
</span><span class="cx"> , m_delayCharacteristicsChangedNotification(0)
</span><span class="lines">@@ -76,7 +76,7 @@
</span><span class="cx"> , m_inbandTrackConfigurationPending(false)
</span><span class="cx"> , m_characteristicsChanged(false)
</span><span class="cx"> , m_shouldMaintainAspectRatio(true)
</span><del>- , m_seekCount(0)
</del><ins>+ , m_seeking(false)
</ins><span class="cx"> {
</span><span class="cx"> LOG(Media, "MediaPlayerPrivateAVFoundation::MediaPlayerPrivateAVFoundation(%p)", this);
</span><span class="cx"> }
</span><span class="lines">@@ -271,6 +271,15 @@
</span><span class="cx">
</span><span class="cx"> void MediaPlayerPrivateAVFoundation::seekWithTolerance(double time, double negativeTolerance, double positiveTolerance)
</span><span class="cx"> {
</span><ins>+ if (m_seeking) {
+ LOG(Media, "MediaPlayerPrivateAVFoundation::seekWithTolerance(%p) - save pending seek", this);
+ m_pendingSeek = [this, time, negativeTolerance, positiveTolerance]() {
+ seekWithTolerance(time, negativeTolerance, positiveTolerance);
+ };
+ return;
+ }
+ m_seeking = true;
+
</ins><span class="cx"> if (!metaDataAvailable())
</span><span class="cx"> return;
</span><span class="cx">
</span><span class="lines">@@ -284,9 +293,7 @@
</span><span class="cx"> currentTrack()->beginSeeking();
</span><span class="cx">
</span><span class="cx"> LOG(Media, "MediaPlayerPrivateAVFoundation::seek(%p) - seeking to %f", this, time);
</span><del>- m_seekTo = time;
</del><span class="cx">
</span><del>- ++m_seekCount;
</del><span class="cx"> seekToTime(time, negativeTolerance, positiveTolerance);
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -311,7 +318,7 @@
</span><span class="cx"> if (!metaDataAvailable())
</span><span class="cx"> return false;
</span><span class="cx">
</span><del>- return m_seekTo != MediaPlayer::invalidTime();
</del><ins>+ return m_seeking;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> IntSize MediaPlayerPrivateAVFoundation::naturalSize() const
</span><span class="lines">@@ -639,14 +646,20 @@
</span><span class="cx"> LOG(Media, "MediaPlayerPrivateAVFoundation::seekCompleted(%p) - finished = %d", this, finished);
</span><span class="cx"> UNUSED_PARAM(finished);
</span><span class="cx">
</span><del>- ASSERT(m_seekCount);
- if (--m_seekCount)
</del><ins>+ m_seeking = false;
+
+ std::function<void()> pendingSeek;
+ std::swap(pendingSeek, m_pendingSeek);
+
+ if (pendingSeek) {
+ LOG(Media, "MediaPlayerPrivateAVFoundation::seekCompleted(%p) - issuing pending seek", this);
+ pendingSeek();
</ins><span class="cx"> return;
</span><ins>+ }
</ins><span class="cx">
</span><span class="cx"> if (currentTrack())
</span><span class="cx"> currentTrack()->endSeeking();
</span><span class="cx">
</span><del>- m_seekTo = MediaPlayer::invalidTime();
</del><span class="cx"> updateStates();
</span><span class="cx"> m_player->timeChanged();
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsavfoundationMediaPlayerPrivateAVFoundationh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h (161612 => 161613)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h 2014-01-10 01:49:21 UTC (rev 161612)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h 2014-01-10 01:57:54 UTC (rev 161613)
</span><span class="lines">@@ -32,8 +32,10 @@
</span><span class="cx"> #include "InbandTextTrackPrivateAVF.h"
</span><span class="cx"> #include "MediaPlayerPrivate.h"
</span><span class="cx"> #include "Timer.h"
</span><ins>+#include <functional>
</ins><span class="cx"> #include <wtf/Functional.h>
</span><span class="cx"> #include <wtf/RetainPtr.h>
</span><ins>+#include <wtf/WeakPtr.h>
</ins><span class="cx">
</span><span class="cx"> namespace WebCore {
</span><span class="cx">
</span><span class="lines">@@ -139,6 +141,8 @@
</span><span class="cx"> MediaPlayerPrivateAVFoundation(MediaPlayer*);
</span><span class="cx"> virtual ~MediaPlayerPrivateAVFoundation();
</span><span class="cx">
</span><ins>+ WeakPtr<MediaPlayerPrivateAVFoundation> createWeakPtr() { return m_weakPtrFactory.createWeakPtr(); }
+
</ins><span class="cx"> // MediaPlayerPrivatePrivateInterface overrides.
</span><span class="cx"> virtual void load(const String& url) OVERRIDE;
</span><span class="cx"> #if ENABLE(MEDIA_SOURCE)
</span><span class="lines">@@ -295,6 +299,10 @@
</span><span class="cx"> private:
</span><span class="cx"> MediaPlayer* m_player;
</span><span class="cx">
</span><ins>+ WeakPtrFactory<MediaPlayerPrivateAVFoundation> m_weakPtrFactory;
+
+ std::function<void()> m_pendingSeek;
+
</ins><span class="cx"> Vector<Notification> m_queuedNotifications;
</span><span class="cx"> mutable Mutex m_queueMutex;
</span><span class="cx">
</span><span class="lines">@@ -313,7 +321,6 @@
</span><span class="cx"> mutable float m_cachedDuration;
</span><span class="cx"> float m_reportedDuration;
</span><span class="cx"> mutable float m_maxTimeLoadedAtLastDidLoadingProgress;
</span><del>- double m_seekTo;
</del><span class="cx"> float m_requestedRate;
</span><span class="cx"> mutable int m_delayCallbacks;
</span><span class="cx"> int m_delayCharacteristicsChangedNotification;
</span><span class="lines">@@ -331,7 +338,7 @@
</span><span class="cx"> bool m_inbandTrackConfigurationPending;
</span><span class="cx"> bool m_characteristicsChanged;
</span><span class="cx"> bool m_shouldMaintainAspectRatio;
</span><del>- size_t m_seekCount;
</del><ins>+ bool m_seeking;
</ins><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> } // namespace WebCore
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsavfoundationobjcMediaPlayerPrivateAVFoundationObjCmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm (161612 => 161613)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm 2014-01-10 01:49:21 UTC (rev 161612)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm 2014-01-10 01:57:54 UTC (rev 161613)
</span><span class="lines">@@ -180,7 +180,6 @@
</span><span class="cx"> -(void)disconnect;
</span><span class="cx"> -(void)playableKnown;
</span><span class="cx"> -(void)metadataLoaded;
</span><del>--(void)seekCompleted:(BOOL)finished;
</del><span class="cx"> -(void)didEnd:(NSNotification *)notification;
</span><span class="cx"> -(void)observeValueForKeyPath:keyPath ofObject:(id)object change:(NSDictionary *)change context:(MediaPlayerAVFoundationObservationContext)context;
</span><span class="cx"> #if HAVE(AVFOUNDATION_MEDIA_SELECTION_GROUP)
</span><span class="lines">@@ -680,12 +679,20 @@
</span><span class="cx"> // setCurrentTime generates several event callbacks, update afterwards.
</span><span class="cx"> setDelayCallbacks(true);
</span><span class="cx">
</span><del>- WebCoreAVFMovieObserver *observer = m_objcObserver.get();
</del><span class="cx"> CMTime cmTime = CMTimeMakeWithSeconds(time, 600);
</span><span class="cx"> CMTime cmBefore = CMTimeMakeWithSeconds(negativeTolerance, 600);
</span><span class="cx"> CMTime cmAfter = CMTimeMakeWithSeconds(positiveTolerance, 600);
</span><ins>+
+ __block auto weakThis = createWeakPtr();
+
</ins><span class="cx"> [m_avPlayerItem.get() seekToTime:cmTime toleranceBefore:cmBefore toleranceAfter:cmAfter completionHandler:^(BOOL finished) {
</span><del>- [observer seekCompleted:finished];
</del><ins>+ dispatch_async(dispatch_get_main_queue(), ^{
+ auto _this = weakThis.get();
+ if (!_this)
+ return;
+
+ _this->seekCompleted(finished);
+ });
</ins><span class="cx"> }];
</span><span class="cx">
</span><span class="cx"> setDelayCallbacks(false);
</span><span class="lines">@@ -1928,14 +1935,6 @@
</span><span class="cx"> m_callback->scheduleMainThreadNotification(MediaPlayerPrivateAVFoundation::Notification::AssetPlayabilityKnown);
</span><span class="cx"> }
</span><span class="cx">
</span><del>-- (void)seekCompleted:(BOOL)finished
-{
- if (!m_callback)
- return;
-
- m_callback->scheduleMainThreadNotification(MediaPlayerPrivateAVFoundation::Notification::SeekCompleted, static_cast<bool>(finished));
-}
-
</del><span class="cx"> - (void)didEnd:(NSNotification *)unusedNotification
</span><span class="cx"> {
</span><span class="cx"> UNUSED_PARAM(unusedNotification);
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsavfoundationobjcMediaPlayerPrivateMediaSourceAVFObjCmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm (161612 => 161613)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm 2014-01-10 01:49:21 UTC (rev 161612)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm 2014-01-10 01:57:54 UTC (rev 161613)
</span><span class="lines">@@ -137,10 +137,9 @@
</span><span class="cx"> // addPeriodicTimeObserverForInterval: throws an exception if you pass a non-numeric CMTime, so just use
</span><span class="cx"> // an arbitrarily large time value of once an hour:
</span><span class="cx"> m_timeJumpedObserver = [m_synchronizer addPeriodicTimeObserverForInterval:toCMTime(MediaTime::createWithDouble(3600)) queue:dispatch_get_main_queue() usingBlock:^(CMTime){
</span><del>- if (m_seeking) {
</del><ins>+ if (m_seeking)
</ins><span class="cx"> m_seeking = false;
</span><del>- m_player->timeChanged();
- }
</del><ins>+ m_player->timeChanged();
</ins><span class="cx"> }];
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>