Skip to content
Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Unreleased

- ALSA(process_output): Pass `silent=true` to `PCM.try_recover`, so it doesn't write to stderr.
- ALSA: Fix buffer and period size selection by rounding to supported values. Actual buffer size may be different from the requested size or may be a device-specified default size. Additionally sets ALSA "periods" to 2 (previously 4). (error 22)
- CoreAudio: `Device::supported_configs` now returns a single element containing the available sample rate range when all elements have the same `mMinimum` and `mMaximum` values (which is the most common case).
- CoreAudio: Detect default audio device lazily when building a stream, instead of during device enumeration.
- iOS: Fix example by properly activating audio session.
Expand Down
158 changes: 139 additions & 19 deletions src/host/alsa/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@ use self::alsa::poll::Descriptors;
use crate::traits::{DeviceTrait, HostTrait, StreamTrait};
use crate::{
BackendSpecificError, BufferSize, BuildStreamError, ChannelCount, Data,
DefaultStreamConfigError, DeviceNameError, DevicesError, InputCallbackInfo, OutputCallbackInfo,
PauseStreamError, PlayStreamError, SampleFormat, SampleRate, StreamConfig, StreamError,
SupportedBufferSize, SupportedStreamConfig, SupportedStreamConfigRange,
DefaultStreamConfigError, DeviceNameError, DevicesError, FrameCount, InputCallbackInfo,
OutputCallbackInfo, PauseStreamError, PlayStreamError, SampleFormat, SampleRate, StreamConfig,
StreamError, SupportedBufferSize, SupportedStreamConfig, SupportedStreamConfigRange,
SupportedStreamConfigsError,
};
use std::cell::Cell;
use std::cmp;
use std::convert::TryInto;
use std::ops::RangeInclusive;
use std::sync::{Arc, Mutex};
use std::thread::{self, JoinHandle};
use std::time::Duration;
Expand All @@ -25,6 +26,9 @@ pub type SupportedOutputConfigs = VecIntoIter<SupportedStreamConfigRange>;

mod enumerate;

const VALID_BUFFER_SIZE: RangeInclusive<alsa::pcm::Frames> =
1..=FrameCount::MAX as alsa::pcm::Frames;

/// The default linux, dragonfly, freebsd and netbsd host type.
#[derive(Debug)]
pub struct Host;
Expand Down Expand Up @@ -413,12 +417,10 @@ impl Device {
})
.collect::<Vec<_>>();

let min_buffer_size = hw_params.get_buffer_size_min()?;
let max_buffer_size = hw_params.get_buffer_size_max()?;

let (min_buffer_size, max_buffer_size) = hw_params_buffer_size_min_max(&hw_params);
let buffer_size_range = SupportedBufferSize::Range {
min: min_buffer_size as u32,
max: max_buffer_size as u32,
min: min_buffer_size,
max: max_buffer_size,
};

let mut output = Vec::with_capacity(
Expand Down Expand Up @@ -1040,6 +1042,35 @@ impl StreamTrait for Stream {
}
}

// Overly safe clamp because alsa Frames are i64
fn clamp_frame_count(buffer_size: alsa::pcm::Frames) -> FrameCount {
buffer_size.clamp(1, FrameCount::MAX as _) as _
}

fn hw_params_buffer_size_min_max(hw_params: &alsa::pcm::HwParams) -> (FrameCount, FrameCount) {
let min_buf = hw_params
.get_buffer_size_min()
.map(clamp_frame_count)
.unwrap_or(1);
let max_buf = hw_params
.get_buffer_size_max()
.map(clamp_frame_count)
.unwrap_or(FrameCount::MAX);
(min_buf, max_buf)
}

fn hw_params_period_size_min_max(hw_params: &alsa::pcm::HwParams) -> (FrameCount, FrameCount) {
let min_buf = hw_params
.get_period_size_min()
.map(clamp_frame_count)
.unwrap_or(1);
let max_buf = hw_params
.get_period_size_max()
.map(clamp_frame_count)
.unwrap_or(FrameCount::MAX);
(min_buf, max_buf)
}

