Bug 97777 - Clipping with per-stream volume above 100% even though device volume below 100%
Summary: Clipping with per-stream volume above 100% even though device volume below 100%
Status: RESOLVED MOVED
Alias: None
Product: PulseAudio
Classification: Unclassified
Component: daemon (show other bugs)
Version: unspecified
Hardware: x86-64 (AMD64) Linux (All)
: medium normal
Assignee: pulseaudio-bugs
QA Contact: pulseaudio-bugs
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-09-12 14:03 UTC by Niklas Haas
Modified: 2018-07-30 10:07 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Niklas Haas 2016-09-12 14:03:32 UTC
I am doing the following:

1. Set my device volume to about 70% (approx. -10 dB gain)
2. Play a sine wave at -0 dBFS using a simple test client:

λ import Sound.Pulse.Simple 
λ p <- simpleNew Nothing "pulse-simple" Play Nothing "testing" (SampleSpec (F32 LittleEndian) 48000 2) Nothing Nothing
λ simpleWrite p [ 1.0 * sin (w*x) | x <- [0..48000*5], let w = 0.01 :: Float ]
λ ...
λ simpleFree p

3. Change the per-stream volume for this stream (“pulse-simple” in pavucontrol)

Any value below 100% is fine, but as soon as I go above 100% even by a little bit (e.g. 101%), I immediately hear obvious clipping artifacts.

4. Set the per-stream volume to 100%, and change the amplitude of the sine wave produced instead (e.g. changing 1.0 to 1.1). I again hear obvious clipping artifacts (even though the signal format is floating point).

Why is this the case? Shouldn't the overall volume of 70% give me more than enough headroom to go above 100% on the per-stream volume? The only explanation I can come up with is that the conversion to integer happens before the device volume adjustment, e.g. perhaps if the device volume is being applied by ALSA instead of by PulseAudio (since it does seem that changes to the device volume are also visible in alsamixer).

If this is indeed the case and this is a limitation of the way device volumes are implemented, would it be possible to apply some sort of global “gain” setting to all sinks instead? I want to give them all 10 dB or so of mixing headroom so I can use software volume controls to go above 100% without clipping.

