Skip to content

Commit

Permalink
Make IntoInterleavedSamples(Iterator) stop yielding samples on exhaus…
Browse files Browse the repository at this point in the history
…tion (#178)

* Make IntoInterleavedSamples(Iterator) stop yielding samples on exhaustion

`IntoInterleavedSamples`, especially its companion
`IntoInterleavedSamplesIterator` type, is, in my opinion, very useful to
easily process samples from an audio signal with elegance.

However, these helper types do not handle the source signal being
exhausted at all. When the signal is exhausted, they always return
equilibrium-valued samples, turning finite signals into infinite ones in
an arguably counter-intuitive manner. I also think that quirk, even
though it is documented, is somewhat hard to discover, because it is
indirectly mentioned in the description of the
`IntoInterleavedSamplesIterator` type, which is not shown when using the
features of most IDEs.

In particular, I think the following reasons justify this change to
achieve a better API:

- Exhaustion is contagious for the rest of adapters returned by `Signal`
  methods. It's inconsistent that `into_interleaved_samples` behaves
  differently.
- In some scenarios the total number of samples of a finite signal is
  not known, and is not feasible to read it all to a buffer. Also, valid
  samples may have a value that is numerically equal to the equilibrium
  value, so checking for equilibrium values is not a good solution. On
  the contrary, it always is possible to make the finite sequence
  infinite easily. When dealing with iterators, this is as easy as
  calling `.chain(std::iter::repeat(f32::EQUILIBRIUM)`, for example.
- It provides for more elegant and failure-proof client code in some
  cases. For example, the following code now terminates when the input
  signal is finite, as I think most people would expect:

```rust
dasp_signal::from_interleaved_samples_iter::<_, Stereo<f32>>(signal.into_interleaved_samples().into_iter().map(|s| s * 2)))
```

To address this situation, these changes modify how the related methods
are implemented and their signatures. `cargo test` runs successfully,
and I have updated the related examples and documentation accordingly.
  • Loading branch information
AlexTMjugador authored Jul 20, 2022
1 parent 66e8b83 commit 6b15274
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 12 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
# Unreleased

- Renamed `window-hanning` to `window-hann`
- Made `IntoInterleavedSamples` and `IntoInterleavedSamplesIterator` stop
yielding samples when the underlying signal gets exhausted. This is a breaking
change. The return type of the `IntoInterleavedSamples#next_sample` method was
modified.

---

Expand Down
34 changes: 22 additions & 12 deletions dasp_signal/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -547,18 +547,17 @@ pub trait Signal {
/// let frames = [[0.1, 0.2], [0.3, 0.4]];
/// let signal = signal::from_iter(frames.iter().cloned());
/// let samples = signal.into_interleaved_samples();
/// let samples: Vec<_> = samples.into_iter().take(4).collect();
/// let samples: Vec<_> = samples.into_iter().collect();
/// assert_eq!(samples, vec![0.1, 0.2, 0.3, 0.4]);
/// }
/// ```
fn into_interleaved_samples(mut self) -> IntoInterleavedSamples<Self>
fn into_interleaved_samples(self) -> IntoInterleavedSamples<Self>
where
Self: Sized,
{
let first = self.next().channels();
IntoInterleavedSamples {
signal: self,
current_frame: first,
current_frame: None,
}
}

Expand Down Expand Up @@ -1052,10 +1051,11 @@ where
S: Signal,
{
signal: S,
current_frame: <S::Frame as Frame>::Channels,
current_frame: Option<<S::Frame as Frame>::Channels>,
}

/// Converts the `IntoInterleavedSamples` into an `Iterator` that always returns `Some`.
/// Converts the `IntoInterleavedSamples` into an `Iterator` that returns `Some`
/// until the signal is exhausted.
pub struct IntoInterleavedSamplesIterator<S>
where
S: Signal,
Expand Down Expand Up @@ -2286,13 +2286,23 @@ where
S: Signal,
{
/// Yield the next interleaved sample from the inner `Signal`.
///
/// The returned value is `None` if the signal is exhausted.
#[inline]
pub fn next_sample(&mut self) -> <S::Frame as Frame>::Sample {
loop {
match self.current_frame.next() {
Some(channel) => return channel,
None => self.current_frame = self.signal.next().channels(),
pub fn next_sample(&mut self) -> Option<<S::Frame as Frame>::Sample> {
if self.current_frame.is_none() && !self.signal.is_exhausted() {
self.current_frame = Some(self.signal.next().channels());
}

if let Some(current_frame) = &mut self.current_frame {
if let Some(sample) = current_frame.next() {
Some(sample)
} else {
self.current_frame = None;
self.next_sample()
}
} else {
None
}
}

Expand All @@ -2311,7 +2321,7 @@ where

#[inline]
fn next(&mut self) -> Option<Self::Item> {
Some(self.samples.next_sample())
self.samples.next_sample()
}
}

Expand Down

0 comments on commit 6b15274

Please sign in to comment.