Phase1a of audio engine rewrite by yara-blue · Pull Request #903 · RustAudio/rodio

yara-blue · 2026-07-23T12:07:29Z

supersedes #902

Tracking issue: #901

Very minimal still, to get an idea of what this will all look like see: rodio-experiments.

Implementation notes:

I've moved some documentation into md files and include_str! those to prevent duplication. We'll be using that even more for the effects.
For now does not rename Source but do make it available under it's new name via a pub re-export in lib.rs
There is a placeholder so the example in the docs works

roderickvd · 2026-08-22T20:00:22Z

+
+/// A buffer of samples treated as a source.
+#[derive(Debug, Clone)]
+pub struct SamplesBuffer<const SR: u32, const CH: u16> {


Can we use the SampleRate and ChannelCount type aliases?

Sadly not, const params are limited to primitive types atm :(

roderickvd · 2026-08-22T20:02:47Z

@@ -0,0 +1,20 @@
+//! Code shared between source types.
+//!
+//! Since we have three source types there is a lot of code duplication. We


I thought this shouldn't be user-facing Rustdoc when I saw it probably isn't, because the module isn't imported as public. Still, may want to consider the Rustdoc level.

It's still useful to use doc comments instead of simple comments as rust-analyzer makes hover doc available for this when working on Rodio.

roderickvd · 2026-08-22T20:03:48Z

+impl<const SR: u32, const CH: u16> SamplesBuffer<SR, CH> {
+    /// Builds a new `SamplesBuffer`.
+    ///
+    /// Note any call to total_duration will panic if the buffer is larger then


I see you moved that code from new to total_duration which probably makes sense. But I'd say it doesn't need docs here then.

roderickvd · 2026-08-22T20:05:21Z

+    where
+        D: Into<Vec<Sample>>,
+    {
+        const { assert!(SR > 0) };


Not too familiar with const generics; I guess we can't use NonZero as we wanted?

yes, pretty sad. One day we'll get rustc there. But thanks to the const block that assert will fire compile time.

roderickvd · 2026-08-22T20:09:22Z

+                    .map_err(|e| SeekError::Other(e))
+            } else {
+                self.second
+                    .try_seek(pos)


I think this needs to be pos - first. As-is, chaining two 1-second sources and seeking to 1.5s puts the second source at 1.5s, which is past its own end.

roderickvd · 2026-08-22T20:27:10Z

This still needs try_seek, size_hint and ExactSizeIterator implemented.

roderickvd · 2026-08-22T20:28:53Z

+
+#[derive(Debug, thiserror::Error)]
+pub enum ChainSeekError {
+    #[error("Could not get duration of first source ({ty}")]


Add closing ).

roderickvd · 2026-08-22T20:30:57Z

+use crate::source::SeekError;
+
+#[derive(Debug, thiserror::Error)]
+pub enum ChainSeekError {


Should this be exported publicly so users can downcast to them from a SeekError::Other(Arc<dyn Error>)?

It should be re-exported in the source implementation as wel. See the: pub use crate::common::source::chain::ChainSeekError; in const_source/chain.rs

roderickvd · 2026-08-22T20:32:30Z

+pub enum ChainSeekError {
+    #[error("Could not get duration of first source ({ty}")]
+    NoTotalDurationForFirst { ty: &'static str },
+    #[error("Could not seek in first source")]


Add ({ty}) here too? And below.

roderickvd · 2026-08-22T20:33:48Z

+        type Item = crate::Sample;
+
+        fn next(&mut self) -> Option<Self::Item> {
+            Some(0.0)


Sample::EQUILIBRIUM

yara-blue · 2026-08-23T14:33:00Z

Thanks for the review! Should have addressed all of the feedback. I'll see if I can fix the Phase1b tonight.

PetrGlad · 2026-08-24T18:24:38Z

OK, I see channel and seeking precision bugs I saw yesterday, are already fixed :)
I do not really like use of macroses(like macro_rules! source_impl) since it is harder to see usages, and it postpones some checks to final compilation. Was there a reason to use macroses instead of traits, or is it just to save typing? This implementation would do, bit I'd considered something more explicit.

yara-blue · 2026-08-25T23:42:47Z

First of all thanks for finding the time for the review! Secondly, I did not have the time to write a short answer so you get a very long one, apologies.

Was there a reason to use macroses instead of traits, or is it just to save typing? This implementation would do, bit I'd considered something more explicit.

There are going to be even more macro calls when we get to implementing all the effects... Lets see what we can do:

Traits

I've tried doing something with traits, separating out the repeated parts:

trait FixedSource {
  fn sample_rate(&self) -> SampleRate;
  fn channel_count(&self) -> ChannelCount;
  fn total_duration(&self) -> Duration;
  fn try_seek(&mut self, pos: Duration) -> Result<(), SeekError>;
}
trait ConstSource {
  fn sample_rate(&self) -> SampleRate;
  fn channel_count(&self) -> ChannelCount;
  fn total_duration(&self) -> Duration;
  fn try_seek(&mut self, pos: Duration) -> Result<(), SeekError>;
}

We would have:

trait FixedSource: TotalDuration + TrySeek {
  fn sample_rate(&self) -> SampleRate;
  fn channel_count(&self) -> ChannelCount;
}
trait ConstSource: TotalDuration + TrySeek {
  fn sample_rate(&self) -> SampleRate;
  fn channel_count(&self) -> ChannelCount;
}
trait TotalDuration {
  fn total_duration(&self) -> Duration;
}
trait TrySeek {
  fn try_seek(&mut self, pos: Duration) -> Result<(), SeekError>;
}

Now we can implement TotalDuration and TrySeek separately. This does not win us anything unfortunately, we still need a separate object for ConstSource since it will need to have const generic args.

Marker trait

Why is ConstSource not just a marker trait for FixedSource?

You'd do:

impl FixedSource for Amplify {
  fn sample_rate() -> SampleRate {
     ... 
  }
  ...
}

impl<SR, CH> ConstSource for Amplify<SR, CH>;

Well the reason for that is we do not want any type to implement both FixedSource and ConstSource. They mean different things so they must be separate types. Still we seem to be onto something here. Lets turn it around and wrap instead of marking:

Wrapping

struct Amplify(fixed_source::Amplify);

impl<SR, CH> ConstSource<SR, CH> for Amplify {
   fn total_duration(&self) -> Duration {
     self.0.total_duration()
   }
   ...
}

That seems nice, no more code duplication and no more macro's. But then we realize that S will probably wrap some existing source (like chain does or a future amplify (in phase 3a). But fixed_source::S can only be generic over a FixedSource and S here is a ConstSource so it is generic over one of those.

// Error: argument T to fixed_source::Amplify must implement FixedSource
struct Amplify<T: ConstSource>(fixed_source::Amplify<T>);

Well for that we have the IntoFixedSource wrapper! So we get:

struct Amplify<T>(fixed_source::Amplify<IntoFixedSource<T>);

impl<SR, CH> ConstSource<SR, CH> for Amplify {
   fn total_duration(&self) -> Duration {
     self.0.total_duration()
   }
   ...
}

I don't like that this leads to a lot of IntoFixedSource's in there but those should compile out... I have to think about it. What do you all think?

PetrGlad · 2026-08-29T13:06:33Z

Yes, this is the explanation I was looking for. Of the versions you mentioned this one I had in mind, actually:

trait FixedSource: TotalDuration + TrySeek {
  fn sample_rate(&self) -> SampleRate;
  fn channel_count(&self) -> ChannelCount;
}
trait ConstSource: TotalDuration + TrySeek {
  fn sample_rate(&self) -> SampleRate;
  fn channel_count(&self) -> ChannelCount;
}
trait TotalDuration {
  fn total_duration(&self) -> Duration;
}
trait TrySeek {
  fn try_seek(&mut self, pos: Duration) -> Result<(), SeekError>;
}

Regarding ConstSource, why not

trait ConstSource<const SR: u32, const CH: u16>: TotalDuration + TrySeek {
  fn sample_rate(&self) -> SampleRate;
  fn channel_count(&self) -> ChannelCount;
}

PetrGlad · 2026-08-29T13:11:17Z

// I understand that FixedSource is primarily a temporary shim to distinguish it from existing Source. But for a reader "ConstSource" may imply that it supplies a constant value for each channel, for example. A better name for what is now ConstSource would be actually FixedSource... This renaming can be done later, but in its present form I find the naming confusing.

PetrGlad · 2026-08-29T13:21:53Z

Regarding simultaneous implementation of ConstSource and FixedSource - this does not seem to be an actual problem, but maybe there will be situations where it could be (e.g. you anticipate API that cannot be supported by both)?

yara-blue · 2026-08-29T15:04:00Z

I understand that FixedSource is primarily a temporary shim to distinguish it from existing Source.

Not really, for example the implementation of queue gets far simpler (and more performant) when it no longer needs to watch for parameter changes. That was the primary motivation for this rework.

But for a reader "ConstSource" may imply that it supplies a constant value for each channel, for example. A better name for what is now ConstSource would be actually FixedSource... This renaming can be done later, but in its present form I find the naming confusing.

I would not mind eventually removing DynamicSource (the current source) but ideally the next release does not do that yet. That way users can slowly move over. (We can eventually mark it as deprecated then later remove). Even if we rename the current Source to DynamicSource in the upcoming release I'm not sure if we should reuse the Source name (could be pretty confusing).

Regarding ConstSource, why not

trait ConstSource<const SR: u32, const CH: u16>: TotalDuration + TrySeek {
  fn sample_rate(&self) -> SampleRate;
  fn channel_count(&self) -> ChannelCount;
}

We could, but it would not really help us. You'd still need to implement four methods except now they are divided by between three traits. It would help us if we could simultaneously implement the source trait for one object (see next answer for why that is impossible).

Regarding simultaneous implementation of ConstSource and FixedSource - this does not seem to be an actual problem

do you mean a struct implementing both traits? While you can do that it becomes really annoying to then use the struct. You'd have to disambiguate (specify which trait you want a method from) every time you call .amplify() or .take_duration() etc. Only way to solve that is to have two separate objects each implementing only one trait.

yara-blue added 3 commits July 22, 2026 01:44

Warn for in progress rewrite in Readme

a365e10

land const source

3f086ff

Add silence generator (only for const source)

8ed9f6e

yara-blue mentioned this pull request Jul 23, 2026

Phase1 of audio engine rewrite #902

Closed

yara-blue force-pushed the phase1a branch from a7b9c20 to 808cd06 Compare July 23, 2026 14:00

This was referenced Jul 23, 2026

Phase1b of audio engine rewrite #904

Open

Tracking issue for audio engine rewrite #901

Open

add seeking

f4e9464

yara-blue force-pushed the phase1a branch from 808cd06 to f4e9464 Compare July 23, 2026 14:56

Extract code shared between sources to common/source

a5e02a9

yara-blue requested a review from roderickvd July 23, 2026 21:28

roderickvd reviewed Aug 22, 2026

View reviewed changes

review feedback

49a3fa0

Uh oh!

Conversation

yara-blue commented Jul 23, 2026

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!

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!

Choose a reason for hiding this comment

Uh oh!

yara-blue commented Aug 23, 2026

Uh oh!

PetrGlad commented Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yara-blue commented Aug 25, 2026

Traits

Marker trait

Wrapping

Uh oh!

PetrGlad commented Aug 29, 2026

Uh oh!

PetrGlad commented Aug 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PetrGlad commented Aug 29, 2026

Uh oh!

yara-blue commented Aug 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

PetrGlad commented Aug 24, 2026 •

edited

Loading

PetrGlad commented Aug 29, 2026 •

edited

Loading