Obviously I can achieve this by setting up some sort of LADPSA filter or whatever in the signal path of every single stream, but that seems like a hassle to set up properly (especially since I'm using multiple audio devices). If it would be possible to just add a simple “mixing headroom” variable to daemon.conf that I can set to get headroom for free, I would be a very happy man.
Comment 1 Niklas Haas 2016-09-13 15:04:02 UTC
An update:

I realized that if I set my device profile in pavucontrol's configuration tab from “Analog Output” to “Digital Output”, pulseaudio will stop using the device's built-in output volume adjustment and instead use software volume adjustment.

This solves #3 of my problem, meaning I can now set my device volume to -10 dB, and my stream to +10dB, and get no clipping artifacts.

However, it leaves #4 of my issue, which is that out-of-range floats are clipped by PulseAudio (or if not pulseaudio, then the pulseaudio client). It does seem like the signal format is to be described as explicitly being in [-1,1] even for floats in the documentation here:
https://freedesktop.org/software/pulseaudio/doxygen/sample_8h.html#a41051ceaa5cfbe60c9b176deb7bfed0e

Could that restriction be conceivably dropped? I would love to be able to send out-of-range floats and have PulseAudio mix them without clipping.
Comment 2 Tanu Kaskinen 2016-09-13 16:33:57 UTC
Regarding the "unnecessary" clipping of float data: since we talk to alsa using integers, the sink works in integer mode (you can see the sink sample spec in "pactl list sinks"). When the audio moves from the sink input (representing the application stream), the data has to be converted to integers. Since 1.1 isn't representable in the int16 range, clipping occurs. The sink volume hasn't yet been applied at this point.

If the sink used floats too, I think the audio wouldn't get clipped.

I suppose we could reorder the processing so that sink and stream volumes are applied to the stream audio before we convert the audio to ints and mix it with other streams. This would mean applying the sink volume multiple times if there are multiple streams, though, whereas the current code only applies the sink volume once to the mix result.

The case where you successfully play a stream with +10dB volume to a sink with -10dB volume without clipping is a case where we skip the stream volume application step and merge the stream volume with the sink volume when the sink volume is applied. I think you'd get clipping if there was another stream playing at the same time, because in this situation the stream and sink volume application has to be two separate steps.

Your goal is to be able to increase the volume of a specific stream higher than other streams, without risk of clipping, right? This could be solved by adding some headroom in the mixing phase as you suggested. The processing order would have to be changed too, as explained above. Another solution would be to set the default stream volume to something less than 100%. That would be simpler. The two solutions aren't equivalent, however: do you wish that the volume of the loud stream is remembered the next time the application starts to play, or should it revert to the default volume? We cap the remembered volume to 100%, so the "mixing headroom" solution is good if you don't want to remember the volume, and the "lower default volume" solution is good if you want to remember the volume.
Comment 3 Niklas Haas 2016-09-13 16:54:02 UTC
> If the sink used floats too, I think the audio wouldn't get clipped.

I could try that, although I'm not quite sure how. The ALSA sink doesn't support float formats natively, so I'd need a conversion plugin (`type plug`) in the signal path - and I'm not sure how to get PulseAudio to include one when opening the ALSA device.

> I suppose we could reorder the processing so that sink and stream volumes are applied to the stream audio before we convert the audio to ints and mix it with other streams. This would mean applying the sink volume multiple times if there are multiple streams, though, whereas the current code only applies the sink volume once to the mix result.

I'm confused by this. Isn't resampling and mixing done on floats rather than integers?

> The case where you successfully play a stream with +10dB volume to a sink with -10dB volume without clipping is a case where we skip the stream volume application step and merge the stream volume with the sink volume when the sink volume is applied. I think you'd get clipping if there was another stream playing at the same time, because in this situation the stream and sink volume application has to be two separate steps.

I tried playing multiple streams simultaneously, and I still get no clipping from pulse-test until the moment I cross the +10 dB threshold, after which I get immediate and obvious clipping. (My device is still set to -10 dB)

> Your goal is to be able to increase the volume of a specific stream higher than other streams, without risk of clipping, right? 

Exactly.

> Another solution would be to set the default stream volume to something less than 100%. That would be simpler. The two solutions aren't equivalent, 

They both solve my use case as far as I'm concerned.

> however: do you wish that the volume of the loud stream is remembered the next time the application starts to play, or should it revert to the default volume? We cap the remembered volume to 100%, so the "mixing headroom" solution is good if you don't want to remember the volume, and the "lower default volume" solution is good if you want to remember the volume.

Depends on the application, actually. For MPD, yes, for mpv no. If it's possible to set the default volume for never-before-seen streams to 70% or so (instead of 100%), I would be perfectly happy since I consider that a completely acceptable solution, and it would also allow volume-remembering to continue working. It would also allow me to use software volume controls in mpv without running into any clipping-related issues, since 100% in mpv would again map to the maximum possible volume (rather than 140% in the mixing-headroom approach).
Comment 4 Tanu Kaskinen 2016-09-13 20:34:59 UTC
(In reply to Niklas Haas from comment #3)
> > If the sink used floats too, I think the audio wouldn't get clipped.
> 
> I could try that, although I'm not quite sure how. The ALSA sink doesn't
> support float formats natively, so I'd need a conversion plugin (`type
> plug`) in the signal path - and I'm not sure how to get PulseAudio to
> include one when opening the ALSA device.

You could hack /usr/share/pulseaudio/alsa-mixer/profiles/default.conf. For example, the analog stereo sink and source have this configuration:

[Mapping analog-stereo]
device-strings = front:%f hw:%f
channel-map = left,right
paths-output = analog-output analog-output-lineout analog-output-speaker analog-output-headphones analog-output-headphones-2
paths-input = analog-input-front-mic analog-input-rear-mic analog-input-internal-mic analog-input-dock-mic analog-input analog-input-mic analog-input-linein analog-input-aux analog-input-video analog-input-tvtuner analog-input-fm analog-input-mic-line analog-input-headphone-mic analog-input-headset-mic
priority = 10

Replacing front:%f with plughw:%f would probably work. ("%f" is the card index or name.)

To configure the sample format, you need to set "default-sample-format = float32ne" in /etc/pulse/daemon.conf ("ne" in "float32ne" means native endianness. "le" and "be" can be used too for little and big endianness).

> > I suppose we could reorder the processing so that sink and stream volumes
> > are applied to the stream audio before we convert the audio to ints and mix
> > it with other streams. This would mean applying the sink volume multiple
> > times if there are multiple streams, though, whereas the current code only
> > applies the sink volume once to the mix result.
> 
> I'm confused by this. Isn't resampling and mixing done on floats rather than
> integers?

Internally resamplers will use whatever they want, and if a resampler uses integers internally, the resampling will destroy float values outside the normal range, even if both the sink and the stream use floats. We don't have any clever logic that would pick the resampler implementation based on the sink and stream format (and I'm not sure that would even be desirable, because we don't have clear pairs of resamplers with equivalent speed and quality that would only differ in their internal sample format).

Mixing is done with the sink sample format.

> > The case where you successfully play a stream with +10dB volume to a sink
> > with -10dB volume without clipping is a case where we skip the stream
> > volume application step and merge the stream volume with the sink volume
> > when the sink volume is applied. I think you'd get clipping if there was
> > another stream playing at the same time, because in this situation the
> > stream and sink volume application has to be two separate steps.
> 
> I tried playing multiple streams simultaneously, and I still get no clipping
> from pulse-test until the moment I cross the +10 dB threshold, after which I
> get immediate and obvious clipping. (My device is still set to -10 dB)

It seems that I was wrong about how we do things. Having multiple streams doesn't prevent merging the two volume application steps into one. The thing that prevents the merging is having a mismatch in channel maps. If you just swap left and right channels in your stream's channel map, I expect you to get clipping.

> > however: do you wish that the volume of the loud stream is remembered the
> > next time the application starts to play, or should it revert to the
> > default volume? We cap the remembered volume to 100%, so the "mixing
> > headroom" solution is good if you don't want to remember the volume, and
> > the "lower default volume" solution is good if you want to remember the
> > volume.
> 
> Depends on the application, actually. For MPD, yes, for mpv no. If it's
> possible to set the default volume for never-before-seen streams to 70% or
> so (instead of 100%), I would be perfectly happy since I consider that a
> completely acceptable solution, and it would also allow volume-remembering
> to continue working. It would also allow me to use software volume controls
> in mpv without running into any clipping-related issues, since 100% in mpv
> would again map to the maximum possible volume (rather than 140% in the
> mixing-headroom approach).

module-match can almost do what you want. The only problem is that it overrides volumes set by module-stream-restore. See [1] for documentation. Adding a new configuration option to change the hook callback priority should be pretty trivial (the module uses a hook to get notified of new streams, and the callback priorities decide the ordering between module-match and module-stream-restore).

[1] https://wiki.freedesktop.org/www/Software/PulseAudio/Documentation/User/Modules/#index53h3
Comment 5 Niklas Haas 2016-09-14 00:46:33 UTC
> Internally resamplers will use whatever they want, and if a resampler uses
> integers internally, the resampling will destroy float values outside the
> normal range, even if both the sink and the stream use floats. We don't have
> any clever logic that would pick the resampler implementation based on the
> sink and stream format (and I'm not sure that would even be desirable, because
> we don't have clear pairs of resamplers with equivalent speed and quality that
> would only differ in their internal sample format).

Makes sense. For the record, I am using soxr-vhq.

> module-match can almost do what you want. The only problem is that it
> overrides volumes set by module-stream-restore. See [1] for documentation.
> Adding a new configuration option to change the hook callback priority should
> be pretty trivial (the module uses a hook to get notified of new streams, and
> the callback priorities decide the ordering between module-match and
> module-stream-restore).

Indeed it does, and it does work well for me, apart from the part where it
overrides stream restoring. On the subject of making it configurable, it feels
sort of “weird” to have this filter expose its hook priority as a user
configurable variable, and using the filter also seem like a bit of abuse.

If we're touching source code, wouldn't it be better to add an option to
module-stream-restore indicating what volume to pick for streams that it fails
observing? Or perhaps adding a new `module-default-volume` that runs after
everything else and just sets a configurable volume if unset.

That being said, module-match being capable of running before
module-stream-restore definitely seems like a useful feature to have regardless,
because I might want to set different default volumes based on the title (but
still retain the ability to make changes of my own).

P.s. I wonder if it would be possible to get module-match to detect changes to
the match table in realtime, although for that I suppose I could use inotify +
unload-module && load-module.

Anyway, how do you propose exposing this to the user? I noticed while digging
through the source that there's a pa_sink_input_new_data->volume_is_set bool,
maybe the right way to approach this would be to add a ‘force’ option to
module_stream_restore which forcibly applies saved volumes even if volume_is_set
was already true by the time the fixate hook got called?
Comment 6 Tanu Kaskinen 2016-09-14 08:47:45 UTC
(In reply to Niklas Haas from comment #5)
> > module-match can almost do what you want. The only problem is that it
> > overrides volumes set by module-stream-restore. See [1] for documentation.
> > Adding a new configuration option to change the hook callback priority
> > should be pretty trivial (the module uses a hook to get notified of new
> > streams, and the callback priorities decide the ordering between
> > module-match and module-stream-restore).
> 
> Indeed it does, and it does work well for me, apart from the part where it
> overrides stream restoring. On the subject of making it configurable, it
> feels sort of “weird” to have this filter expose its hook priority as a user
> configurable variable, and using the filter also seem like a bit of abuse.

Yes, exposing the hook priority to users is awkward, but I couldn't think of better ways to configure the ordering between module-match and module-stream-restore.

> If we're touching source code, wouldn't it be better to add an option to
> module-stream-restore indicating what volume to pick for streams that it
> fails observing?

That sounds quite good.

> Or perhaps adding a new `module-default-volume` that runs
> after everything else and just sets a configurable volume if unset.

That's certainly a possibility, but having a separate module for such a small bit of functionality doesn't feel very nice.

Another possibility would be to add an option for default stream volume to daemon.conf.

Currently I like the idea of adding an option to module-stream-restore the most.

> That being said, module-match being capable of running before
> module-stream-restore definitely seems like a useful feature to have
> regardless, because I might want to set different default volumes based on
> the title (but still retain the ability to make changes of my own).
> 
> P.s. I wonder if it would be possible to get module-match to detect changes
> to the match table in realtime, although for that I suppose I could use
> inotify + unload-module && load-module.

My preferred method of realtime configuration changes is to use pactl or other client (the infrastructure for communicating between clients and individual modules sucks currently, though). That is, no manual editing of configuration files. That said, reloading configuration files could still be useful for distribution maintainers, if for example installing some package causes configuration changes, so I don't think I'd oppose a patch that implements configuration reloading.

I'm a bit concerned that automatic reloading could cause trouble when updating pulseaudio, if the update brings new configuration that is not supported by the currently-running daemon process. Requiring manual action ("pactl reload-configuration" or SIGHUP) might be a good idea.

> Anyway, how do you propose exposing this to the user? I noticed while digging
> through the source that there's a pa_sink_input_new_data->volume_is_set bool,
> maybe the right way to approach this would be to add a ‘force’ option to
> module_stream_restore which forcibly applies saved volumes even if
> volume_is_set was already true by the time the fixate hook got called?

I don't think that would be much better than having a hook priority option in module-match, because module-stream-restore can't reliably force anything that happens in hook callbacks after module-stream-restore's own callback. Explaining the semantics accurately to users would be as problematic as explaining the hook priorities. Adding a default volume option to module-stream-restore seems like a better solution.
Comment 7 GitLab Migration User 2018-07-30 10:07:35 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/issues/217.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.