Closed Bug 1397173 Opened 2 years ago Closed 2 years ago

Playback shows black frame before seek is done.

Categories

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

defect

Tracking

()

VERIFIED FIXED
Firefox 59
Tracking Status
firefox58 --- verified
firefox59 --- fixed

People

(Reporter: bwu, Assigned: ralin)

Details

Attachments

(1 file)

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.
John, 
This is the bug I talked about.
Flags: needinfo?(jolin)
Priority: -- → P3
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
Flags: needinfo?(jolin)
Flags: needinfo?(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
Flags: needinfo?(ralin)
(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
(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
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.
Flags: needinfo?(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.
Assignee: nobody → ralin
Flags: needinfo?(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.
Status: NEW → ASSIGNED
FWIW, this problem is much easier to notice if users play some non-MSE videos, like pornhub.
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!
Attachment #8930810 - Flags: review?(jolin) → review+
Thanks!
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/7fba7dcf6d36
Apply dark overlay style when only error occurs in mobile video controls. r=jolin
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/7fba7dcf6d36
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Ray,
Can you request an uplift to beta if it is not risky? 
The black frame looks really bad for some websites.
Flags: needinfo?(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!
Flags: needinfo?(ralin)
Attachment #8930810 - Flags: approval-mozilla-beta?
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+.
Attachment #8930810 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
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)
Status: RESOLVED → VERIFIED
Remove qe-verify based on comment 19.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.