Closed Bug 1512958 Opened 11 months ago Closed 11 months ago

Media playback stalls because DecodedStream stops consuming data

Categories

(Core :: Audio/Video, defect, P1)

65 Branch
defect

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)

Attached file testAudio.zip
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.
Rank: 15
Flags: needinfo?(alwu)
Priority: -- → P2
I can reproduce this issue, will investigate what's happened.
Assignee: nobody → alwu
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(alwu)
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
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)
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.
Tracking to make sure we follow up for 65.
Depends on: 1505250
Duplicate of this bug: 1515220
Duplicate of this bug: 1515099
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 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
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?
https://hg.mozilla.org/mozilla-central/rev/1d4816ef6e01
https://hg.mozilla.org/mozilla-central/rev/74101900e7d4
Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
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)
https://hg.mozilla.org/mozilla-central/rev/e7becb0458ac
https://hg.mozilla.org/mozilla-central/rev/4d03957994c1
Status: REOPENED → RESOLVED
Closed: 11 months ago11 months 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+
Needs a rebased patch for Beta uplift.
Flags: needinfo?(apehrson)
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+
You need to log in before you can comment on or make changes to this bug.