1415596 - Grey haze over video while buffering

Status: RESOLVED FIXED

(Toolkit :: Video/Audio Controls, defect, P1)

Description

[Gian-Carlo Pascutto [:gcp]]

STR:

1) Visit any webm video. e.g: http://techslides.com/demos/sample-videos/small.webm
2) Observe that the video has a grey haze for the first few seconds.

This does not happen on a second playback. I think the grey haze is from when we are buffering, but as the video playback starts immediately the haze actually overlays the video image, making it look terrible.

Comment 1

[Gian-Carlo Pascutto [:gcp]]

8:39.02 INFO: Narrowed inbound regression window from [7183dce1, f8164446] (3 builds) to [c708d8ef, f8164446] (2 builds) (~1 steps left)
 8:39.02 INFO: No more inbound revisions, bisection finished.
 8:39.02 INFO: Last good revision: c708d8efbaa391999fbbddb9ab6f8a9c1cd5eca4
 8:39.02 INFO: First bad revision: f8164446177a17254ee649da73e12b726eb9d325
 8:39.02 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=c708d8efbaa391999fbbddb9ab6f8a9c1cd5eca4&tochange=f8164446177a17254ee649da73e12b726eb9d325

Comment 2

[Ray Lin[:ralin]]

IIRC, the haze has existed for years. The latest style change was in bug 1271765, but just made the opaque haze darker than before. It's unlikely bug 1412210 would regress that, I don't think the visibility change of clickToPlay can affect. Nonetheless, the issue is more like is any way we could make the buffering time more predictable and not to show the overlay if we know the video can play through soon.

Comment 3

[Ray Lin[:ralin]]

No proper idea comes to mind. It's hard to tell how long the buffering work would take, it can be 100ms or 10secs, depend on network condition or the video size. Perhaps, we can set a 300ms delay before overlay show up, which is cancel-able once the buffered frames is enough. Jared, do you have any idea? Thanks.

Comment 4

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Why not just remove the haze?

Comment 5

[Ray Lin[:ralin]]

