Intermittent dom/media/test/test_streams_individual_pause.html | video1 video frame should not have updated since video1 paused - got "r0g0b0a0", expected "r0g255b0a255"
Categories
(Core :: WebRTC: Audio/Video, defect, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox69 | --- | fixed |
People
(Reporter: intermittent-bug-filer, Assigned: pehrsons)
References
Details
(Keywords: dev-doc-needed, intermittent-failure)
Attachments
(11 files)
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review |
Filed by: philringnalda [at] gmail.com https://treeherder.mozilla.org/logviewer.html#?job_id=11788185&repo=fx-team https://queue.taskcluster.net/v1/task/IOFk5cv1Su6G_PI66VlB3w/runs/0/artifacts/public%2Flogs%2Flive_backing.log
|
||
Updated•8 years ago
|
|
Assignee | |
Comment 1•8 years ago
|
||
We pause a video element playing a MediaStream and it's visible image becomes black. Even with no alpha. It's as if the drawImage() call bailed and nothing was drawn to the canvas.
|
|
Assignee | |
Comment 2•8 years ago
|
||
We actually have a screenshot here that shows both video elements displaying images at different times. That definitely points to the drawImage() call.
| Comment hidden (Intermittent Failures Robot) |
| Comment hidden (Intermittent Failures Robot) |
| Comment hidden (Intermittent Failures Robot) |
| Comment hidden (Intermittent Failures Robot) |
| Comment hidden (Intermittent Failures Robot) |
| Comment hidden (Intermittent Failures Robot) |
|
||
Comment 9•7 years ago
|
||
https://wiki.mozilla.org/Bugmasters#Intermittent_Test_Failure_Cleanup
|
||
Updated•6 years ago
|
| Comment hidden (Intermittent Failures Robot) |
|
|
||
Comment 11•6 years ago
|
||
https://wiki.mozilla.org/Bug_Triage#Intermittent_Test_Failure_Cleanup
|
||
Comment 12•6 years ago
|
||
This is still happening: https://treeherder.mozilla.org/#/jobs?repo=autoland&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified&fromchange=ad861c124e5d793c3cb776ec3e50088a4786d547&selectedJob=176406223&filter-searchStr=Linux+x64+QuantumRender+debug+Mochitests+with+e10s+test-linux64-qr%2Fdebug-mochitest-media-e10s-2+M-e10s%28mda2%29 https://treeherder.mozilla.org/logviewer.html#?job_id=176406223&repo=autoland
|
|
||
Updated•6 years ago
|
| Comment hidden (Intermittent Failures Robot) |
|
||
Comment 14•6 years ago
|
||
https://wiki.mozilla.org/Bug_Triage#Intermittent_Test_Failure_Cleanup
|
||
Comment 15•6 years ago
|
||
https://treeherder.mozilla.org/logviewer.html#?job_id=198104499&repo=autoland&lineNumber=3022
| Comment hidden (Intermittent Failures Robot) |
| Comment hidden (Intermittent Failures Robot) |
| Comment hidden (Intermittent Failures Robot) |
|
|
Assignee | |
Comment 19•6 years ago
|
||
I caught this in rr. The reference frame for v1 is captured when v1 is in HAVE_NOTHING, so clearly there's nothing to show. The odd thing is that we get "playing" in HAVE_NOTHING, which is against the spec. This is due to [1] where we don't check that we have a frame to display when checking whether we are eligible for autoplay. And since this is for a MediaStream which has only live-data we won't have a frame to display until we are playing. We don't display the current frame as soon as it's been loaded, as we do for playback. Perhaps we should. I don't think the spec is clear about how to handle this, but it will require some spec language archeology to figure out. [1] https://searchfox.org/mozilla-central/rev/3d989d097fa35afe19e814c6d5bc2f2bf10867e1/dom/html/HTMLMediaElement.cpp#6330
|
|
Assignee | |
Updated•6 years ago
|
|
|
Assignee | |
Comment 20•6 years ago
|
||
I found [2]: > When the video element is paused, the current playback position is the first frame of video, and the element's show poster flag is set > The video element represents its poster frame, if any, or else the first frame of the video. This in itself sounds a bit contradictory. What if the show poster flag is not set? Then it do we do the "or else the first frame" or fall through to the next: > The video element represents the last frame of the video to have been rendered. There wouldn't be a "last frame to have been rendered" yet. I suppose we'd go with the "or else the first frame of the video". This then becomes a question on whether MediaStreams have a "first frame of the video". [2] https://html.spec.whatwg.org/multipage/media.html#the-video-element:dom-media-paused
| Comment hidden (Intermittent Failures Robot) |
| Comment hidden (Intermittent Failures Robot) |
|
|
Assignee | |
Comment 23•6 years ago
|
||
Bug 1423241 has made this spike.
|
|
Assignee | |
Comment 24•6 years ago
|
||
jya, do you have thoughts on how we should behave here? In particular this bit: (In reply to Andreas Pehrson [:pehrsons] from comment #20) > I suppose we'd go with the "or else the first frame of the video". This then > becomes a question on whether MediaStreams have a "first frame of the video". Since MediaStreams are not buffered, a "first frame of the video" would be outdated immediately after it's been displayed. How does this work for streaming through the playback stack? Such streams would be buffered, right?
|
|
Assignee | |
Comment 25•6 years ago
|
||
|
|
Assignee | |
Comment 26•6 years ago
|
||
This patch is a stop-gap fix for the intermittent. I plan to follow up with a proper fix after this lands, if we can reach come to a conclusion on how it should be.
|
||
Comment 27•6 years ago
|
||
Pushed by pehrsons@gmail.com: https://hg.mozilla.org/integration/autoland/rev/759ec08eebaa Change from "playing" to "loadeddata" before assuming there's a video frame. r=jib
|
||
Comment 28•6 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/759ec08eebaa
| Comment hidden (Intermittent Failures Robot) |
|
||
Updated•6 years ago
|
|
|
Assignee | |
Comment 30•6 years ago
|
||
I'd still like your thoughts on comment 24 so we can proceed with a long-term solution to this.
|
|
||
Comment 31•6 years ago
|
||
playing is fired when you call play(), irrespective of the readyState. (In reply to Andreas Pehrson [:pehrsons] from comment #24) > jya, do you have thoughts on how we should behave here? > > In particular this bit: > > (In reply to Andreas Pehrson [:pehrsons] from comment #20) > > I suppose we'd go with the "or else the first frame of the video". This then > > becomes a question on whether MediaStreams have a "first frame of the video". > > Since MediaStreams are not buffered, a "first frame of the video" would be > outdated immediately after it's been displayed. > > > How does this work for streaming through the playback stack? Such streams > would be buffered, right? A frame is displayed until there's a new one ready to be displayed. Regardless of buffering. I'm not sure I understand the problem at hand however. "loadeddata" is fired when we have a frame, as such, if the element is playing you can't guarantee that what you see will be the first frame, it may be the next one and so forth. loadeddata is *queued* when there's a first frame, playback is asynchronous so by the time loadeddata actually is fired it's likely it would no longer be the first frame. With playback what you would do here is not call play at all. when loadeddata is fired you're sure to look at the first frame. Waiting on playing was wrong regardless.
|
|
Assignee | |
Comment 32•6 years ago
|
||
(In reply to Jean-Yves Avenard [:jya] from comment #31) > playing is fired when you call play(), irrespective of the readyState. This test doesn't play(). It's using autoplay. So "playing" should fire when we enter HAVE_ENOUGH_DATA. > (In reply to Andreas Pehrson [:pehrsons] from comment #24) > > jya, do you have thoughts on how we should behave here? > > > > In particular this bit: > > > > (In reply to Andreas Pehrson [:pehrsons] from comment #20) > > > I suppose we'd go with the "or else the first frame of the video". This then > > > becomes a question on whether MediaStreams have a "first frame of the video". > > > > Since MediaStreams are not buffered, a "first frame of the video" would be > > outdated immediately after it's been displayed. > > > > > > How does this work for streaming through the playback stack? Such streams > > would be buffered, right? > > A frame is displayed until there's a new one ready to be displayed. > Regardless of buffering. > > I'm not sure I understand the problem at hand however. > > "loadeddata" is fired when we have a frame, as such, if the element is > playing you can't guarantee that what you see will be the first frame, it > may be the next one and so forth. > > loadeddata is *queued* when there's a first frame, playback is asynchronous > so by the time loadeddata actually is fired it's likely it would no longer > be the first frame. > > With playback what you would do here is not call play at all. when > loadeddata is fired you're sure to look at the first frame. > > Waiting on playing was wrong regardless. Well no, I interpret it as correct since we're relying on autoplay to fire "playing". For autoplay, the first frame should be visible while paused, then we start playing automatically when entering HAVE_ENOUGH_DATA and video frames continue from the first frame. But for a MediaStream we don't behave in this way. We don't check for a frame so we start playing in HAVE_NOTHING. To me it then boils down to whether a MediaStream is considered having a first frame or not. If it does, well that's weird since we don't buffer the frames and there'll be an observable jump once playing starts. If it doesn't, well then I have followup questions on when playing should be fired. I'm not sure which makes more sense and that's the dilemma here atm.
|
|
Assignee | |
Comment 33•6 years ago
|
||
I filed https://github.com/whatwg/html/issues/4215 to try to progress this issue.
| Comment hidden (Intermittent Failures Robot) |
|
|
Assignee | |
Comment 35•5 years ago
|
||
The spec issue seems clear -- we should display the first frame even when paused. This will lead to the jump once playing starts, but that's all right. We should be able to align the MediaStream handling in HTMLMediaElement more to that of playback with this change.
Adding dev-doc-needed because this will be a very observable change.
|
|
Assignee | |
Comment 37•5 years ago
|
||
|
|
Assignee | |
Comment 38•5 years ago
|
||
Depends on D33289
|
|
Assignee | |
Comment 39•5 years ago
|
||
Depends on D33290
|
|
Assignee | |
Comment 40•5 years ago
|
||
This allows it to intercept frames in the rendering pipe, so that we don't have
to duplicate the logic for converting VideoChunks to NonOwningImages.
Depends on D33291
|
|
Assignee | |
Comment 41•5 years ago
|
||
Depends on D33292
|
|
Assignee | |
Comment 42•5 years ago
|
||
With the extra first-frame-logic in HTMLMediaElement there are two VideoOutput
instances feeding frames to the same ImageContainer. To maintain FrameID
invariants these need to have unique ProducerIDs.
Depends on D33293
|
|
Assignee | |
Comment 43•5 years ago
|
||
Unsetting a playing video element's MediaStream-srcObject attribute will
otherwise leave the element displaying the latest frame of the video track.
Depends on D33294
|
|
Assignee | |
Comment 44•5 years ago
|
||
Depends on D33295
|
|
Assignee | |
Comment 45•5 years ago
|
||
Depends on D33296
|
|
Assignee | |
Comment 46•5 years ago
|
||
Depends on D33297
|
|
Assignee | |
Comment 47•5 years ago
|
||
|
||
Updated•5 years ago
|
|
|
||
Updated•5 years ago
|
|
|
Assignee | |
Comment 48•5 years ago
|
||
| dev-doc-info | ||
Note that what needs a dev-doc here is that loading a MediaStream in a HTMLMediaElement is now spec-compliant.
The change that makes it so is that we now load a video frame automatically after assigning a MediaStream with a video MediaStreamTrack to the srcObject attribute. Previously we wouldn't load a frame until the element was playing.
Js-observable changes here are most notably that there are a bunch of new events fired on a HTMLMediaElement that does not have the autoplay attribute set to true, and whose play() method isn't called; including "loadedmetadata", "loadeddata", "canplay" and "canplaythrough". This means that the readyState attribute will also automatically advance to HAVE_ENOUGH_DATA once a frame is available.
User-visible changes are most notably that the media element will load the first video frame of the selected video track (NB in case of multiple video tracks, selected here is just internal) and display it as a still frame until the application calls play() or sets the autoplay attribute to true.
|
|
||
Comment 49•5 years ago
|
||
Pushed by pehrsons@gmail.com: https://hg.mozilla.org/integration/autoland/rev/5b2a3e5bd246 Revert temporary fix. r=jib https://hg.mozilla.org/integration/autoland/rev/bd60d9099b47 Add mochitest. r=jib https://hg.mozilla.org/integration/autoland/rev/32c521c1b373 Add WPT. r=jib https://hg.mozilla.org/integration/autoland/rev/c1473d84bdec Move HTMLMediaElement's VideoFrameListener to a VideoOutput. r=jib https://hg.mozilla.org/integration/autoland/rev/e13f35cff432 Load the first frame of a MediaStream with video into a media element when not playing or autoplaying. r=jib https://hg.mozilla.org/integration/autoland/rev/143bc31d3d70 Use ProducerIDs in VideoOutput. r=padenot https://hg.mozilla.org/integration/autoland/rev/d83d0d34de2a Invalidate HTMLVideoElement after resetting videoWidth and videoHeight. r=jya https://hg.mozilla.org/integration/autoland/rev/f03b1a7c02ab Reset mSrcStreamPlaybackEnded when unsetting srcObject. r=padenot https://hg.mozilla.org/integration/autoland/rev/375a2a438161 Modernize test_streams_individual_pause.html. r=jib https://hg.mozilla.org/integration/autoland/rev/0ff338ed7e92 Make HTMLMediaElement::CanActivateAutoplay() spec compliant. r=jya,jib
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/17254 for changes under testing/web-platform/tests
Can't merge web-platform-tests PR due to failing upstream checks: Github PR https://github.com/web-platform-tests/wpt/pull/17254 * Taskcluster (pull_request) (https://tools.taskcluster.net/task-group-inspector/#/dEeqwZhBSS29KeY2hTM7vQ)
|
|
Assignee | |
Comment 52•5 years ago
|
||
(In reply to Web Platform Test Sync Bot from comment #51)
Can't merge web-platform-tests PR due to failing upstream checks:
Github PR https://github.com/web-platform-tests/wpt/pull/17254
- Taskcluster (pull_request)
(https://tools.taskcluster.net/task-group-inspector/#/dEeqwZhBSS29KeY2hTM7vQ)
Why would it try to verify the added WPT tests on current Nightly when there are also pending platform changes...?
|
||
Comment 53•5 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/5b2a3e5bd246
https://hg.mozilla.org/mozilla-central/rev/bd60d9099b47
https://hg.mozilla.org/mozilla-central/rev/32c521c1b373
https://hg.mozilla.org/mozilla-central/rev/c1473d84bdec
https://hg.mozilla.org/mozilla-central/rev/e13f35cff432
https://hg.mozilla.org/mozilla-central/rev/143bc31d3d70
https://hg.mozilla.org/mozilla-central/rev/d83d0d34de2a
https://hg.mozilla.org/mozilla-central/rev/f03b1a7c02ab
https://hg.mozilla.org/mozilla-central/rev/375a2a438161
https://hg.mozilla.org/mozilla-central/rev/0ff338ed7e92
|
||
Comment 54•5 years ago
|
||
Why would it try to verify the added WPT tests on current Nightly when there are also pending platform changes...?
Because the system isn't nearly clever enough to know when test changes are accompanied by platform changes that mean the stability checks ought to be deferred until some future revision is available.
I can force-merge the PR.
Upstream PR merged
|
|
Assignee | |
Comment 56•5 years ago
|
||
(In reply to James Graham [:jgraham] from comment #54)
Why would it try to verify the added WPT tests on current Nightly when there are also pending platform changes...?
Because the system isn't nearly clever enough to know when test changes are accompanied by platform changes that mean the stability checks ought to be deferred until some future revision is available.
I can force-merge the PR.
Ok, thanks!
What revision does it use for the tests? In case I were to try to work around this by postponing the landing of just the WPT changes.
And is there a tracking bug for making it clever enough to figure this out? It feels like it will be quite frequent, at least in my regular workflow.
|
|
||
Comment 57•5 years ago
|
||
The tests are running on GitHub and it doesn't know anything about the Gecko release process; it just pulls the latest nightly to run the tests. It's quite hard to even think about how it would work otherwise; finding some way to use the build from our CI just for upstreamed changes might be possible? But it would certainly not be a small amount of work to handle synchonising the two different systems. I think in the foreseeable future this is likely to be something that's handled manually. FWIW when tests from Gecko only fail stability checks in Firefox I tend to assume that landing them upstream isn't making things worse than they are in m-c and so end up force merging.
|
|
Assignee | |
Comment 58•5 years ago
|
||
(In reply to James Graham [:jgraham] from comment #57)
The tests are running on GitHub and it doesn't know anything about the Gecko release process; it just pulls the latest nightly to run the tests. It's quite hard to even think about how it would work otherwise; finding some way to use the build from our CI just for upstreamed changes might be possible? But it would certainly not be a small amount of work to handle synchonising the two different systems. I think in the foreseeable future this is likely to be something that's handled manually. FWIW when tests from Gecko only fail stability checks in Firefox I tend to assume that landing them upstream isn't making things worse than they are in m-c and so end up force merging.
Without knowing any details..,
The initial comment on github PR has the gecko rev and repo [1], and from those you can figure out what to query hg.mo with to get to the try run on treeherder [2]. Can those be parsed out, and then have it wait for the appropriate taskcluster build to complete, before pulling and running on that?
Or wait for the bugzilla bug to become RESOLVED, and another Nightly build released?
[1] https://github.com/web-platform-tests/wpt/pull/17254
[2] https://hg.mozilla.org/integration/autoland/rev/32c521c1b373336081db70bad913793a1421568c
Upstream PR merged
Upstream PR merged
Upstream PR merged
Upstream PR merged
Upstream PR merged
Upstream PR merged
|
|
Assignee | |
Comment 65•5 years ago
|
||
The upstream PR has gotten "merged" a bit too many times now. James, is everything in order?
|
|
||
Comment 66•5 years ago
|
||
I don't think anything bad is happening apart from the spam, but yeah it shouldn't be doing that. Filed https://github.com/mozilla/wpt-sync/issues/377
Upstream PR merged
Description
•