1359815 - Spinning loading UI is not used when resuming video element's video decoder takes longer than 250ms

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect)

Description

[Adrian Florinescu [:aflorinescu]]

[Description:]
I haven't really figured out how to measure the resume video's element timer, but I've assumed that using full HD videos which apparently take more than 1-2 seconds to resume should use the spinning loading UI.

[Steps:]
1. Open FF Nightly.
2. Open and start playing several HD videos and one empty tab.
3. Focus the empty tab.
4. Either set the "media.suspend-bkgnd-video.delay-ms" to a lower value (1000) or wait 10 seconds for video decoder to be shutdown.
5. Start focusing the HD youtube tabs, one by one.

[Actual Result:]
In no circumstance the spinning loading UI is shown, not even the resume from last frame takes more than 250ms.

[Expected Result:]
My assumption is that the resume should encounter the 250ms scenario quite often, especially in the HD videos area.

Comment 1

[Tzuhao Kuo [:kaku]]

I have looked into it and the problem is that I implement the 250ms timer to wait untie "the decoder is resumed" and "the first frame is decoded", however, there is a gap between "the first frame is decoded" and "is rendered". What users experienced is the time of rendered and I need to find a way to indicate our video controller the time of " the first frame is rendered".

Comment 2

[Tzuhao Kuo [:kaku]]

Here is another root cause.

Here, http://searchfox.org/mozilla-central/rev/8b70b0a5038ef9472fe7c53e04a70c978bb06aed/toolkit/content/widgets/videocontrols.xml#521-522, we request the throbber after waiting for 250ms by calling `this.setupStatusFader()` without giving the `immediate` argument which leads to another 750ms delay time and 300ms animation duration before we can really see the throbber. The two variables are defined here: http://searchfox.org/mozilla-central/rev/8b70b0a5038ef9472fe7c53e04a70c978bb06aed/toolkit/themes/shared/media/videocontrols.css#451-452

This one is easy to fix, a patch is following.

Comment 3

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1359815 - show throbber sooner;

Review commit: https://reviewboard.mozilla.org/r/137016/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/137016/

Comment 4

[Ray Lin[:ralin]]

Comment on attachment 8865362 [details]
Bug 1359815 - show throbber sooner;

https://reviewboard.mozilla.org/r/137016/#review139982

Thanks for this quick fix, Looks good to me :)

Comment 5

[Tzuhao Kuo [:kaku]]

Thanks for the review.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/204520aa1099
show throbber sooner; r=ralin

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/204520aa1099

Comment 8

[Adrian Florinescu [:aflorinescu]]

I have checked the fix on latest Nightly 55 (20170509100208). With the fix applied, the spinning loading UI is now visible. I can only speculate at this point that from the code perspective, the implementation is correct, but from the user experience point of view, there are cases in which I would expect the loading UI to be displayed but it is not shown. IMHO, I don't think this is an implementation issue, but more likely an incomplete specification of this part of shutdown decoder feature. The reason for which I say this is that as an user I see big resume time that do not display the loading UI. My guess is that there are events in the video resume or browser that we are not considering when deciding to show/not show the spinning_UI. 

One simple use case to observe this would be: open https://people-mozilla.org/~aflorinescu/ShutdownDecoder/1080video.html in several tabs, wait for the shutdown to kick in, then start to switch the tabs. You will notice that there are cases in which from the user experience point of view you would expect to get a spinning loading_UI, but you don't get it.

I speculate that to get the spinning loading_UI function properly from the user perspective, the spinning UI algorithm will probably need to get over-complicated and is likely to involve taking into account areas that are out-of-scope of Shutdown decoder.

I'm not sure what should be the next step here. :kaku, thoughts?