Closed
Bug 1512958
Opened 6 years ago
Closed 6 years ago
Media playback stalls because DecodedStream stops consuming data
Categories
(Core :: Audio/Video, defect, P1)
Tracking
()
VERIFIED
FIXED
mozilla66
Tracking | Status | |
---|---|---|
thunderbird_esr52 | --- | unaffected |
thunderbird_esr60 | --- | unaffected |
geckoview64 | --- | unaffected |
geckoview65 | + | fixed |
firefox-esr60 | --- | unaffected |
firefox64 | --- | unaffected |
firefox65 | + | verified |
firefox66 | + | verified |
People
(Reporter: babib.bamogol, Assigned: pehrsons)
References
Details
(Keywords: regression)
Attachments
(4 files)
(deleted),
application/x-zip-compressed
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
lizzard
:
approval-mozilla-beta+
|
Details |
(deleted),
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).
Assignee | ||
Comment 1•6 years ago
|
||
Alastor, can you take a look?
I'm marking this a P2 for now. Please update appropriately as you find out more.
Rank: 15
Flags: needinfo?(alwu)
Priority: -- → P2
Comment 2•6 years ago
|
||
I can reproduce this issue, will investigate what's happened.
Assignee: nobody → alwu
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(alwu)
Comment 3•6 years ago
|
||
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
Flags: needinfo?(apehrson)
Summary: New autoplay policy makes some WebAudio sounds to never resume → Media playback stalls because DecodedStream stops consuming data
Assignee | ||
Comment 4•6 years ago
|
||
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
Flags: needinfo?(apehrson)
Assignee | ||
Comment 5•6 years ago
|
||
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.
Assignee | ||
Updated•6 years ago
|
Blocks: 1423241
status-firefox64:
--- → unaffected
status-firefox65:
--- → affected
status-firefox66:
--- → affected
status-firefox-esr60:
--- → unaffected
status-geckoview64:
--- → unaffected
status-geckoview65:
--- → affected
tracking-firefox65:
--- → ?
tracking-firefox66:
--- → ?
tracking-geckoview65:
--- → ?
Keywords: regression
Assignee | ||
Comment 6•6 years ago
|
||
Assignee | ||
Comment 7•6 years ago
|
||
Assignee | ||
Comment 8•6 years ago
|
||
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
Tracking to make sure we follow up for 65.
Assignee | ||
Comment 10•6 years ago
|
||
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
Assignee | ||
Comment 13•6 years ago
|
||
Making this a P1 as it has busted seeking on SoundCloud (bug 1515099).
Rank: 15 → 5
Priority: P2 → P1
Updated•6 years ago
|
Attachment #9031490 -
Attachment description: Bug 1512958 - Add mochitest. r?jya → Bug 1512958 - Add mochitest. r?jya!, r?jib!
Assignee | ||
Comment 14•6 years ago
|
||
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.
Comment 15•6 years ago
|
||
Pushed by pehrsons@gmail.com:
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
Assignee | ||
Comment 16•6 years ago
|
||
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?
Comment 17•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1d4816ef6e01
https://hg.mozilla.org/mozilla-central/rev/74101900e7d4
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Comment 18•6 years ago
|
||
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
Status: RESOLVED → REOPENED
Flags: needinfo?(apehrson)
Resolution: FIXED → ---
Target Milestone: mozilla66 → ---
Comment 19•6 years ago
|
||
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)
Comment 20•6 years ago
|
||
This was relanded: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=4d03957994c13b57e6d325f59c004be995342636
Flags: needinfo?(ccoroiu)
Comment 21•6 years ago
|
||
Pushed by aciure@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/e7becb0458ac
Add mochitest. r=jya,jib
Comment 22•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e7becb0458ac
https://hg.mozilla.org/mozilla-central/rev/4d03957994c1
Status: REOPENED → RESOLVED
Closed: 6 years ago → 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
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+
Assignee | ||
Comment 25•6 years ago
|
||
Rebased.
Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6470db6c864a90de99b8fe649a1f168db7f62875
Flags: needinfo?(apehrson)
Comment 26•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/3b1807d65b23
https://hg.mozilla.org/releases/mozilla-beta/rev/45373d310ef9
Flags: in-testsuite+
Updated•6 years ago
|
Flags: qe-verify+
Comment 27•6 years ago
|
||
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
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Updated•6 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•