Code tidying, particularly SampleFormat, with many functions as const by Marco-Farruggio · Pull Request #1353 · RustAudio/cpal

Marco-Farruggio · 2026-09-03T13:47:16Z

Cleaned up a debug assertion in equilibrium.rs
Made frames_to_duration a bit more legible
Removed commented out i48/i48 lines
Made a macro to implement SizedSample
Made many methods on SampleFormat const

Lined up match arms (I can undo this quickly if you'd like)

…unctions

LastExceed · 2026-09-03T14:22:14Z

replacing assert_eq!(x,y) with assert!(x == y) seems backwards to me. Isn't this the very thing assert_eq exists for?
LGTM
LGTM
LGTM
LGTM - particularly excited about this one, as I (kind of) needed it for new ASIO host implementation #1331 :D
I too like lining things up, and I've been wondering if rust-fmt can be configured to allow this (as well as a few other things that I am currently too stubborn to fix in new ASIO host implementation #1331). But we should probably gather a few more opinions on this

PS: Try to record each change in a separate commit in the future. The enumeration you provided is the ideal degree of segmentation in my eyes. Probably not worth rewriting history retroactively here, as the PR is pretty easy to comprehend as-is, but as a general rule of thumb: "Commit early and often". You almost cannot overdo this

Marco-Farruggio · 2026-09-03T14:45:18Z

Yeah I personally like neat-looking code, and I can easily add back the _eq! if you'd like, I just removed it because when I thought the == was a bit cleaner, and sure, I'll stage it as separate commits going forward, thanks for the tip :)

Note on the branch name: I was originally going to clean up the fill_equilibrium function, alot more, but every time I refactored it I realised the original approach was good)

Marco-Farruggio · 2026-09-03T16:40:57Z

Clippy shows 62 functions which could be const for me (I think some could be false positives), I see zero downside of making many of these const, but I spose making some of them const may come across as a false 'promise' that it will be const forever? For some methods it doesn't really even make sense, like SupportedStreamConfig::new, which would be runtime only. Things such as StreamInstant::new() though, I think should just be const as there's no reason for them not to be

roderickvd

Direction is good and I appreciate these quality-fof-code PRs. A few points.

roderickvd · 2026-09-06T20:38:48Z

