Audiosinks are not cleaned up on navigation
Categories
(Core :: Audio/Video: Playback, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox75 | --- | verified |
People
(Reporter: u644666, Assigned: achronop)
References
Details
Attachments
(2 files, 2 obsolete files)
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Ubuntu Chromium/75.0.3770.142 Chrome/75.0.3770.142 Safari/537.36
Steps to reproduce:
- Opened youtube, watched a video
- Went back to youtube homepage using a bookmark
Actual results:
- Opened youtube, watched a video
- Went back to youtube homepage using a bookmark
- i noticed that even with 1 tab, there are 2 volume sliders in the system audio section, i repeated this step and each time a new slider was created. None of them disappeared until i closed Firefox
Expected results:
Firefox should not keep creating new audio channels without deleting the previous ones, and even kill thoose that are not playing anything to keep things clean. Or have an option for the entirety of firefox to be controlled with 1 slider.
Updated•5 years ago
|
Comment 2•5 years ago
|
||
I was unable to confirm this behavior on my Ubuntu 18.04 machine, but someone else may be able to repro.
https://www.youtube.com/watch?v=YD2Mtnsxp-U&feature=youtu.be
sorry for bad quality and recording artifacts. They are not related, i had to upload with this quality because of my weak internet connection and the artifacts are caused by badly made as always nvidia properitary drivers
Comment 6•5 years ago
|
||
I can reproduce.
I don't see the output streams in Ubuntu chrome like you, but I use pavuctl
to monitor them, and they build up until I close the tab.
For me it was enough to open a youtube video with audio, play it so the output stream gets opened, then reload and do it again. Seems silly to have the old output streams stay alive like that.
I'll try to capture it in a pernosco recording so we can debug efficiently.
Comment 7•5 years ago
|
||
I got an rr recording with the STR:
- load a youtube video
- play (plays an ad)
- wait until actual video plays
- reload
- play
- reload
- play (plays another ad)
- wait until actual video plays
- there are now 2 audio streams (expected 3, one went away)
- open new, empty tab
- close youtube tab
- wait until audio streams are gone
- close firefox
Uploading to pernosco now.
Comment 8•5 years ago
|
||
The pernosco recording is at https://pernos.co/debug/sj056teOMSKZHm7eBBxb0A/index.html
So normally we'd stop the audio stream when playback ends, but these youtube videos don't end for some reason.
One can see this by filtering in pernosco on the executions of HTMLMediaElement::PlaybackEnded.
See also the notebook for the relevant moments in time creating and tearing down decoders; and when the AudioStreams were paused instead of stopped (the bug, if you will).
These three videos are played using MediaSource Extensions (MSE), where it's up to the application (youtube) to handle buffering of data. If I filter pernosco on executions of MediaSource::EndOfStream, I can see that it's called for the two ads, and for the second video. Once in a ProgressEvent, and once in an "updateend" event.
The first and third videos' MediaSources do not get any calls to endOfStream()
at all. For the second one we do so I'd kind of expect us to play it until the end, and then shut down. But that doesn't happen either, because HTMLMediaElement::NotifyOwnerDocumentActivityChanged()
pauses and suspends it.
I wonder how hard it would be to kill the MediaSink when we get suspended.
Comment 9•5 years ago
|
||
Adding some people to CC in case anyone is interested in picking this up.
Reporter | ||
Comment 10•5 years ago
|
||
(In reply to Andreas Pehrson [:pehrsons] from comment #9)
Adding some people to CC in case anyone is interested in picking this up.
Will it be fixed in the next update ? I really like Firefox but cant use it because of this
Comment 11•5 years ago
|
||
(In reply to Qik from comment #10)
Will it be fixed in the next update ? I really like Firefox but cant use it because of this
That depends. If we get a patch soon enough it could get into Firefox 70. If anyone wants to contribute to this, that's also welcome.
Reporter | ||
Comment 12•5 years ago
|
||
Just tested this on my brothers computer running Mint 19 XFCE
Happens EXACTLY line on my computer.
Tested on firefox 70.0.1
Assignee | ||
Comment 13•5 years ago
|
||
I gave a try to this one. My finding is that after a refresh the old dom tree, which includes the media element, is not deleted. I guess this is due to some kind of caching (bfcache maybe?). So after the refresh the old stream remains and, in parallel, a new media element is created with a new stream attached. When the tab is closing, all dom trees are being cleaned up so the media elements and the streams are cleaned up, eventually.
I also noticed that on refresh, the old media element is paused and suspended. What suggested in comment 8 can be the solution here. We could implement the MediaDecoder::Suspend()
method, which is currently blank, to shutdown the MediaSink. That looks relatively easy and will clear the streams on refresh. We must be careful though, to restore everything if the medial element becomes active again.
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 14•5 years ago
|
||
Implement the MediaDecoder::Suspend/Resume
to shutdown and recreate the MediaSink. This is important to refresh because the existing tree does not shutdown. It stays alive but remains suspended. Thus it is necessary to clean up the low-level media resource on suspend. Otherwise, for example in audio case, the system audio stream does not clean up and it is listed in system settings.
Assignee | ||
Comment 15•5 years ago
|
||
Assignee | ||
Comment 16•5 years ago
|
||
Assignee | ||
Comment 17•5 years ago
|
||
Reporter | ||
Comment 18•5 years ago
|
||
(In reply to Alex Chronopoulos [:achronop] from comment #17)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=169a98e2955e211ab1363cb8233e960fa260767d
Is this being worked on ? If so, THANK YOU !
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 19•5 years ago
|
||
The situation has been improved after Bug 1610193 has landed. The old tree including the media element and the audio resources are collected eventually at some point bedore tab close. The system audio streams remain open till then.
Reporter | ||
Comment 20•5 years ago
|
||
(In reply to Alex Chronopoulos [:achronop] from comment #19)
The situation has been improved after Bug 1610193 has landed. The old tree including the media element and the audio resources are collected eventually at some point bedore tab close. The system audio streams remain open till then.
For me the audio channels remain until i close the tab. I left my laptop running for 2 houts to test that
Assignee | ||
Comment 21•5 years ago
|
||
Hmm that's strange, I tested and I see the audio streams (or channels) collected before closing the tab. Are you sure you are using the latest Nightly? No need to wait for 2 hours, 30 secs is enough. In any case, this will not change anything about this bug. I'll get it fixed.
Reporter | ||
Comment 22•5 years ago
|
||
(In reply to Alex Chronopoulos [:achronop] from comment #21)
Hmm that's strange, I tested and I see the audio streams (or channels) collected before closing the tab. Are you sure you are using the latest Nightly? No need to wait for 2 hours, 30 secs is enough. In any case, this will not change anything about this bug. I'll get it fixed.
Daily i use the stable version or chromium if audio channels go wild, but i conducted the test on the latest nightly
Assignee | ||
Comment 23•5 years ago
|
||
I am dealing with an intermittent that I have not been able to clean up. The run with MediaDecoder logs:
https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=288906079&revision=4a8bbccfa1d8983b499e24e0320d63b18ae049cf
Assignee | ||
Comment 24•5 years ago
|
||
Assignee | ||
Comment 25•5 years ago
|
||
Assignee | ||
Comment 26•5 years ago
|
||
Assignee | ||
Comment 27•5 years ago
|
||
Try run of the patches under review:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c62a63b7bad4dd92d6b6a3ab86571bfe590b3aa4
Assignee | ||
Comment 28•5 years ago
|
||
Track if MediaSink is suspended and disallow to start it or change the sink id. Also, take care of how the suspended state interacts with a DecodedStream. When media capture is requested allow the MediaSink to change to a DecodedStream but starting the sink is restricted until the resume of the sink.
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Comment 29•5 years ago
|
||
mPrincipalHandle
and mPlaying` listeners were being connected in the ctor of a DecodedStream. However, this is not necessary because their attributes will be modified after the sink start. In addition to that, it was causing problems if a sink was replaced before being started or stopped (and shutdown). This is a valid scenario, though, that we need to support.
Assignee | ||
Comment 30•5 years ago
|
||
Implement the MediaDecoder::Suspend/Resume
to shutdown and recreate the MediaSink. This is important the page is refreshed because the existing tree does not shutdown immediatelly. It stays alive but remains suspended thus it is necessary to clean up the low-level system resources. In order to support the suspended state with the existing work flow, if MediaSink is suspended it is disallowed to start it or change the sink id. Recreating the MediaSink is possible but starting it is restricted until resume.
Comment 31•5 years ago
|
||
Comment 32•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/aac6ada5bac3
https://hg.mozilla.org/mozilla-central/rev/cc1790a6b7e4
Updated•5 years ago
|
Comment 33•5 years ago
|
||
Hello,
I can confirm that this issue is verified in Fx 75.0b5 (Build ID: 20200317211402).
Reporter | ||
Comment 34•5 years ago
|
||
(In reply to Daniel Cicas [:dcicas], Release QA from comment #33)
Hello,
I can confirm that this issue is verified in Fx 75.0b5 (Build ID: 20200317211402).
So its not fixed ?
Comment 35•5 years ago
|
||
(In reply to Qik from comment #34)
So its not fixed ?
It's fixed, the comment 33 is about verifying the fix.
Description
•