Closed Bug 1415596 Opened 3 years ago Closed 3 years ago

Grey haze over video while buffering

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox-esr52 --- unaffected
firefox57 --- unaffected
firefox58 --- fixed
firefox59 --- fixed

People

(Reporter: gcp, Assigned: ralin)

References

Details

(Keywords: regression)

Attachments

(1 file)

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.
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
Blocks: 1412210
Keywords: regression
Flags: needinfo?(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.
Flags: needinfo?(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.
Flags: needinfo?(jaws)
Component: Audio/Video: Playback → Video/Audio Controls
Product: Core → Toolkit
Why not just remove the haze?
(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.
(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.
(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.
Assignee: nobody → ralin
Flags: needinfo?(jaws)
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
Triage - Flagging as P1 as this is already assigned and prioritized.
Priority: -- → P1
Status: NEW → ASSIGNED
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.
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 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.
Attachment #8927726 - Flags: review?(jaws) → review+
issue fixed, thanks!
Keywords: checkin-needed
Pushed by dluca@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/87bd99540f77
Make click to play button transparent when status overlay is present on video controls. r=jaws
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/87bd99540f77
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
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
Attachment #8927726 - Flags: approval-mozilla-beta?
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+.
Attachment #8927726 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.