udoprog / audio

A crate for working with audio in Rust
Apache License 2.0
78 stars 11 forks source link

consider renaming Sequential and Dynamic #14

Open Be-ing opened 1 year ago

Be-ing commented 1 year ago

I haven't seen these terms used to refer to audio (or image/video) buffers in other contexts. What "Sequential" does is more commonly called "planar", which I think would be a good name for the struct.

"Dynamic" kinda implies that the others are "static", but they all allow resizing both the number of frames and channels. "Disjoint" would describe how "Dynamic" organizes memory, but that feels like an awkward name...

udoprog commented 1 year ago

Planar seems like a good change. I'd mourn the loss of inherent comprehension. It's specialised terminology that in my mind is harder to grok without looking at reference documentation, but it is what pcm buffers with a linear channel topology tends to be called elsewhere.

Dynamic I'm not entirely sure about. I have half a mind yeeting the type myself, since I only introduced it because it mimics the default topology in rubato (Vec<Vec<T>>) but I don't think the behavior is super useful. I.e. spurious resizes are really not a thing once you look into it and other types can largely cope.

Be-ing commented 1 year ago

Dynamic I'm not entirely sure about. I have half a mind yeeting the type myself, since I only introduced it because it mimics the default topology in rubato (Vec<Vec>) but I don't think the behavior is super useful. I.e. spurious resizes are really not a thing once you look into it and other types can largely cope.

I've also been trying to come up with a real world use case for Dynamic and... I can't think of one. Even if a strange use case involved resizing a buffer more frequently than reading or writing it, resizing Dynamic requires a reallocation for every channel. By contrast, Sequential and Interleaved only require a single reallocation to resize, which I would think would be substantially faster, even though data has to be copied around within the memory.

udoprog commented 1 year ago

Yeah, it really is all about having a type which can ensure that it's compatible with rubato's output. But since I haven't found the use for it anywhere I'm very sceptical as well.

Even if a strange use case involved resizing a buffer more frequently than reading or writing it, resizing Dynamic requires a reallocation for every channel. By contrast, Sequential and Interleaved only require a single reallocation to resize, which I would think would be substantially faster, even though data has to be copied around within the memory.

There's two aspects to resizing: growing / shrinking and topology changes.

Dynamic only requires additional work when growing the number of samples beyond its current capacity (which grows by a factor for low amortized overhead), everything else is "free" including shrinking since that doesn't touch the underlying allocations.

This can be useful to avoid work in something like a scratch buffer that is used with more than one topology.

Be-ing commented 1 year ago

A planar buffer requires a lot of work when changing the number of samples per channel where potentially a lot of things need to be copied around.

Sequential could be implemented such that it doesn't move data when shrinking the number of frames and keeps track of the distance in memory between channels separately from the current frames per channel, like Vec. I doubt that's worth the implementation complexity and spreading out the data more in memory (potentially worse cache locality) to optimize resizing, which is generally done much less frequently than reading/writing.

Be-ing commented 1 year ago

Yeah, it really is all about having a type which can ensure that it's compatible with rubato's output.

FWIW, I got Sequential to work with Rubato's next-0.13 branch (https://github.com/HEnquist/rubato/pull/45) which has the API introduced by https://github.com/HEnquist/rubato/pull/50. However, this required some ugly hacks creating arrays of LinearChannels/mut slices. If I didn't assume the number of channels, creating an array for that wouldn't be possible and a Vec or something would be needed on the heap somewhere. I considered using rsor inside Sequential to return a slice of slices, but still, that would require storing an rsor::Slice inside Sequential and resizing the rsor::slice when Sequential's number of channels changes. That didn't seem worthwhile just to satisfy the current API of Rubato considering the Rubato maintainer has already expressed support for using audio::Buf in https://github.com/HEnquist/rubato/issues/51.