Closed Bug 1512958 Opened 11 months ago Closed 11 months ago
Media playback stalls because Decoded
Stream stops consuming data
877.03 KB, application/x-zip-compressed
47 bytes, text/x-phabricator-request
|Details | Review|
47 bytes, text/x-phabricator-request
|Details | Review|
7.91 KB, patch
|Details | Diff | Splinter Review|
User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:65.0) Gecko/20100101 Firefox/65.0 Steps to reproduce: - Take a look at the attached sample and run it, in Nightly 65. - Click to play the sound. - When you switch between tabs, the audio should pause or resume, as per the visibilitychange listener: when you go back to the tab after 2 or 3 seconds, the sound resumes as expected. However, if you stay outside from the tab just some seconds more (let's say 10 seconds), the sound will resume when you'll be back to the tab but will be stopped quite immediately after that. Actual results: The sound has stopped playing, instead of resuming. Expected results: The sound should have resumed. This issue seems to only apply to Audio with WebAudio/createMediaElementSource (there is another test case in the html file, which works fine for "basic" Audio).
Alastor, can you take a look? I'm marking this a P2 for now. Please update appropriately as you find out more.
Priority: -- → P2
I can reproduce this issue, will investigate what's happened.
Assignee: nobody → alwu
Status: UNCONFIRMED → NEW
Ever confirmed: true
This issue is unrelated with autoplay policy, you can reproduce this issue even if turning off blocking-autoplay. After some digging, this issue seems to me that the `DecodedStream` didn't consume data correctly. MDSM had decoded enough audio data in queue but nobody consumed them, so that the media playback stalled. In the meanwhile, the AudioContext and AudioDestnationNode were both running well. -- Andreas, do you have any idea about that? Thank you!
Assignee: alwu → nobody
Summary: New autoplay policy makes some WebAudio sounds to never resume → Media playback stalls because DecodedStream stops consuming data
I reproduced it under rr, so I'll have a look. It happened to me just after looping caused a seek back to the beginning, so perhaps it's related to seeking when captured.
Assignee: nobody → apehrson
Status: NEW → ASSIGNED
It is related to seeking, and also triggered by a pause/play cycle. Because on seek and pause/play we tear down the track we're feeding to the MSG and add a new one on the same stream. After the refactor in bug 1423241 we are however not properly cleaning the old tracks up on pause, and so when data for them run out, we stall - even though the new track has data. Such is the logic of the MediaStreamGraph. This will get clearer in the future when all audio is pulled (this is the only audio source that is pushed atm). For the moment however, this is fine. What is missing is marking the old tracks ended. That will make everything jolly. I also noticed we are not cleaning up track listeners for the old tracks, so while they are not doing anything, they are getting unnecessarily notified. After a lot of seeking or pause/play cycles there might be many many many listeners around slowing us down. I'll write a patch to also clean those up. Obviously this needs a test too.
This mochitest reproduces locally, and the fix fixes it. Let's see what try thinks. Test-only: https://treeherder.mozilla.org/#/jobs?repo=try&revision=782229c849ede9b00e9d2fc5280a184bc70449da With fix: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0b407851520b3959100f42529b449a6a6a5124b9
The test was reworked a bit. Test-only: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4d12f7a89593588ade3ff1d5a3743f4b708804b7 With fix: https://treeherder.mozilla.org/#/jobs?repo=try&revision=446a19005dc615351231f595b3e8ad946e0ef246
Making this a P1 as it has busted seeking on SoundCloud (bug 1515099).
Rank: 15 → 5
Priority: P2 → P1
Attachment #9031490 - Attachment description: Bug 1512958 - Add mochitest. r?jya → Bug 1512958 - Add mochitest. r?jya!, r?jib!
It looks like this is not triggering bug 1505250 so much on latest m-c. I'll make an attempt at landing and we'll see if it sticks. If it doesn't, the easiest would be to disable the failing test, unless bug 1505250 is ready by then.
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/1d4816ef6e01 Add mochitest. r=jya,jib https://hg.mozilla.org/integration/autoland/rev/74101900e7d4 Properly clean up produced tracks in DecodedStream. r=jya
Comment on attachment 9031491 [details] Bug 1512958 - Properly clean up produced tracks in DecodedStream. r?jya [Beta/Release Uplift Approval Request] Feature/Bug causing the regression: Bug 1423241 User impact if declined: Playback of media elements can stall in some cases. A broken high-profile site is Soundcloud. 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 comment 0, and bug 1515099 comment 0. List of other uplifts needed: None Risk to taking this patch: Low Why is the change risky/not risky? (and alternatives if risky): We're adding a cleanup step that was missing before, and clarifying the threading model somewhat. The formal is trivial, the latter is simplifying the code. This plus the coverage in automation makes this low risk. String changes made/needed: None
Attachment #9031491 - Flags: approval-mozilla-beta?
Backed out for frequently failing mda Backout: https://hg.mozilla.org/mozilla-central/rev/66865fdea5afa8c7613300502f6f1a33d704754a Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=74101900e7d484cc9ddcba2cd867ca172b961ea0&searchStr=mda1&selectedJob=218453848 Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=218453848&repo=autoland&lineNumber=5035
Hi, Cristina, The fix for the test fail has just landed (bug1505250), so I think now there won't see this fail anymore. Could you help me reland this patch? Thank you.
Flags: needinfo?(apehrson) → needinfo?(ccoroiu)
This was relanded: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=4d03957994c13b57e6d325f59c004be995342636
Pushed by email@example.com: https://hg.mozilla.org/mozilla-central/rev/e7becb0458ac Add mochitest. r=jya,jib
Comment on attachment 9031491 [details] Bug 1512958 - Properly clean up produced tracks in DecodedStream. r?jya Fix for regression in 65, has test coverage, let's uplift for next week's beta 8 build.
Attachment #9031491 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Needs a rebased patch for Beta uplift.
Rebased. Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6470db6c864a90de99b8fe649a1f168db7f62875
Reproduced the bug with 65.0b6 2018-12-20, using test case https://bugzilla.mozilla.org/show_bug.cgi?id=1515099#c0 . Verified as fixed on Ubuntu 16.04x64, Windows 10 x64, Osx 10.14.2 on: 66.0a1 20190103220533 65.0b8 20190103150357
You need to log in before you can comment on or make changes to this bug.