-            debug_assert_eq!(
-                buffer.len() % sample_size,
-                0,
+            debug_assert!(


Please revert these to use _eq!.

roderickvd · 2026-09-06T20:51:11Z

    #[must_use]
-    pub fn sample_size(&self) -> usize {
-        match *self {
+    pub const fn sample_size(self) -> usize {


These changes are breaking and should be documented both in CHANGELOG.md and UPGRADING.md. (consider stuff like formats.iter().map(SampleFormat::is_int) that will no longer compile)

roderickvd · 2026-09-06T20:52:09Z

            SampleFormat::I64 => i64::BITS,
            SampleFormat::U64 => u64::BITS,
-            SampleFormat::F32 => 32,
+            SampleFormat::F32 => 32, // f32/64::BITS is currently unstable, so we hardcode the values here.


Nitpick: I'd prefer the comment to go above both lines, or to be duplicated to the F64 arm for visibility.

I'd vote for duplicating

roderickvd · 2026-09-06T20:52:44Z

 /// match config.buffer_size() {
 ///     SupportedBufferSize::Range { min, max } => {
-///         println!("Buffer size range: {} - {}", min, max);
+///         println!("Buffer size range: {min} - {max}");  


Nitpick: two trailing spaces.

roderickvd · 2026-09-06T20:55:58Z

To the const point:

I'd leave SupportedStreamConfig::new alone because in the future it may want to do validation or clamping (#956) and then we want to have the flexibility to call something non-const.

I think we'd really need to go over it case-by-case. StreamInstant and SampleFormat you did in this PR were good ones to do it for.

roderickvd · 2026-09-07T19:10:43Z

@Marco-Farruggio thanks for the changes - I hope that means your op went well I hope! - let me know when you'd like me to do another review pass.

Marco-Farruggio · 2026-09-08T08:21:56Z

The op did go well thank you! I needed to take a break though before writing up the documentation, so I’ll do the change log and upgrading.md today, see if there’s anything else I want to do in this pr, and then request review/merge

Marco-Farruggio · 2026-09-08T09:00:00Z

Just noticed the 18.2 docs had the commented out u48 line in them

roderickvd

Glad to hear your op went well. A few nitpicks.

roderickvd · 2026-09-10T20:00:13Z

+**Impact:** `SampleFormat` can now be used in a `const` environment.
+
+**Why:** `SampleFormat` is a simple enum, there was no reason why it shouldn't be const-friendly, and since every method was both `inline` and it implements `Copy`, there
+is no performance downside to it taking `self`, but simply more legible than derefrencing.


Typo: "dereferencing".

In the rest of the file we don't use newlines.

roderickvd · 2026-09-10T20:03:31Z

 // After (v0.18)
 for line in desc.extended() {   // impl Iterator<Item = &str>
-    println!("{}", line);       // line: &str — Display, write!, format! all unchanged
+    println!("{line}");        // line: &str — Display, write!, format! all unchanged


The spacing of the comment seems unaligned with the one above it.

roderickvd · 2026-09-10T20:03:43Z

 // Before (v0.17)
 for line in desc.extended() {   // &[String]
-    println!("{}", line);       // line: &String
+    println!("{line}");       // line: &String


The spacing of the comment seems unaligned with the one above it.

roderickvd · 2026-09-11T18:20:22Z


 [`CallbackInfo::xrun()`]: https://docs.rs/cpal/latest/cpal/struct.CallbackInfo.html#method.xrun

+## 5. `SampleFormat` methods made `const`, and take `self`


For consistency, this should also have an entry in the numbered list at the top.

roderickvd · 2026-09-11T18:20:43Z

 - `StreamTrait::play` is renamed to `start`.
 - `InputCallbackInfo`/`OutputCallbackInfo` merged into `CallbackInfo`.
 - `InputStreamTimestamp`/`OutputStreamTimestamp` merged into `StreamTimestamp`; `capture`/`playback` renamed `device`.
+- `StreamInstant` creation is now `const`


Nitpick: trailing period.

Marco-Farruggio added 3 commits September 3, 2026 14:37

general clean ups, particularly sample formats, with const for many f…

3999b15

…unctions

undid the match arm lining-up, so that code quality is happy

8609e77

submitted to rust-fmt

1718ccb

Marco-Farruggio added 2 commits September 5, 2026 12:41

make frames_to_duration const

0a1d753

use {text} in format strings, where logical

9615dc7

LastExceed mentioned this pull request Sep 6, 2026

new ASIO host implementation #1331

Open

roderickvd requested changes Sep 6, 2026

View reviewed changes

Marco-Farruggio added 3 commits September 7, 2026 17:30

timestamp creation fns now const

255d063

reverted debug assertion formatting in equilibrium.rs

ff7aa55

fixed layout nitpicks

3b49e43

updated changelog and upgrading docs

41e840c

Marco-Farruggio requested a review from roderickvd September 8, 2026 09:07

Merge branch 'RustAudio:master' into simplify-fill-equilibrium

b9ab2bc

roderickvd requested changes Sep 11, 2026

View reviewed changes


		[`CallbackInfo::xrun()`]: https://docs.rs/cpal/latest/cpal/struct.CallbackInfo.html#method.xrun

		## 5. `SampleFormat` methods made `const`, and take `self`

Uh oh!

Conversation

Marco-Farruggio commented Sep 3, 2026

Uh oh!

LastExceed commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Marco-Farruggio commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Marco-Farruggio commented Sep 3, 2026

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!

roderickvd commented Sep 6, 2026

Uh oh!

roderickvd commented Sep 7, 2026

Uh oh!

Marco-Farruggio commented Sep 8, 2026

Uh oh!

Marco-Farruggio commented Sep 8, 2026

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

LastExceed commented Sep 3, 2026 •

edited

Loading

Marco-Farruggio commented Sep 3, 2026 •

edited

Loading