1397173 - Playback shows black frame before seek is done.

Status: VERIFIED FIXED

(Firefox for Android :: Audio/Video, defect, P3)

Description

[Blake Wu [:bwu][:blakewu]]

When seeking on Youtube videos, I can see black frame before seek is done. The better user experience should keep the last video frame until seek is done instead of showing black screen.

Comment 1

[Blake Wu [:bwu][:blakewu]]

John, 
This is the bug I talked about.

Comment 2

[John Lin [:jhlin][:jolin]]

Looks like the fading effect is controlled by builtin video controls [1]. 

FWICT, in YT both seeking and network loading could trigger that.

How about keeping it only for error state?

[1] http://searchfox.org/mozilla-central/source/toolkit/content/widgets/videocontrols.xml#259

Comment 3

[Ray Lin[:ralin]]

Our built-in controls only uses a grey opaque overlay instead of totally fading out into black in status fader. I would suspect YT put another layer on the top of our controls. (there's a precedent that YT shows their own throbber while seeking/loading....)

I tried to reproduce it on other websites like https://www.html5rocks.com/en/tutorials/video/basics/ or https://www.iandevlin.com/html5test/webvtt/html5-video-webvtt-sample.html, but didn't spot the issue. In case I misunderstood the problem, could you help to check if the problem occurs as well on those websites? Thanks

Comment 4

[John Lin [:jhlin][:jolin]]

(In reply to Ray Lin[:ralin] from comment #3)
> Our built-in controls only uses a grey opaque overlay instead of totally
> fading out into black in status fader. I would suspect YT put another layer
> on the top of our controls. (there's a precedent that YT shows their own
> throbber while seeking/loading....)

 The bug summary is kind of misleading. What's observed is short transitions into/from (very) dark gray frame instead of pure black.
 When I modified `setupStatusFader()` [1] to not setting `show` at [1], the transition just disappeared.

> I tried to reproduce it on other websites like
> https://www.html5rocks.com/en/tutorials/video/basics/ or
> https://www.iandevlin.com/html5test/webvtt/html5-video-webvtt-sample.html,
> but didn't spot the issue. In case I misunderstood the problem, could you
> help to check if the problem occurs as well on those websites? Thanks

 I didn't see the effect on these websites either, nor on Vimeo (though that could be because Vimeo uses custom controls). So far only YT and some direct links to online video show this symptom.

[1] http://searchfox.org/mozilla-central/source/toolkit/content/widgets/videocontrols.xml#279

Comment 5

[Ray Lin[:ralin]]

(In reply to John Lin [:jolin][:jhlin] from comment #4)

>  The bug summary is kind of misleading. What's observed is short transitions
> into/from (very) dark gray frame instead of pure black.
>  When I modified `setupStatusFader()` [1] to not setting `show` at [1], the
> transition just disappeared.
Thanks for the clarification. If so, it makes sense to consider disabling the transition is this case. Though we'll have to make sure this won't regress desktop media controls unless this behavior is desired for both

Comment 6

[John Lin [:jhlin][:jolin]]

I don't think desktop pages are affected by it as often as mobile ones because a. desktop usually has faster connections and b. sites tend to implement custom controls for desktop pages AFAICT.

Ray, could you please help disable it only for mobile at first to minimize the UX impact? Thanks a lot.

Comment 7

[Ray Lin[:ralin]]

Ok, I can disable that for only mobile and when it's under seeking state. And it makes sense since we've removed the throbber on Fennec in Bug 1289412 , no need to fade-in the dark overlay showing w/o any icon.

Comment 8

[Ray Lin[:ralin]]

Attachment: Bug 1397173 - Apply dark overlay style when only error occurs in mobile video controls.

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

Comment 9

[Ray Lin[:ralin]]

The patch is a simple one liner applies the dark styles for only error occurrence, since no other cases will need the overlay after the throbber icon was removed. Thanks.

Comment 10

[Blake Wu [:bwu][:blakewu]]

FWIW, this problem is much easier to notice if users play some non-MSE videos, like pornhub.

Comment 11

[John Lin [:jhlin][:jolin]]

Comment on attachment 8930810 [details]
Bug 1397173 - Apply dark overlay style when only error occurs in mobile video controls.

https://reviewboard.mozilla.org/r/201904/#review207272

LGTM. Thanks!

Comment 12

[Ray Lin[:ralin]]

Thanks!

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7fba7dcf6d36
Apply dark overlay style when only error occurs in mobile video controls. r=jolin

Comment 14

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/7fba7dcf6d36

Comment 15

[Blake Wu [:bwu][:blakewu]]

Ray,
Can you request an uplift to beta if it is not risky? 
The black frame looks really bad for some websites.

Comment 16

[Ray Lin[:ralin]]

Comment on attachment 8930810 [details]
Bug 1397173 - Apply dark overlay style when only error occurs in mobile video controls.

Approval Request Comment
[Feature/Bug causing the regression]: Video controls seeking
[User impact if declined]: distracting grey overlay shows up on mobile video controls every seeking
[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?]: very low
[Why is the change risky/not risky?]: one line CSS rule fix to prevent grey overlay from present except getting error
[String changes made/needed]: none

Thanks!

Comment 17

[Gerry Chang [:gchang]]

Comment on attachment 8930810 [details]
Bug 1397173 - Apply dark overlay style when only error occurs in mobile video controls.

Fix a black frame issue when seeking. Beta58+.

Comment 18

[Andrei Ciure[:andrei_ciure]]

https://hg.mozilla.org/releases/mozilla-beta/rev/725b718e10c8

Comment 19

[Oana Horvath]

Verified as fixed on Beta 58.0b7.
Devices:
Google Pixel (Android 8.0)
LG Nexus 5 (Android 6.0.1)
Samsung Galaxy Tab 3 (Android 7.0)

Comment 20

[Stefan Deiac]

Remove qe-verify based on comment 19.