Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Iterator-based version of process_into_buffer() #51

Open
ilmai opened this issue Oct 17, 2022 · 7 comments
Open

Iterator-based version of process_into_buffer() #51

ilmai opened this issue Oct 17, 2022 · 7 comments

Comments

@ilmai
Copy link

ilmai commented Oct 17, 2022

#49 proposes making process_into_buffer() more generic by accepting a slice of mutable references instead of a slice of vectors. However, even this doesn't cater to all use cases. In my codebase, I have a multi-channel signal type which can have an arbitrary number of channels whose number isn't known at compile time. I can't make its interface return &mut [&mut [f32]] because that would be unsound: the caller could assign an arbitrary reference to the inner slice instead of using it in the intended way of simply mutating the inner slices. I'm solving the issue by having an interface that returns iterators of slices, so you can still do stuff like for channel in signal.channels_mut() { for sample in channel { *sample *= 0.1 } }.

At the moment, interfacing my signal type with Rubato requires me to have a temporary buffer which I use to copy to/from my signal type and that's then processed by Rubato. If Rubato had a version of process_into_buffer() that accepted iterators of slices, I could get rid of the extra copies and temporary buffers.

I would be happy to submit a pull request for this if this sounds like something that you would consider accepting.

@HEnquist
Copy link
Owner

I have been considering something like this and I think it's a good idea. I just didn't find the time to do anything yet.
It would be really great if it could be done completely without the slices, like an iterator for the channels, that in turn provides an iterators for the samples of each channel. I'm not sure if that's practical though, haven't really thought about it.
Perhaps it would be a good start to make a mockup of the new api, something like I made here: #50 (comment)

@Be-ing
Copy link
Contributor

Be-ing commented Oct 17, 2022

It would be really great if it could be done completely without the slices, like an iterator for the channels, that in turn provides an iterators for the samples of each channel.

Instead of reinventing the wheel, how about using the audio crate? @udoprog is planning on releasing a major rewrite soon with the immanent stabilization of generic associated types.

@udoprog
Copy link
Contributor

udoprog commented Oct 17, 2022

I'd be happy to join efforts if you're up for it. I've got a rubato branch where I'm vetting the audio crate prior to its initial release. It's currently 90 commits behind. But should give an idea of how things work: https://github.com/udoprog/rubato/tree/next.

The idea for me is to release the 0.2 versions of all the audio crate abstractions on November 6 (Rust 1.65!) when GATs are available. This version can then be used to vet the implementation against the ecosystem and garner feedback.

@HEnquist
Copy link
Owner

audio is definitely interesting. That branch also has some very useful refactoring to reduce the duplication in the asynchronous resamplers. The new polynomial types would benefit from the same treatment. Maybe the sinc and polynomial types can even be merged, they are quite similar.
One question is if there are use cases where using the audio buffers for input and output won't work. @ilmai would it work for you?

@ilmai
Copy link
Author

ilmai commented Oct 18, 2022

Oh yeah that would work! I have my own set of audio signal traits and types in my codebase, but the Buf trait looks simple enough to implement for my types so it would definitely work.

@Be-ing
Copy link
Contributor

Be-ing commented Nov 24, 2022

@udoprog Are you planning to rebase your rubato branch and open a PR?

@Be-ing
Copy link
Contributor

Be-ing commented Dec 11, 2022

I've got a rubato branch where I'm vetting the audio crate prior to its initial release. It's currently 90 commits behind. But should give an idea of how things work: https://github.com/udoprog/rubato/tree/next.

That branch started from Rubato in August 2021. Instead of trying to bring it up to date, I've started a new branch in #52 starting from #45. It's not done yet, but I opened the PR early to get preliminary feedback on the API. So far the refactoring has been straightforward albeit tedious. I changed the signature of process_into_buffer to:

fn process_into_buffer<In, Out>(
    &mut self,
    wave_in: &In,
    wave_out: &mut Out,
    active_channels_mask: Option<&[bool]>,
) -> ResampleResult<(usize, usize)>
where
    In: ExactSizeBuf<Sample = T>,
    Out: ExactSizeBuf<Sample = T> + BufMut<Sample = T>;

The ExactSizeBuf bound is required rather than the base Buf trait for the validate_buffers function to check that the buffers' numbers of frames are sufficient.

The audio crate has a nice wrap module which provides wrappers for standard library types (Vecs and slices) that implement its traits without requiring copying. By re-exporting that module, Rubato can accept buffers in a variety of forms (including interleaved buffers) without requiring that Rubato users have to use the audio crate's buffer structs (though I recommend using them; their APIs are quite nice).

Be-ing pushed a commit to Be-ing/rubato that referenced this issue Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants