Title: [120020] trunk/Source/WebCore
- Revision
- 120020
- Author
- [email protected]
- Date
- 2012-06-11 17:02:08 -0700 (Mon, 11 Jun 2012)
Log Message
[BlackBerry] Seek calls are being unnecessarily delayed
https://bugs.webkit.org/show_bug.cgi?id=88732
Patch by Max Feil <[email protected]> on 2012-06-11
Reviewed by Antonio Gomes.
There is a problem with the way the m_userDrivenSeekTimer is
implemented. When MediaPlayerPrivate::seek() is called, there
is always a 100ms delay even if the timer is not running. The
timer is supposed to space out (i.e. throttle) repeated seeks
that come in too soon after a previous seek, but currently it
is slowing down even single seeks and seeks that come in with
adequate delay after a previous seek. I fixed this in my patch
by improving the way the timer fired function is called.
A note on the new m_lastSeekTimePending flag: This flag is
needed so that userDrivenSeekTimerFired() knows whether or not
to perform the seek. The only case where this flag will not be
set is if no MediaPlayerPrivate::seek() call came in while the
timer was active, in which case it's important to do nothing.
I could encode this flag's information into the m_lastSeekTime
float, for example by initializing it and resetting it to NAN
and using isnan(). But I feel that using a separate bool is a
more portable approach.
No new tests. I would like to propose not including a layout test
with this fix. Doing timing tests for 100ms delays is tricky
from _javascript_, and I don't think the benefit of such a test
outweighs the extra time it would take to develop one. The test
would also be a problem to maintain as it may give different
results over different runs and across different target hardware.
* platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:
(WebCore::MediaPlayerPrivate::MediaPlayerPrivate):
(WebCore::MediaPlayerPrivate::seek):
(WebCore::MediaPlayerPrivate::userDrivenSeekTimerFired):
* platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.h:
(MediaPlayerPrivate):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (120019 => 120020)
--- trunk/Source/WebCore/ChangeLog 2012-06-11 23:48:11 UTC (rev 120019)
+++ trunk/Source/WebCore/ChangeLog 2012-06-12 00:02:08 UTC (rev 120020)
@@ -1,5 +1,45 @@
2012-06-11 Max Feil <[email protected]>
+ [BlackBerry] Seek calls are being unnecessarily delayed
+ https://bugs.webkit.org/show_bug.cgi?id=88732
+
+ Reviewed by Antonio Gomes.
+
+ There is a problem with the way the m_userDrivenSeekTimer is
+ implemented. When MediaPlayerPrivate::seek() is called, there
+ is always a 100ms delay even if the timer is not running. The
+ timer is supposed to space out (i.e. throttle) repeated seeks
+ that come in too soon after a previous seek, but currently it
+ is slowing down even single seeks and seeks that come in with
+ adequate delay after a previous seek. I fixed this in my patch
+ by improving the way the timer fired function is called.
+
+ A note on the new m_lastSeekTimePending flag: This flag is
+ needed so that userDrivenSeekTimerFired() knows whether or not
+ to perform the seek. The only case where this flag will not be
+ set is if no MediaPlayerPrivate::seek() call came in while the
+ timer was active, in which case it's important to do nothing.
+ I could encode this flag's information into the m_lastSeekTime
+ float, for example by initializing it and resetting it to NAN
+ and using isnan(). But I feel that using a separate bool is a
+ more portable approach.
+
+ No new tests. I would like to propose not including a layout test
+ with this fix. Doing timing tests for 100ms delays is tricky
+ from _javascript_, and I don't think the benefit of such a test
+ outweighs the extra time it would take to develop one. The test
+ would also be a problem to maintain as it may give different
+ results over different runs and across different target hardware.
+
+ * platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:
+ (WebCore::MediaPlayerPrivate::MediaPlayerPrivate):
+ (WebCore::MediaPlayerPrivate::seek):
+ (WebCore::MediaPlayerPrivate::userDrivenSeekTimerFired):
+ * platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.h:
+ (MediaPlayerPrivate):
+
+2012-06-11 Max Feil <[email protected]>
+
[BlackBerry] Unexpected repeats of short media
https://bugs.webkit.org/show_bug.cgi?id=88733
Modified: trunk/Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp (120019 => 120020)
--- trunk/Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp 2012-06-11 23:48:11 UTC (rev 120019)
+++ trunk/Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp 2012-06-12 00:02:08 UTC (rev 120020)
@@ -110,6 +110,7 @@
#endif
, m_userDrivenSeekTimer(this, &MediaPlayerPrivate::userDrivenSeekTimerFired)
, m_lastSeekTime(0)
+ , m_lastSeekTimePending(false)
, m_waitMetadataTimer(this, &MediaPlayerPrivate::waitMetadataTimerFired)
, m_waitMetadataPopDialogCounter(0)
{
@@ -240,13 +241,18 @@
void MediaPlayerPrivate::seek(float time)
{
m_lastSeekTime = time;
+ m_lastSeekTimePending = true;
if (!m_userDrivenSeekTimer.isActive())
- m_userDrivenSeekTimer.startOneShot(SeekSubmissionDelay);
+ userDrivenSeekTimerFired(0);
}
void MediaPlayerPrivate::userDrivenSeekTimerFired(Timer<MediaPlayerPrivate>*)
{
- m_platformPlayer->seek(m_lastSeekTime);
+ if (m_lastSeekTimePending) {
+ m_platformPlayer->seek(m_lastSeekTime);
+ m_lastSeekTimePending = false;
+ m_userDrivenSeekTimer.startOneShot(SeekSubmissionDelay);
+ }
}
bool MediaPlayerPrivate::seeking() const
Modified: trunk/Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.h (120019 => 120020)
--- trunk/Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.h 2012-06-11 23:48:11 UTC (rev 120019)
+++ trunk/Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.h 2012-06-12 00:02:08 UTC (rev 120020)
@@ -166,6 +166,7 @@
void userDrivenSeekTimerFired(Timer<MediaPlayerPrivate>*);
Timer<MediaPlayerPrivate> m_userDrivenSeekTimer;
float m_lastSeekTime;
+ bool m_lastSeekTimePending;
void waitMetadataTimerFired(Timer<MediaPlayerPrivate>*);
Timer<MediaPlayerPrivate> m_waitMetadataTimer;
int m_waitMetadataPopDialogCounter;
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
