What to do about ExactSizeBuf and wrap::Dynamic?

[Be-ing]

Currently, audio::wrap::Dynamic does not implement the ExactSizeBuf trait. I think it is the only buffer struct in audio that does not implement this trait.

Rubato needs the number of frames in the buffer to verify that the user has passed in buffers of a sufficient size. Combined, these create a problem for integrating audio into Rubato due to HEnquist/rubato#52 (comment):

It must be reasonably easy to upgrade an existing project using non-audio Rubato. This and the previous point mean that it should be possible to somehow still use vecs of vecs, perhaps by wrapping them up as audio buffers. I don't want to force people to copy data back and forth between buffers, or to migrate the entire project to use audio buffers.

Currently, Rubato uses Vec<Vec<T>>s for its input and output buffers, so having an easy way to wrap those and pass them into a new version of Rubato using the Buf and BufMut traits is required.

I think it could make sense to implement ExactSizeBuf for wrap::Dynamic by returning the number of frames in the shortest channel. To start with, it would be odd to have any kind of multichannel buffer where the channels are different lengths. I'm having trouble thinking of a case where leaving off some frames of a bigger channel would be a problem. I think most likely, some data at the end of bigger channels would be skipped, but that's probably less bad than going out of bounds if the returned frame count was too large.

However, if ExactSizeBuf is implemented for every Buf, that raises the question, why have a separate trait for this instead of including a frames(&self) -> usize function in the Buf trait? Is the frames_hint method really needed?

In addition to this issue with Rubato, implementing a Range wrapper (#28) for Bufs gets awkward without a frames(&self) -> usize method on Buf.

[udoprog]

Just about rubato: I managed to port rubato using existing utilities (without a performance loss) in the branch I maintain. I believe size assumptions were being ensured by issuing resize_topology which I believe matches what rubato was currently doing manually if there's any topology changes so ExactSizeBuf wasn't necessary.

[Be-ing]

Rubato no longer resizes the buffers in process_into_buffer since HEnquist/rubato#50. Now it returns an error. IMO this is an improvement. I don't want a realtime safe library call allocating behind my back under any circumstance. If reallocation is required, that's a bug in my code calling the library and I want to know about it with an error.

Regardless of Rubato, it is very common for code using a buffer to require the number of frames and it's cumbersome to separate that from the base Buf trait.

[Be-ing]

I don't think Rubato (or other libraries, generally) should require the ResizableBuf trait for the same reason it was removed in HEnquist/rubato#50: it is an unnecessarily strict requirement. ResizableBuf isn't implemented for audio::wrap::Sequential (and I don't think it could be implemented easily) and it's only implemented for audio::wrap::Dynamic in the special case of Vec<Vec>.

[udoprog]

Neat that rubato removed that, all though I wasn't super worried since resizing basically only happens on the first call (and nothing happens if you preallocate appropriately).

Just sticking with the specific rubato issue for now: So channel validation still basically seems to be the same. That should be implementable already using Channel::len?

More broadly as for the minimum number of frames, this is something you could get out of any buffers like this:

let min_frames = buf.iter_channels().map(Channel::len).min();

And this would necessarily match the procedure of any dynamic data structure I can think of (unless they do internal accounting of shortest channel which is a bit esoteric).

Could you describe in a more condense format what it is that you want to do and why? As-in a bullet point?

[Be-ing]

More broadly as for the minimum number of frames, this is something you could get out of any buffers like this:

Ah, yeah, that could work. It's not super obvious that's an option though. I think that would make sense as a default implementation of fn frames(&self) -> usize for the Buf trait.

[udoprog]

🤔 Note that it returns Option<usize> - it's not entirely clear what it's supposed to be returning for dynamic structures which do not have any channels.

But a refactoring of Dynamic I'm considering is reworking it so that it behaves more like an over-allocated sequential buffer that once it shrinks it doesn't move anything but rather leaves unused regions between each channel. Such a structure would have a defined frames length even if it contains no channels.

But something like &mut [Vec<T>] I don't see how it could ever have one. And reporting a default value like 0 could be very misleading. This is the whole reason why ExactSizeBuf is a separate trait, like how ExactSizeIterator is a separate trait in the Iterator family of traits.

[Be-ing]

But something like &mut [Vec] I don't see how it could ever have one.

There are two ways that could be accomplished. Either that invariant could be checked when constructing the wrapper, or the wrapper could use Limit or something to limit the number of frames to the length of the shortest channel. I'd lean towards checking upon construction rather than silently doing something that may be surprising.

[Be-ing]

The more I think about this, the more I think it makes sense for the Buf trait to guarantee that each channel is the same length. I can't think of a real use case for buffers with channels of different lengths and allowing for that possibility adds complexity and ambiguity both to this library and downstream code. If there was ever a situation where I needed channels of different lengths, I'd use multiple buffers for that rather than put channels of different lengths into one data structure.

[udoprog]

First test would be to propose it to rubato and see how they feel about it! To be a generic API my primary concern is compatibility with downstream needs. And if they need to support &mut [Vec<T>] buffers I don't see much of a choice.

It's also hard to rule out other unknown downstreams, but hopefully more will be looked at as the API is being vetted.

[Be-ing]

I'm not clear what you're suggesting. The rubato maintainer has already made it clear that moving the API to use audio would require it to be easy to continue to use Vec<Vec<T>> with rubato's API.

[udoprog]

What I thought you suggested was that you'd propose changing rubato (and all downstreams) to require the use of either some type-safe wrapper or other type that guarantees uniform channel lengths.

But, since rubato requires the use of Vec<Vec<T>> that means that Buf can't assume a uniform channel length: Because there's no way to enforce that through Vec<Vec<T>> and enforcing some uniform version of Buf::frames would be incoherent with the underlying representation. So there doesn't seem like there's anything that can be done.

But if I loop back to the specific issue in rubato: since it already checks that channel lengths conform to spec (something we can do) there doesn't appear to be an issue?

[Be-ing]

require the use of either some type-safe wrapper or other type that guarantees uniform channel lengths.
But, since rubato requires the use of Vec<Vec>

Oh, I think we may be talking past each other a bit. Quoting HEnquist/rubato#52 (comment) again (emphasis mine):

it should be possible to somehow still use vecs of vecs, perhaps by wrapping them up as audio buffers

Literally passing Vec<Vec<T>> verbatim is not a requirement. It just needs to be easy to pass a Vec<Vec<T>> into a function that requires impl Buf without copying data. So I think providing a type safe wrapper for Vec<Vec<T>> that guarantees that all the inner Vecs are the same length would be sufficient. That would allow getting rid of the ambiguity of "what do I do when frames_hint returns None?" (which occurs in more situations than this one question about rubato).