Closed Bug 1493089 Opened 3 years ago Closed 3 years ago

video loop blinking with controls attribute

Categories

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

62 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla64
Tracking Status
firefox-esr60 --- unaffected
firefox62 --- wontfix
firefox63 + verified
firefox64 --- verified

People

(Reporter: didit21, Assigned: timdream)

References

Details

(Keywords: regression)

Attachments

(2 files)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:62.0) Gecko/20100101 Firefox/62.0
Build ID: 20180905224245

Steps to reproduce:

When visiting a webpage with a video element having both the loop and controls attributes,
the video blinks when looping because it briefly shows the loading animation.

Here is a small example to reproduce the issue:

<!DOCTYPE html>
<html>
<body>
  <video src="vid.mp4" autoplay="true" loop></video>
  <video src="vid.mp4" controls autoplay="true" loop></video>
</body>
</html>

Copy a short vid.mp4 video file in the same directory.
The second video blinks when looping most of the time (but not always).
No blinking with the first video element.


Actual results:

The second video blinks when looping most of the time (often but not always).


Expected results:

This issue appeared in the last few months, probably with Firefox 61 but I am not 100% sure. With previous versions of Firefox, there was no blinking.
Component: Untriaged → Audio/Video: Playback
Product: Firefox → Core
I can reproduce the issue on Windows10 Nightly64.0a1.

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=cbfbccdf8d2f0eacf04e4d6477bfe27f8ae3fad2&tochange=64486670492f5c9cc2e890c5931284bb6a85d194

Regressed by: Bug 1449532
Blocks: 1449532
Status: UNCONFIRMED → NEW
Component: Audio/Video: Playback → Video/Audio Controls
Ever confirmed: true
Keywords: regression
Product: Core → Toolkit
Attached file sample html
:timdream

Your bunch of patch seems to cause the problem. Could you please look into this?
Flags: needinfo?(timdream)
[Tracking Requested - why for this release]:
user-visible regression in a recent release.
Priority: -- → P1
Assignee: nobody → timdream
Status: NEW → ASSIGNED
Flags: needinfo?(timdream)
My understand of Web Animation API was wrong in bug 1449532. If an running
animation is pending, playing a new (reversed) animation will cause it to
play from the beginning of the animation, instead of have the new animation
to continue from the current position.

This has caused the user to see an fade out animation that shouldn't take
place when the video loops, i.e. going from "seeking" to "seeked".

The code now probe into the state of the animation instance and cancel
the pending animation, reverse the running animation, or start the new
(reversed) animation accordingly.
Sadly this does not fix bug 1486278...
Attachment #9011601 - Attachment description: Bug 1493089 - Cancel the pending animation instead reversing it r=Gijs → Bug 1493089 - Cancel the pending animation instead of reversing it r=Gijs
Comment on attachment 9011601 [details]
Bug 1493089 - Cancel the pending animation instead of reversing it r=Gijs

:Gijs (out Thu 27 - Sun 30 / 9;  he/him) has approved the revision.
Attachment #9011601 - Flags: review+
Pushed by tchien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7856d822921f
Cancel the pending animation instead of reversing it r=Gijs
https://hg.mozilla.org/mozilla-central/rev/7856d822921f
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Tim, could you request beta uplift on this one? Thanks
Flags: needinfo?(timdream)
Will ask when bug verifies.

Note: This only happens on platforms where when video loops, going from "seeking" to "seeked" is quicker than Web Animation starts the fade-in animation. Essentially a race between media backend and graphics. So this might not be easy to reproduce.
Cornel, could you verify this bug?

Tim, we only have 2 betas left before entering RC week, could you prepare a parch for uplift to beta please? Thanks
Flags: qe-verify+
Flags: needinfo?(cornel.ionce)
QA Contact: jaws
Comment on attachment 9011601 [details]
Bug 1493089 - Cancel the pending animation instead of reversing it r=Gijs

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1449532

User impact if declined: Video controls will blink when video loops, see test case

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: No

Needs manual test from QE?: Yes

If yes, steps to reproduce: Check the test case

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): localized to video controls.

String changes made/needed: none
Flags: needinfo?(timdream)
Attachment #9011601 - Flags: approval-mozilla-beta?
Comment on attachment 9011601 [details]
Bug 1493089 - Cancel the pending animation instead of reversing it r=Gijs

Tracked 63 P1 regression, uplift approved for 63 beta 13, thanks.
Attachment #9011601 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Contact: jaws
I have managed to reproduce this issue by using the sample html (provided in Comment 2) with Firefox 64.0a1 (BuildId:20180921220134).

This issue is verified fixed using Firefox 64.0a1 (BuildId:20181007220217) on Windows 10 64bit, macOS 10.13.6 and Ubuntu 16.04 64bit.

Leaving a ni? on myself as a reminder to verify this on Firefox 63 beta 13 as well.
Status: RESOLVED → VERIFIED
Flags: needinfo?(cornel.ionce) → needinfo?(emil.ghitta)
This issue is verified fixed using Firefox 63.0b13 (BuildId:20181008155858) on Windows 10 64bit, macOS 10.13.6 and Ubuntu 16.04 64bit.
Flags: qe-verify+
Flags: needinfo?(emil.ghitta)
You need to log in before you can comment on or make changes to this bug.