1729420 - video frames are displayed unordered when updated the video.currentTime directly

Status: VERIFIED FIXED

(Core :: Audio/Video: Playback, defect, P2)

Description

[shaca]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/92.0.4515.131 Safari/537.36

Steps to reproduce:

to reproduce:
https://codesandbox.io/s/zealous-grass-y7b2b?file=/index.html

click the play buttom

Actual results:

when setting the video.currentTime to play the video, the video played back and forth. (unordered)
It happens when the video is 10fps and the currentTime equals the key frame time. (0.1s interval)

Expected results:

play the video in order.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Audio/Video: Playback' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 2

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Thanks for the bug report and the test case!

This appears to be a regression from bug 1718709 (as verified via mozregression). Alastor, would you be able to take a look at this when you have time? Please adjust severity and priority as you see fit.

Comment 3

[Alastor Wu [:alwu]]

I can reproduce this issue as well, will update more information after I figure out what happens.

Comment 5

[Alastor Wu [:alwu]]

Attachment: Bug 1729420 - part1 : compare the output to the inputs in order to determine if it's an invalid first sample or not.

Comment 6

[Alastor Wu [:alwu]]

Attachment: Bug 1729420 - part2 : add a ref-test to check frame order.

Depends on D124858

Comment 7

[Alastor Wu [:alwu]]

Attachment: Bug 1729420 - part3 : make generateREF.html easier to be used and fix the cross-origin error during capturing the video frame.

Depends on D124882

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/30a53fd882ef
part1 : compare the output to the inputs in order to determine if it's an invalid first sample or not. r=bryce
https://hg.mozilla.org/integration/autoland/rev/b0a5b9acbbe1
part2 : add a ref-test to check frame order. r=bryce
https://hg.mozilla.org/integration/autoland/rev/00789f397362
part3 : make generateREF.html easier to be used and fix the cross-origin error during capturing the video frame. r=bryce

Comment 9

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/30a53fd882ef
https://hg.mozilla.org/mozilla-central/rev/b0a5b9acbbe1
https://hg.mozilla.org/mozilla-central/rev/00789f397362

Comment 10

[Alastor Wu [:alwu]]

Comment on attachment 9239950 [details]
Bug 1729420 - part1 : compare the output to the inputs in order to determine if it's an invalid first sample or not.

Beta/Release Uplift Approval Request

• User impact if declined: Under some situations, user might see video being rendered out-of-order.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See comment0.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): These patches improve the mechanism of discarding the first invalid frame on Windows, and ensure that it won't affect other non-first frames which causes discarding correct frames (this bug). These changes also come with the automation test and passed the other automation tests that we added before for the similar issue.
• String changes made/needed: No

Comment 11

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9239950 [details]
Bug 1729420 - part1 : compare the output to the inputs in order to determine if it's an invalid first sample or not.

P2/S2, has tests and is evaluated to low risk, approved for 93 beta 6, thanks.

Comment 12

[Pascal Chevrel:pascalc (PTO until April 26)]

https://hg.mozilla.org/releases/mozilla-beta/rev/d38d54f1da64
https://hg.mozilla.org/releases/mozilla-beta/rev/32c7fbcbd682
https://hg.mozilla.org/releases/mozilla-beta/rev/c6450091c502

Comment 13

[Ciprian Georgiu, Desktop QA]

I've reproduced this bug using the steps from comment 0, on an affected Nightly build (2021-09-07).

The fix is verified on latest Nightly 94 and Beta 93.0b6, across platforms: Win 10 x64, macOS 11 and Ubuntu 18.04 x64.

Comment 14

[Alastor Wu [:alwu]]

Comment on attachment 9239950 [details]
Bug 1729420 - part1 : compare the output to the inputs in order to determine if it's an invalid first sample or not.

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: These patches handle the case where (1) video playback might be handing for 1+ second during seeking (1721458) (2) video frames might be displayed in an incorrect order (skip some frames)
• User impact if declined: As above, there are two possible bugs.
• Fix Landed on Version: 93
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): These patches implement a better approach to filter out the invalid first video frame on Windows, which aims to solve the issue where we incorrectly discard more frames than we want, and causes video hang. We didn't introduce new feature or change the architecture in these patches, no any huge change gets introduced.In addition, we have automation test to cover this issue. Therefore, these changes are suppose to be low risk.
• String or UUID changes made by this patch:

Comment 15

[Pascal Chevrel:pascalc (PTO until April 26)]

The patches do not apply to the esr91 branch as they are on top of bug 1721458, should we also uplift bug 1721458?

Comment 16

[Pascal Chevrel:pascalc (PTO until April 26)]

Let's track this uplift request for 91.3esr next cycle.

Comment 17

[Alastor Wu [:alwu]]

Yes, thank you. Should I file another approval? The reason I didn't mention that bug is because the the fix of bug 1721458 will be removed in this bug.

Comment 18

[Pascal Chevrel:pascalc (PTO until April 26)]

(In reply to Alastor Wu [:alwu] from comment #17)

Yes, thank you. Should I file another approval? The reason I didn't mention that bug is because the the fix of bug 1721458 will be removed in this bug.

Yes please, and reference this bug in the uplift request, thanks.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9239950 [details]
Bug 1729420 - part1 : compare the output to the inputs in order to determine if it's an invalid first sample or not.

Approved for 91.3esr.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/41935c7e3418
https://hg.mozilla.org/releases/mozilla-esr91/rev/b56444694354
https://hg.mozilla.org/releases/mozilla-esr91/rev/23286eb22089

Comment 21

[Ciprian Georgiu, Desktop QA]

This is also verified as fixed on 91.3.0esr (2021-10-28).