Bug 196198 – [GTK] Unexpected timeout in http/tests/media/video-play-stall-seek.html

• Component: New Bugs
• Status: RESOLVED FIXED

[Diego Pino]

The test started to time out since r243058 ([GStreamer] Rewrite HTTP source element using pushsrc base class https://bugs.webkit.org/show_bug.cgi?id=195631)

[Philippe Normand]

I suspect it's just slow now, not timing out. So probably a better expectation would be "Slow".

[Diego Pino]

I run the test individually several times and every time it took more than 75 seconds. So it seems the test is actually timing out.

[Diego Pino]

Sometimes the test passes, but most of the times out or fails.

[Diego Pino]

I'm going to change the test to Slow anyway to see if that helps to get more passes. Sorry for the noise.

[Lauro Moura]

Link to results archive: https://results.webkit.org/?platform=GTK&platform=WPE&suite=layout-tests&test=http%2Ftests%2Fmedia%2Fvideo-play-stall-seek.html

Still timing out most times. When it passes, the run time is <= 3s, so this does not look like a Slow test.

Also, it is only passing in X11 Release. Always Timeout or Fail in Debug or Wayland.

I will update the test expectations.

[Enrique Ocaña]

I've ran this test in X11 Release and the test doesn't timeout (Lauro already mentioned this), but fails.

The test is making a request of a media file to the load-and-stall.cgi, which serves part of the file and then stalls on purpose. At some point a seek back to 0 is done and the test expects the readyState >= HAVE_CURRENT_DATA (meaning "part of the video is loaded, including position 0", which is true) and networkState == NETWORK_LOADING (the loading still hasn't finished, this isn't always true, as sometimes it's NETWORK_IDLE).

Debugged why the last condition isn't met and found that CachedResourceStreamingClient::loadFinished() is called (the download is declared to be finished forever because 0 bytes have been read). I went down through the layers down to the IPC that listens the NetworkProcess messages, continued in the NetworkProcess and found that g_input_stream_read_async() was returning 0 bytes because libsoup just behaves that way (or that's whay I could understand with my limited knowledge of libsoup). If no bytes are available immediately, then 0 bytes are returned and the upper layers just assume EOS.

With this in mind, I think the test should be changed and just avoid the networkState check. I don't think anybody would care much, since the test is skipped on win, glib, gtk-wayland and mac (nobody uses it?).

[Philippe Normand]

It still times out on the bots, sometimes fails, but mostly just times out.

If you overload your build machine with, say, a debug build running in another terminal, is the test now timing out?

[Enrique Ocaña]

All my previous comments were about the test failing because of the networkState condition being false, without timeout. Now I've also been able to reproduced sometimes an initial timeout caused by the "waiting" event not being even emitted (a second time, what the test expects) when the video starts. I'm debugging that now.

[Enrique Ocaña]

Created attachment 430863 [details]
Patch

[Enrique Ocaña]

Created attachment 431064 [details]
Patch

[Enrique Ocaña]

Comment on attachment 431064 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:457
> +        GST_DEBUG_OBJECT(pipeline(), "Setting high-percent=0 on GstDownloadBuffer to force 100% buffered reporting");

I forgot to use %% here to escape the percent sign.

[Philippe Normand]

Comment on attachment 431064 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:491
> +    bool m_isWebKitWebSrcHasEos { false };

Two verbs in this variable name ? Maybe one is enough :) What about m_hasWebKitWebSrcSentEOS ?

[Enrique Ocaña]

Comment on attachment 431064 [details]
Patch

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

>> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:491
>> +    bool m_isWebKitWebSrcHasEos { false };
> 
> Two verbs in this variable name ? Maybe one is enough :) What about m_hasWebKitWebSrcSentEOS ?

Sounds good, I'm changing it.

[Enrique Ocaña]

Created attachment 431446 [details]
Patch

[EWS]

Committed r278988 (238914@main): <https://commits.webkit.org/238914@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 431446 [details].

[Radar WebKit Bug Importer]

<rdar://problem/79453713>