Bug 188645 - [GStreamer] reduce position queries frequency
Summary: [GStreamer] reduce position queries frequency
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Philippe Normand
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-08-16 06:08 PDT by Philippe Normand
Modified: 2018-08-17 07:08 PDT (History)
3 users (show)

See Also:


Attachments
Patch (5.46 KB, patch)
2018-08-16 06:51 PDT, Philippe Normand
calvaris: review+
ews: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews206 for win-future (12.91 MB, application/zip)
2018-08-16 19:59 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
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 Build Bot 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 Build Bot 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>