Bug 188645

Summary: [GStreamer] reduce position queries frequency
Product: WebKit Reporter: Philippe Normand <pnormand>
Component: PlatformAssignee: Philippe Normand <pnormand>
Status: RESOLVED FIXED    
Severity: Normal CC: calvaris, ews-watchlist, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
calvaris: review+, ews-watchlist: commit-queue-
Archive of layout-test-results from ews206 for win-future none

Description Philippe Normand 2018-08-16 06:08:50 PDT
.
Comment 1 Philippe Normand 2018-08-16 06:51:10 PDT
Created attachment 347261 [details]
Patch
Comment 2 EWS Watchlist 2018-08-16 19:59:32 PDT
Comment on attachment 347261 [details]
Patch

Attachment 347261 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8887216

New failing tests:
http/tests/security/local-video-source-from-remote.html
Comment 3 EWS Watchlist 2018-08-16 19:59:44 PDT
Created attachment 347341 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 4 Xabier Rodríguez Calvar 2018-08-17 02:47:02 PDT
Comment on attachment 347261 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=347261&action=review

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:351
> +    double now = WTF::WallTime::now().secondsSinceEpoch().milliseconds();
> +    if (m_lastQuery > -1 && ((now - m_lastQuery) < 300) && m_cachedPosition.isValid())
> +        return m_cachedPosition;
> +
> +    m_lastQuery = now;

My preference here, to save one variable and one call, would be:
(if m_cachedPosition.isValid()) {
   double now = ...
   if (m_last...
Comment 5 Philippe Normand 2018-08-17 03:08:05 PDT
I don't understand what you mean :)
Comment 6 Xabier Rodríguez Calvar 2018-08-17 03:49:33 PDT
(In reply to Philippe Normand from comment #5)
> I don't understand what you mean :)

I meant this:

if (m_cachedPosition.isValid()) {
    double now = WTF::WallTime::now().secondsSinceEpoch().milliseconds();
    if (m_lastQuery > -1 && ((now - m_lastQuery) < 300))
        return m_cachedPosition;

    m_lastQuery = now;
}

but I realize it is stupid as because m_lastQuery = now needs to be done outsize the loop to update it also in case the cached position is invalid and for that you also need the "now" variable.

Summing up, forget what I said here and land if you want.
Comment 7 Philippe Normand 2018-08-17 07:07:53 PDT
Committed r234983: <https://trac.webkit.org/changeset/234983>
Comment 8 Radar WebKit Bug Importer 2018-08-17 07:08:21 PDT
<rdar://problem/43422708>