dasp icon indicating copy to clipboard operation
dasp copied to clipboard

Windower constructor should take Iterator

Open andrewcsmith opened this issue 8 years ago • 5 comments

Right now it only takes a slice, which unfortunately means that it can't handle windowing continuous streams.

andrewcsmith avatar Nov 17 '16 16:11 andrewcsmith

Should mention – I'm thinking specifically about passing a Converter (which is an Iterator) directly to a Windower.

andrewcsmith avatar Nov 17 '16 16:11 andrewcsmith

@andrewcsmith I guess the trade-off with this is that a Windower that takes an Iterator would need to own its own buffer so that it may store frames in between hops in the case that the hop is smaller than the bin. Perhaps we could make this a new type that takes an Iterator<Item=Frame> (Signal) Edit: i.e. SignalWindower? Or have the Windower be generic over either slices or iterators? Not sure how ergonomic this would be though.

mitchmindtree avatar Nov 18 '16 01:11 mitchmindtree

I tried to figure this out today and lost. It's not just owning its own buffer, but also giving up that buffer as an iterator on each call to next().

I do think it's important, because I would like to be able to do sample rate conversion on a continuous stream, but it should perhaps be two separate structs. One is more efficient for certain purposes, and the other is more efficient for others.

andrewcsmith avatar Nov 18 '16 06:11 andrewcsmith

After circling around a few ideas, I have something of a strategy now:

  • Make Windower have a member buf: &mut [I::Item] (eliding lifetimes...)
  • Within next(), memcpy the old frames to the front of buf (to ensure contiguous memory) and add the new frames to the end (instead of using VecDeque)
  • Return a mul_amp iterator, same as we do now

It would be reasonable to make this a separate type, because the original implementation is certainly more efficient. That said, the implementation of Windower is not that complex (yet) and so the repeated code would be pretty minimal.

Second point: there should also be a version of Windower that holds its calculated window in a buffer to avoid duplicating calculations every call. This could be part of the same PR. In fact, we could just add window_buffer: Option<&mut [I::Item]> as a member of Windower, and avoid creating a whole new struct.

Edit: It's pretty clear that for this to happen in any meaningful way we'd need to create our own trait with a streaming iterator. And discussing point 2, the way to go would probably be to have a Window type that is a lookup table, where the initialization would take another type of Window.

@mitchmindtree

andrewcsmith avatar Nov 19 '16 16:11 andrewcsmith

How about also having some kind of "should be able to Window over a Signal" in here? This is one of those big roadblocks that I feel like experienced DSP programmers have when they hit Rust – "where's my ring buffer," basically.

andrewcsmith avatar Jul 11 '17 01:07 andrewcsmith