use set_buffer_size_near to calculate fixed alsa buffer size #990

attackgoat · 2025-07-23T03:47:52Z

Fixes #743
Closes #917

Hardware parameters cannot be set to some arbitrary values within the supported range.

Two alternate fixes are:

Expose the "near" ALSA function to the user
Return the actual buffer size used

I am not familiar enough with CPAL usage to know if the alternate fixes are needed, but for my use cases they are not.

roderickvd · 2025-07-27T21:13:14Z

Agreed, this is the right thing to do. This highly related commit in librespot actually combines the two approaches you mention: librespot-org/librespot@d5efb8a

roderickvd · 2025-07-29T20:18:16Z

@attackgoat as I became a maintainer today, and agree with your approach, I would like to work with you to getting to a mergable state. Could you update it to:

take the device default when the "near" function fails? (look at the librespot example for inspiration)
add a changelog entry

Thanks!

roderickvd · 2025-07-30T20:52:41Z

I see that #917 also does this (and more).

attackgoat · 2025-07-31T13:50:21Z

I see that #917 also does this (and more).

That PR seems to be the better direction, although it causes me the same issue with large buffers. For now I've addressed the feedback but we may want to close this PR if #917 is cleaned up.

roderickvd · 2025-08-03T20:35:08Z

Thanks a lot @attackgoat. The author of #917 expressed that he is no longer working on it. Would you be willing to merge the two approaches?

roderickvd · 2025-08-03T20:39:06Z

src/host/alsa/mod.rs

+    if let BufferSize::Default = buffer_size {
+        // 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)?;


I'm questioning whether an error here should immediately generate a function result. As you've found, there may be reasons why a device returns a different period size and we probably want to use the actual period size for the buffer time.

As well - though this is opinionated - I'd probably rather have initialization succeed (so we can play sound) rather than fail hard just because the exact period or buffer time can't be set.

The latest commit handles various failures better and relies on the actual ALSA-provided period size after setting time.

roderickvd

Thanks a lot, this is really going in the right direction. Some points to make it even more robust - I've got prior experience where I know that ALSA drivers can be really fickle.

At any time just let me know if and how I can help.

roderickvd · 2025-08-14T12:13:07Z

CHANGELOG.md

@@ -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 size selection. (error 22)


This PR is going to be capable of doing a lot more 😉
It would be good to concisely but clearly capture the changes to the period size, buffer size, and fallback handling.

roderickvd · 2025-08-14T12:28:05Z

src/host/alsa/mod.rs

+    // `default` pcm sometimes fails here, but there no reason to as we attempt to provide a size or
+    // minimum number of periods.
+    hw_params
+        .set_buffer_size(period_size * PERIOD_COUNT as alsa::pcm::Frames)


This fallback is stricter than the primary path: should bed we not also use set_buffer_size_near here?

roderickvd · 2025-08-14T12:31:27Z

src/host/alsa/mod.rs

+fn set_hw_params_periods(hw_params: &alsa::pcm::HwParams, buffer_size: BufferSize) -> bool {
+    const FALLBACK_PERIOD_TIME: u32 = 25_000;
+
+    // When the API is made available, this could rely on snd_pcm_hw_params_get_periods_min and


Indeed, let's make a "TODO" as a future reminder.

roderickvd · 2025-08-14T12:36:12Z

src/host/alsa/mod.rs

+    const PERIOD_COUNT: u32 = 2;
+
+    /// Returns true if the buffer size was reasonably set.
+    fn set_hw_params_buffer_size(hw_params: &alsa::pcm::HwParams, buffer_size: FrameCount) -> bool {


We can make buffer size setting more robust by inspecting the device capabilities and clamping to the supported buffer sizes. Something like:

fn clamp_frames_to_device_caps(hw_params: &alsa::pcm::HwParams, requested: FrameCount) -> FrameCount { let min_buf = hw_params.get_buffer_size_min().unwrap_or(1); let max_buf = hw_params.get_buffer_size_max().unwrap_or(Frames::MAX); // usize::MAX? or something more sensical? requested.clamp(min_buf, max_buf) }

I've added this and also updated these other bits to use the same logic:

Supported stream configuration min/max clamped to valid range

Buffer size set after using BufferSize::Default checked for valid range

roderickvd · 2025-08-14T12:37:13Z

src/host/alsa/mod.rs

+            .set_period_size_near(period_size, alsa::ValueOr::Greater)
+            .is_err()
+        {
+            return false;


If setting the period size fails, we still could continue getting the actual period size and tuning the buffer size to it.

If this path fails, the fallback will use period time to calculate a valid buffer size. The last commit (4da16f1) has moved this all around a bit.

roderickvd · 2025-08-14T16:41:10Z

src/host/alsa/mod.rs

+    /// Returns true if the buffer size was reasonably set.
+    fn set_hw_params_buffer_size(hw_params: &alsa::pcm::HwParams, buffer_size: FrameCount) -> bool {
+        // Desired period size
+        let period_size = (buffer_size / PERIOD_COUNT) as alsa::pcm::Frames;


Period size we could similarly clamp by inspecting device capabilities.

I know it sounds like _near and ValueOr would yield something usable in all cases, but from librespot experience I know that not all ALSA drivers do.

use set_buffer_size_near to calculate fixed alsa buffer size

1508ff3

roderickvd self-assigned this Jul 29, 2025

roderickvd added the host - alsa label Jul 29, 2025

ALSA: Fallback to default buffer size on error

cddb361

roderickvd reviewed Aug 3, 2025

View reviewed changes

attackgoat and others added 4 commits August 9, 2025 13:17

ALSA: Add error handling to period/buffer size setup

0c9c099

Merge branch 'master' into alsa-buffer-size

199b142

Fix merge issue

5086bdb

Merge branch 'master' into alsa-buffer-size

35443ce

attackgoat requested a review from roderickvd August 14, 2025 11:29

roderickvd requested changes Aug 14, 2025

View reviewed changes

roderickvd added the enhancement label Aug 14, 2025

Clamp ALSA buffer size to valid CPAL values

4da16f1

attackgoat requested a review from roderickvd August 18, 2025 17:57

use set_buffer_size_near to calculate fixed alsa buffer size #990

Are you sure you want to change the base?

use set_buffer_size_near to calculate fixed alsa buffer size #990

Uh oh!

Conversation

attackgoat commented Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

roderickvd commented Jul 27, 2025

Uh oh!

roderickvd commented Jul 29, 2025

Uh oh!

roderickvd commented Jul 30, 2025

Uh oh!

attackgoat commented Jul 31, 2025

Uh oh!

roderickvd commented Aug 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roderickvd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

attackgoat Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

attackgoat commented Jul 23, 2025 •

edited

Loading

attackgoat Aug 18, 2025 •

edited

Loading