Open Bug 1638685 Opened 5 years ago Updated 6 months ago

Stop streaming audio when muted

Categories

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

76 Branch
x86_64
Linux
defect

Tracking

()

UNCONFIRMED

People

(Reporter: hugo, Unassigned)

References

Details

User Agent: Mozilla/5.0 (Windows NT 10.0; ) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/83.0.4086.0 Safari/537.36

Steps to reproduce:

Scenario 1

  1. Play a youtube video.
  2. Pause it.

Scenario 2

  1. Mute a tab.
  2. Open a website that autoplays videos with sound (note that firefox actually creates a stream at 0% volume).

Actual results:

The audio stream [when using pulseaudio] is still active (even though there's silence), eg:

$ pactl list sinks | grep -A 3 Sink
Sink #0
	State: RUNNING
	Name: alsa_output.pci-0000_00_1f.3.analog-stereo
	Description: Built-in Audio Analog Stereo

My headphones connect to both my laptop and phone, but since the laptop stream is still running, it won't switch to another device, so I can't play audio with my phone. I need to kill the Firefox process (on my laptop!) to get audio from my phone.

More background on this pulseaudio issue where we narrowed down the cause: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/issues/892

Expected results:

The audio stream should be suspended.

OS: Unspecified → Linux
Hardware: Unspecified → x86_64

(Please ignore User-Agent in the report; I'm spoofing it to get past some websites browser-filtering)

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Component: Untriaged → Audio/Video: cubeb
Product: Firefox → Core

After further detailed analysis, it seems that the problem is that muted tabs have their volume set to 0%, rather than having the stream corked.
This makes pulseaudio keep the stream to headphones (or whatever device is in use) alive.

Because of this, the headphones won't switch to another device, but also apparently don't go into power-saving mode (since they have an active stream).

The fix is for firefox to call pa_stream_cork on a stream when a tab is muted (this is actually what pulseaudio expects when a stream is to be muted).

Component: Audio/Video: cubeb → Audio/Video: Playback

It seems to me that is a cubeb issue. When calling cubeb_stream_set_volume from AudioStream, cubeb should check the audio's volume to decide if we should call something like pa_stream_cork.

Component: Audio/Video: Playback → Audio/Video: cubeb
Flags: needinfo?(achronop)

Note that I'm talking about explicitly muted tabs or paused videos.
This shouldn't really set the volume to zero, but make an explicit mute("cork") call.

cubeb has a cork_io_stream that does exactly this.

The way we mute the tab is to set volume to zero which is a high level concept among all platforms. So the issue you mentioned is the low level detail of how should we do when mute a tab, mute a media or pause a media, so it seems to me that cubeb should consider these conditions at the same time as well.

But I'm curious why the paused media would still have an active stream, calling cubeb_stream_stop seems indicating the stream should be destroyed or at least deactived?

But I'm curious why the paused media would still have an active stream

It's not active: it's corked, which is essentially an equivalent of paused. Corked means that whatever is sent is not played, it's essentially paused/muted. Media players cork the stream when a video is paused.

The dedicated function is there because detecting silence is complex, and there's really no need low level (there's an explicit function to silence/"cork"), nor at a high level (there's also a dedicated mute/pause button button in any GUI).

Why would a mute gui button not end up calling the low-level "mute" (in this case, pa_stream_cork) function?

(In reply to Hugo Osvaldo Barrera from comment #7)

Why would a mute gui button not end up calling the low-level "mute" (in this case, pa_stream_cork) function?

That is because we don't have such a function in general cubeb public method [1]. That is why I think cubeb should handle that when we set the volume to zero.

[1] https://github.com/kinetiknz/cubeb/blob/35190a8da650be297edb91d2db778bed622d8691/include/cubeb/cubeb.h

Oh, the function is not public on cubeb's side, I get it.

They don't seem to call the function aside from when creating or destroying the stream.
Maybe it would make sense for cubeb to expose corking, it sounds like both the easiest and cleanest solution.

Sorry, I should have added more context. I assigned this to Playback mostly for the Pause case. I don't agree with the idea of corking the stream on mute and I will explain why.

The pa_stream_cork is not the appropriate method for being called when the volume changes to 0. It is the method that stops a PA stream, thus it is called from cubeb_stream_stop and it results in stopping the audio thread. This, for example, will result in that the position of the playback file will stop being updated, probably decoding will stop etc, and we don't want that when the stream is muted. Cubeb, as a low-level library, cannot do that. However, a cubeb client can implement something like that, maybe when special circumstances are met (like the ones described here) by using the cubeb_stream_stop instead of cubeb_stream_set_volume and somehow faking the audio thread.

My personal opinion is that we should not do it but I am not the one to decide. I am against it because it seems to be a big change, it is only needed under special conditions which are hard to detect, and on unmute, we would need to restart the device, which is risky.

Regrading the Pause case I would expect that the stream is stopped (corked) but Alastor knows better.

Flags: needinfo?(achronop)

The pause case is the most infrequent one though. The more frequent case that causes issue is some muted backgroud tab.

That scenario is very annoying since it decreases the battery life on headphones, and also prevents me from using them on my phone when I'm not on my computer (workaround right now is to walk to my PC, and kill FIrefox, which is far from optimal). This seems to affect speakers too.

I've inspected how media players work a bit (mostly mpv), and they seem to cork on pause, and completely close the stream on mute. Maybe that would be a good behaviour to imitate?

(In reply to Alex Chronopoulos [:achronop] from comment #10)

Sorry, I should have added more context. I assigned this to Playback mostly for the Pause case. I don't agree with the idea of corking the stream on mute and I will explain why.

According to your comment, calling cubeb_stream_stop should result in calling pa_stream_cork as well. But when we pause media, it would eventually result in calling AudioStream::Pause() where we would call cubeb_stream_stop().

So I wonder why this issue would happen on pause case? Is it because cubeb doesn't call pa_stream_cork correctly?

The pa_stream_cork is not the appropriate method for being called when the volume changes to 0. It is the method that stops a PA stream, thus it is called from cubeb_stream_stop and it results in stopping the audio thread. This, for example, will result in that the position of the playback file will stop being updated, probably decoding will stop etc, and we don't want that when the stream is muted.

I think it can be solved by switching back to the system clock, like this [1]. That would happen when we're playing a video without an audio track.

[1] https://searchfox.org/mozilla-central/rev/61fceb7c0729773f544a9656f474e36cd636e5ea/dom/media/mediasink/AudioSinkWrapper.cpp#62-64

My personal opinion is that we should not do it but I am not the one to decide. I am against it because it seems to be a big change, it is only needed under special conditions which are hard to detect, and on unmute, we would need to restart the device, which is risky.

What I'm thinking about is that will this change help us to reduce the power usage significantly? Because there are a lot of muted autoplay video on the web, sand I'm curious how significant keeping audio stream active to affect battery usage would be? Would you mind to explain the risk of restarting the device?

My concern for making this change is that, is restarting/reactivating the stream fast enough? We don't want to have a noticeable audio delay when we unmute the media.

Flags: needinfo?(achronop)

Hmm... I can't replicate the pause and uncorked scenario, even when having just the one tab.
I've done some harder testing, and apparently paused streams are always corked, it's only muted ones that keep the stream active.

I guess maybe I had some other background tab misbehaving when I did my initial testing?

I think it can be solved by switching back to the system clock, like this [1]. That would happen when we're playing a video without an audio track.

[1] https://searchfox.org/mozilla-central/rev/61fceb7c0729773f544a9656f474e36cd636e5ea/dom/media/mediasink/AudioSinkWrapper.cpp#62-64

Could the not-handling-audio-at-all part of this idea also slightly improve rendering performance?

I'm curious how significant keeping audio stream active to affect battery usage would be?

This would keep bluetooth headphones from going into standby (or LE mode), so the difference is quite big.

I can really tell the difference since keeping them connected to my laptop required me to charge them daily, but I only did around once a week when only using them with my phone (I seldom turn them off, but just let them go into standby). I'm not sure how battery usage is optimised on the laptop side.

Hugo, thanks for retesting the pause case. It is nice that pausing and crocking works as expected.

(In reply to Alastor Wu [:alwu] from comment #12)

I think it can be solved by switching back to the system clock, like this [1]. That would happen when we're playing a video without an audio track.

I did not know that there is a solution in place, it's much easier to support it then.

I'm curious how significant keeping audio stream active to affect battery usage would be?

It's a major improvement for the battery consumption in general, not only for the bluetooth devices. In some cases, when the pulse daemon is configured to use flat volumes, we apply our own volume instead of changing the volume in the device. That means that we keep delivering audio buffers (full of zeros in case of a muted stream) to the soundcard and the consumption is similar to the normal playback.

Would you mind to explain the risk of restarting the device?

The risk is that the device will become idle so if anything happens to the device in between mute/unmute, it will cause the state of the playback stream to be out of date. In other words, we must be very careful to keep the state of the stream updated according to what happens to the device. Another thing is that unmuting will reach the hardware which can always fail and I don't know, from the user perspective, how acceptable it is to fail to unmute a playback stream. Finally, as you already have thought, time can be an issue. Starting/stopping my internal device is fast enough but I don't know what happens on external devices and especially on bluetooth devices. You can try pausing/playing to get an idea.

Flags: needinfo?(achronop)

(In reply to Alex Chronopoulos [:achronop] from comment #14)

It's a major improvement for the battery consumption in general, not only for the bluetooth devices. In some cases, when the pulse daemon is configured to use flat volumes, we apply our own volume instead of changing the volume in the device. That means that we keep delivering audio buffers (full of zeros in case of a muted stream) to the soundcard and the consumption is similar to the normal playback.

It sounds a good thing to do. Is it benefit for all platforms?

The risk is that the device will become idle so if anything happens to the device in between mute/unmute, it will cause the state of the playback stream to be out of date. In other words, we must be very careful to keep the state of the stream updated according to what happens to the device. Another thing is that unmuting will reach the hardware which can always fail and I don't know, from the user perspective, how acceptable it is to fail to unmute a playback stream. Finally, as you already have thought, time can be an issue. Starting/stopping my internal device is fast enough but I don't know what happens on external devices and especially on bluetooth devices. You can try pausing/playing to get an idea.

If the original device is closed during muting, I assume that we would route the audio to another available device when unmuting the audio? Letting device being idle can reduce its battery usage, but I wonder if there is any drawback which might be caused by being idle? How to verify if the audio stream is active or not, I would like to check how other browsers behave on this situation. What kind of solution you think we should do? should AudioStream call cubeb_stream_stop explicitly when the stream is muted? or we want to do something inside Cubeb? Do we want to stop the stream whenever the audio becomes inaudible? or we want to have a time threshold, like the dormant mechanism in MDSM to avoid the possibility of opeing/closing stream within a short period of time?

Flags: needinfo?(achronop)

If the original device is closed during muting, I assume that we would route the audio to another available device when unmuting the audio?

FWIW, this is what happens if I disconnect my headphones mid-playback. Firefox does not pause when the device is disconnected.

It's also what happens if I pause/disconnect/resume, so this approach seems consistent enough in all ways.

Sounds like we're agreed that cubeb doesn't seem to be the right place to make the decision re whether jitters from switching time source are acceptable.
I'm somewhat surprised if video players typically switch time sources every time the volume touches or leaves zero.
Switching time sources on resume may be harder than switching on end of audio, but if those can be seamless and resume delays are not too slow, then corking the stream after some period of zero volume may be a option.

Switching time sources would be fine for background tabs at least, and other media without visible video. Perhaps that's a good place to start, and most of the win for battery, but if we want a heuristic based on the time muted, then the visibility detection would not be required.

Severity: -- → S3
Summary: Stop streaming audio when nothing is playing → Stop streaming audio when muted

(In reply to Alastor Wu [:alwu] from comment #15)

Is it benefit for all platforms?

Yeah it must be beneficial for every platform, if we want to support it would be good to have it everywhere. The risk of unplugged or disabled devices on the desktop platforms is higher but again it would be more than good to have it everywhere.

If the original device is closed during muting, I assume that we would route the audio to another available device when unmuting the audio?

Yes, this is supported in all desktop platforms. However, many bugs have been discovered in that area and this is mostly because the devices out there are too many and unpredictable. For example, the notifications might arrive with a lag or in an unexpected order or a device can go silent without reporting an error, etc. Don't get me wrong, I am not against it, I just want to provide the full picture.

Letting device being idle can reduce its battery usage, but I wonder if there is any drawback which might be caused by being idle?

The device will be stopped but not deleted. Which means we don't have to re-initialize it, just to start it. The stream is there, all the way down to the system level but the audio thread is not iterating. I am not sure if that means the same for every device/platform.

How to verify if the audio stream is active or not, I would like to check how other browsers behave on this situation.

On Linux you can use pavucontrol a small tool to see the system streams. When the stream is stopped but not deleted, the stream is still listed but the visual indicator of the audio level remains inactive. When the stream is deleted, that stream disappears from the list. If you ping me we can go through an example together.

What kind of solution you think we should do? should AudioStream call cubeb_stream_stop explicitly when the stream is muted? or we want to do something inside Cubeb?

If we added something in cubeb it will be equivalent to cubeb_stream_stop and it would create one more special case that any cubeb's client should know about. In addition to that, it would not sort out the whole situation. So my opinion is that it would not be in the right direction to put something in there.

In my opinion when the volume goes to zero, we should call cubeb_stream_stop and when it is unmuted cubeb_stream_start. In parallel we must be registered for CUBEB_STATE change callback. If a state error is reported, when in mute, probably (you might know better here) we must close the playback. Alternatively, if we are in the system clock so the user does not 'feel' the error, we can attempt to reinitialize the stream, without starting it. Also those state change callbacks, including the CUBEB_STATE_START and CUBEB_STATE_STOP on mute unmute respectively must not be handed to the AudioStream because it relies on those to make decisions about the state of the playback. In addition to that, we must have an alternative behavior if cubeb_stream_stop fails on mute, or cubeb_stream_start fails on unmute. In the first case, we could call the regular volume API. Btw this would increase the complexity to separate the case. In the second case, there is not much we could do. We can attempt to reinitialize the stream but it will take some time, and if that fails, I guess, the playback should fail/stop too. Feel free to add your thoughts on that.

For the device change, plug, or unplug cases the situation is better. In general, the way playback configures cubeb, it allows for the output device to change automatically to the next default, and to remain ready to be started. However, if the playback code needs to know the updated device id, there are more things to be done. Let me know if that's the case.

Do we want to stop the stream whenever the audio becomes inaudible? or we want to have a time threshold, like the dormant mechanism in MDSM to avoid the possibility of opening/closing stream within a short period of time?

Well, I am not the best to comment on that depends on what you want. In general, it would make sense, it would reduce the risk since it would reduce the number of times the hardware is accessed with the cost of increasing the complexity.

Flags: needinfo?(achronop)

Not sure if this is pushing the change too much, maybe videos on muted background/non-visible tabs should be entirely stopped?

There's a bigger battery saving there in general, avoids any interaction with the sound subsystem (but also GPU), and I can't see a scenario where this would be problematic.

How to verify if the audio stream is active or not, I would like to check how other browsers behave on this situation.

On Linux you can use pavucontrol a small tool to see the system streams. When the stream is stopped but not deleted, the stream is still listed but the visual indicator of the audio level remains inactive. When the stream is deleted, that stream disappears from the list. If you ping me we can go through an example together.

Be careful when using pavucontrol for this. pavucontrol actively monitors streams by opening them, and this prevents them from going into idle. Effectively, when debugging this issue initially, having pavucontrol open affected the outcome of my experiments.

I made a small python script to list devices/application/streams and their idle/corked status, it's in the original upstream issue: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/issues/892#note_502663

See Also: → 1165677
You need to log in before you can comment on or make changes to this bug.