Bug 189349 – [GStreamer] Several media related tests timing out around the same revision

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Miguel Gomez]

media/media-ended.html [ Timeout ]
  media/media-fragments/TC0004.html [ Timeout ]
  media/media-fragments/TC0017.html [ Timeout ]
  media/video-currentTime-delay.html [ Timeout ]
  media/video-currentTime-set.html [ Timeout ]

I already reported bugs for the flakiness of these tests, but I suspect there might be something common to all of them that could be related to their timeouts. All the timeouts started to happen around r234983 (at least one at that revision exactly), which makes me think that it could be related.
Checking that revision I see that it's precisely media related: [GStreamer] reduce position queries frequency, so that could have something to do with the problem.

[Miguel Gomez]

Phil, could you give it a look and check whether your change could be causing the timeouts?

If not, just call me crazy and close the bug ;)

[Philippe Normand]

It's possible yes :) I'll try to check this, soon...

[Philippe Normand]

I almost have a patch.

[Philippe Normand]

*** Bug 189340 has been marked as a duplicate of this bug. ***

[Philippe Normand]

*** Bug 189341 has been marked as a duplicate of this bug. ***

[Philippe Normand]

*** Bug 189342 has been marked as a duplicate of this bug. ***

[Philippe Normand]

*** Bug 189346 has been marked as a duplicate of this bug. ***

[Philippe Normand]

*** Bug 189348 has been marked as a duplicate of this bug. ***

[Philippe Normand]

Created attachment 349163 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 349163 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        [GTK] Several media related tests timing out around the same revision

Why is this GTK specific?

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:348
> -    if (m_lastQuery > -1 && ((now - m_lastQuery) < 300) && m_cachedPosition.isValid())
> +    if (m_lastQuery > -1 && ((now - m_lastQuery) < 200) && m_cachedPosition.isValid())

This would be easier to read if m_lastQuery was a std::optional<Seconds> (and probably renamed to m_lastQuertTime) and now was Seconds too. I would also avoid the magic number, giving 200 a name:

static const Seconds positionCacheThreshold = 200_ms;
Seconds now = WTF::WallTime::now().secondsSinceEpoch();
if (m_lastQueryTime && (now - m_lastQueryTime.value()) < positionCacheThreshold && m_cachedPosition.isValid())

something like that

[Philippe Normand]

Committed r235846: <https://trac.webkit.org/changeset/235846>