(In reply to Anthony Jones (:kentuckyfriedtakahe, :k17e) from comment #4)
> Why not just remove the haze?

A dark background is needed to emphasize the information over the haze, such as error messages or icon, those might be obscure by the image otherwise.

Comment 6

[Gian-Carlo Pascutto [:gcp]]

(In reply to Ray Lin[:ralin] from comment #2)
> IIRC, the haze has existed for years. ...It's unlikely bug
> 1412210 would regress that, I don't think the visibility change of
> clickToPlay can affect.

I don't understand what you are saying here. I already bisected the regression to that bug in comment 1. I re-verified today and the outcome was the same. Bug 1412210 regressed this, plain and simple.

Maybe it didn't introduce the actual underlying problem, but it is what is causing the user-visible regression.

Comment 7

[Ray Lin[:ralin]]

(In reply to Gian-Carlo Pascutto [:gcp] from comment #6)
> (In reply to Ray Lin[:ralin] from comment #2)
> > IIRC, the haze has existed for years. ...It's unlikely bug
> > 1412210 would regress that, I don't think the visibility change of
> > clickToPlay can affect.
> 
> I don't understand what you are saying here. I already bisected the
> regression to that bug in comment 1. I re-verified today and the outcome was
> the same. Bug 1412210 regressed this, plain and simple.
> 
> Maybe it didn't introduce the actual underlying problem, but it is what is
> causing the user-visible regression.

Sorry, I think I know what you mean now. I'll look closer to the possible cause.

Comment 8

[Ray Lin[:ralin]]

I would say bug 1412210 revealed the problem I mentioned in comment 2. Before bug 1412210, the incorrect visibility of clickToPlay button suppress the overlay from showing up[0], but in fact the statusOverlay would be set to visible every time receive 'progress' event before it's ready to play. And even the statusOverlay would hide immediately when video start, the fadeout animation lasts so long that overlays the first few frames.

[0] http://searchfox.org/mozilla-central/rev/c99d035f00dd894feff38e4ad28a73fb679c63a6/toolkit/content/widgets/videocontrols.xml#262-265

Comment 9

[Sam Foster [:sfoster] (he/him)]

Triage - Flagging as P1 as this is already assigned and prioritized.

Comment 10

[Ray Lin[:ralin]]

Attachment: Bug 1415596 - Make click to play button transparent when status overlay is present on video controls.

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

Comment 11

[Ray Lin[:ralin]]

I end up reverting partial patch in bug 1412210 to fix this issue, and use CSS to control clickToPlay's visibility. I'll comment more my observation.

Comment 12

[Ray Lin[:ralin]]

I randomly picked a commit in 57, and followed the STR to inspect the debug log from video controls. The log below shows that the clickToPlay.hidden was set to "false" before "playing" event, which is incorrect as the clickToPlay should be hidden from the beginning to the end if it's an autoplay video. Though Bug 1412210 fixed the problem, it brought the regression, and compared with the incorrect clickToPlay.hidden, this regression looks much worse. So, I reverted the fix in bug 1412210 until we can come up with a proper fix and are confident enough not to regress something.  


videoctl: --- videocontrols initialized ---
videocontrols.xml:1644:11
videoctl: Got media event ----> progress
videocontrols.xml:1644:11
click to play button is hidden:  false  <---- should be true, clickToPlay should be hidden at first timing.
videocontrols.xml:260:9
videoctl: Got media event ----> progress
videocontrols.xml:1644:11
click to play button is hidden:  false
videocontrols.xml:260:9
videoctl: Got media event ----> progress
videocontrols.xml:1644:11
click to play button is hidden:  false
videocontrols.xml:260:9
videoctl: Got media event ----> progress
videocontrols.xml:1644:11
click to play button is hidden:  false
videocontrols.xml:260:9
videoctl: Got media event ----> progress
videocontrols.xml:1644:11
click to play button is hidden:  false
videocontrols.xml:260:9
videoctl: Got media event ----> suspend
videocontrols.xml:1644:11
click to play button is hidden:  false
videocontrols.xml:260:9
videoctl: Got media event ----> loadedmetadata
videocontrols.xml:1644:11
videoctl: Duration is 5568ms.

videocontrols.xml:1644:11
videoctl: time update @ 0ms of 5568ms
videocontrols.xml:1644:11
videoctl: Got media event ----> loadeddata
videocontrols.xml:1644:11
click to play button is hidden:  false
videocontrols.xml:260:9
videoctl: Got media event ----> canplay
videocontrols.xml:1644:11
click to play button is hidden:  false
videocontrols.xml:260:9
videoctl: Got media event ----> play
videocontrols.xml:1644:11
click to play button is hidden:  false
videocontrols.xml:260:9
videoctl: Got media event ----> playing
videocontrols.xml:1644:11
click to play button is hidden:  true
videocontrols.xml:260:9
videoctl: Status overlay: seeking=false error=null readyState=4 paused=false ended=false networkState=1 timeUpdateCount=0 _showThrobberTimer=null --> HIDE
videocontrols.xml:1644:11
videoctl: Got media event ----> canplaythrough
videocontrols.xml:1644:11
click to play button is hidden:  true
videocontrols.xml:260:9
videoctl: Status overlay: seeking=false error=null readyState=4 paused=false ended=false networkState=1 timeUpdateCount=0 _showThrobberTimer=null --> HIDE
videocontrols.xml:1644:11
videoctl: Got media event ----> timeupdate
videocontrols.xml:1644:11
click to play button is hidden:  true

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8927726 [details]
Bug 1415596 - Make click to play button transparent when status overlay is present on video controls.

https://reviewboard.mozilla.org/r/199000/#review204204

::: toolkit/themes/shared/media/videocontrols.css:442
(Diff revision 1)
>    opacity: 0;
>  }
> +.statusOverlay[error] + .controlsOverlay > .controlsSpacerStack {

Please merge this with the rule above it.

Comment 14

[Ray Lin[:ralin]]

Comment on attachment 8927726 [details]
Bug 1415596 - Make click to play button transparent when status overlay is present on video controls.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/199000/diff/1-2/

Comment 15

[Ray Lin[:ralin]]

issue fixed, thanks!

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/87bd99540f77
Make click to play button transparent when status overlay is present on video controls. r=jaws

Comment 17

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/87bd99540f77

Comment 18

[Ray Lin[:ralin]]

Comment on attachment 8927726 [details]
Bug 1415596 - Make click to play button transparent when status overlay is present on video controls.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1412210
[User impact if declined]: bad looking grey haze on autoplay video at the first few seconds. 
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: low
[Why is the change risky/not risky?]: reverted fix in bug 1412210 and only one CSS added
[String changes made/needed]: none

Thanks :D

Comment 19

[Gerry Chang [:gchang]]

Comment on attachment 8927726 [details]
Bug 1415596 - Make click to play button transparent when status overlay is present on video controls.

Fix a grey haze over video issue. Beta58+.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/1d2280710c0d