fn set_hw_params_from_format(
pcm_handle: &alsa::pcm::PCM,
config: &StreamConfig,
Expand Down Expand Up @@ -1104,24 +1135,113 @@ fn set_hw_params_from_format(
hw_params.set_rate(config.sample_rate.0, alsa::ValueOr::Nearest)?;
hw_params.set_channels(config.channels as u32)?;

match config.buffer_size {
BufferSize::Fixed(v) => {
hw_params.set_period_size_near((v / 4) as alsa::pcm::Frames, alsa::ValueOr::Nearest)?;
hw_params.set_buffer_size(v as alsa::pcm::Frames)?;
}
BufferSize::Default => {
// These values together represent a moderate latency and wakeup interval.
// Without them, we are at the mercy of the device
hw_params.set_period_time_near(25_000, alsa::ValueOr::Nearest)?;
hw_params.set_buffer_time_near(100_000, alsa::ValueOr::Nearest)?;
}
if !set_hw_params_periods(&hw_params, config.buffer_size) {
return Err(BackendSpecificError {
description: format!(
"Buffer size '{:?}' is not supported by this backend",
config.buffer_size
),
});
}

pcm_handle.hw_params(&hw_params)?;

Ok(hw_params.can_pause())
}

/// Returns true if the periods were reasonably set. A false result indicates the device default
/// configuration is being used.
fn set_hw_params_periods(hw_params: &alsa::pcm::HwParams, buffer_size: BufferSize) -> bool {
const FALLBACK_PERIOD_TIME: u32 = 25_000;

// TODO: When the API is made available, this could rely on snd_pcm_hw_params_get_periods_min
// and snd_pcm_hw_params_get_periods_max
const PERIOD_COUNT: u32 = 2;

// Restrict the configuration space to contain only one periods count
hw_params
.set_periods(PERIOD_COUNT, alsa::ValueOr::Nearest)
.unwrap_or_default();

let Some(period_count) = hw_params
.get_periods()
.ok()
.filter(|&period_count| period_count > 0)
else {
return false;
};

/// Returns true if the buffer size was reasonably set.
///
/// The buffer is a ring buffer. The buffer size always has to be greater than one period size.
/// Commonly this is 2*period size, but some hardware can do 8 periods per buffer. It is also
/// possible for the buffer size to not be an integer multiple of the period size.
///
/// See: https://www.alsa-project.org/wiki/FramesPeriods
fn set_hw_params_buffer_size(
hw_params: &alsa::pcm::HwParams,
period_count: u32,
mut buffer_size: FrameCount,
) -> bool {
buffer_size = {
let (min_buffer_size, max_buffer_size) = hw_params_buffer_size_min_max(hw_params);
buffer_size.clamp(min_buffer_size, max_buffer_size)
};

// Desired period size
let period_size = {
let (min_period_size, max_period_size) = hw_params_period_size_min_max(hw_params);
(buffer_size / period_count).clamp(min_period_size, max_period_size)
};

// Actual period size
let Ok(period_size) =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As final polish this would be even more resilient:

// Try to set desired period size
let period_size = if let Ok(size) = hw_params.set_period_size_near(period_size as _, alsa::ValueOr::Greater) {
    size  // Use the size we successfully set
} else {
    // If setting fails, try to get whatever period size the device currently has
    if let Ok(current_size) = hw_params.get_period_size() {
        current_size  // Work with device's current period size
    } else {
        return false;  // Only fail if we can't even query the current size
    }
};

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, though I marked the review as “changes requested”, keen to know what you think. It has the upside of pretty much always working, but the downside of potentially ignoring the user’s request.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was been incorporated in b710e04, but after rethinking it I don't think this check is required.

Reasoning: The sub-function set_hw_params_buffer_size takes a buffer size and attempts to fit the desired number of periods. If this fails, the fallback in the outer function already gets the period size and attempts to set valid parameters using a period-time approach, thereby handling the proposed check.

Also:

  • I realized snd_pcm_hw_params_set_periods was only being called in the absolute fallback case, so the desired count of two was never actually respected (fixed)
  • It's possible to make set_hw_params_periods infallible: if it fails it is already ignored anyways (not fixed)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been thinking about this as well - thanks for sticking through this difficult undertaking of getting the period / buffer system right.

We need to have a behavior contract that's clear to what a user would reasonable expect:

  • BufferSize::Fixed(n) → "I need around n frames, fail if you can't get close"
  • BufferSize::Default → "Give me something reasonable for this hardware"

Right now we might be too accommodating for Fixed(n) and succeed even when we request 128 frames and fall back to 2048. By analogy, we also fail opening a 48 kHz stream when a device only supports 44.1 or 96 kHz.

Should be easy to change, right?

  if let BufferSize::Fixed(val) = buffer_size {
      if set_hw_params_buffer_size(hw_params, period_count, val) {
          return true;
      }
      // Currently falls through to fallback
  }

⬇️

  if let BufferSize::Fixed(val) = buffer_size {
      if set_hw_params_buffer_size(hw_params, period_count, val) {
          return true;
      }
      return false; // Fail immediately - don't use fallback
  }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good - the code has been adjusted to return BackendSpecificError in case it cannot set the requested buffer size (default or fixed).

In the future I think it would be helpful to return a better error type that programs could inspect and use to make decisions, because the string-based error is only helpful for the programmer. The log crate would be handy here.

hw_params.set_period_size_near(period_size as _, alsa::ValueOr::Greater)
else {
return false;
};

let Ok(buffer_size) =
hw_params.set_buffer_size_near(period_size * period_count as alsa::pcm::Frames)
else {
return false;
};

// Double-check the set size is within the CPAL range
VALID_BUFFER_SIZE.contains(&buffer_size)
}

if let BufferSize::Fixed(val) = buffer_size {
return set_hw_params_buffer_size(hw_params, period_count, val);
}

if hw_params
.set_period_time_near(FALLBACK_PERIOD_TIME, alsa::ValueOr::Nearest)
.is_err()
{
return false;
}

let period_size = if let Ok(period_size) = hw_params.get_period_size() {
period_size
} else {
return false;
};

// We should not fail if the driver is unhappy here.
// `default` pcm sometimes fails here, but there no reason to as we attempt to provide a size or
// minimum number of periods.
let Ok(buffer_size) =
hw_params.set_buffer_size_near(period_size * period_count as alsa::pcm::Frames)
else {
return hw_params.set_buffer_size_min(1).is_ok()
&& hw_params.set_buffer_size_max(FrameCount::MAX as _).is_ok();
};

// Double-check the set size is within the CPAL range
VALID_BUFFER_SIZE.contains(&buffer_size)
}

fn set_sw_params_from_format(
pcm_handle: &alsa::pcm::PCM,
config: &StreamConfig,
Expand Down
Loading