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

Windower constructor should take Iterator #51

Open
andrewcsmith opened this issue Nov 17, 2016 · 5 comments
Open

Windower constructor should take Iterator #51

andrewcsmith opened this issue Nov 17, 2016 · 5 comments

Comments

@andrewcsmith
Copy link
Contributor

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

@andrewcsmith
Copy link
Contributor Author

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

@mitchmindtree
Copy link
Member

mitchmindtree commented Nov 18, 2016

@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.

@andrewcsmith
Copy link
Contributor Author

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
Copy link
Contributor Author

andrewcsmith commented Nov 19, 2016

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
Copy link
Contributor Author

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.

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

2 participants