[pulseaudio-discuss] [PATCH] sink, source: Add a mode to avoid resampling if possible

[Tanu Kaskinen]

On Sun, 2017-01-29 at 10:29 +0530, Arun Raghavan wrote:
> This adds an "avoid-resampling" option to daemon.conf that makes the
> daemon try to use the stream sample rate if possible (the device needs
> to support it, which currently only ALSA does), and there should not be
> any other stream connected).
> 
> This should enable some of the "audiophile" use-cases where users wish
> to play high sample rate audio files without resampling.
> 
> We still will do conversion if sample formats don't match, though. This
> means that if you want to play 96 kHz/24 bit audio without any
> modification the default format will need to be set to be 24-bit as
> well. This will force all streams to be upconverted, which, other than
> the wasted resources, should be relatively harmless.
> ---
>  man/pulse-daemon.conf.5.xml.in |  9 +++++++++
>  src/daemon/daemon-conf.c       |  3 +++
>  src/daemon/daemon-conf.h       |  1 +
>  src/daemon/daemon.conf.in      |  1 +
>  src/daemon/main.c              |  1 +
>  src/pulsecore/core.h           |  1 +
>  src/pulsecore/sink.c           | 10 ++++++++--
>  src/pulsecore/source.c         |  9 +++++++--
>  8 files changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/man/pulse-daemon.conf.5.xml.in b/man/pulse-daemon.conf.5.xml.in
> index b81a549..cb09b97 100644
> --- a/man/pulse-daemon.conf.5.xml.in
> +++ b/man/pulse-daemon.conf.5.xml.in
> @@ -124,6 +124,15 @@ License along with PulseAudio; if not, see <http://www.gnu.org/licenses/>.
>      </option>
>  
>      <option>
> +      <p><opt>avoid-resampling=</opt> If set, try to configure the
> +      device to avoid resampling. This only works if on devices that

The extra "if" is still here.

> @@ -1442,7 +1443,12 @@ int pa_sink_update_rate(pa_sink *s, uint32_t rate, bool passthrough) {
>      if (PA_UNLIKELY(!pa_sample_rate_valid(desired_rate)))
>          return -1;
>  
> -    if (!passthrough && default_rate != desired_rate && alternate_rate != desired_rate) {
> +    if (avoid_resampling && (rate >= default_rate || rate >= alternate_rate)) {
> +        /* We just try to set the sink input's sample rate if it's not too low */
> +        if (rate > default_rate || rate > alternate_rate)
> +            desired_rate = rate;

Now the code works, which is nice. However, you said that the
"desired_rate = rate" assignment makes the code clearer, but I
disagree. The code looks like the intention is to do something if the
"rate > default_rate || rate > alternate_rate" condition is true, but
actually the code never does anything, regardless of whether the
condition is true or false.

If you think leaving the code block empty or merging the "if" and "else
if" branches makes the code unclear, then I think a comment is more
appropriate way to clarify things than adding code that doesn't do
anything.

-- 
Tanu

https://www.patreon.com/tanuk