Skip to content

Commit 52cf021

Browse files
committed
Fix Player::skip_one not updating player's length immediately.
Achieve by adding `Skippable::skipped`, and updating `Done` to call a callback instead of decreasing an `Arc<AtomicUsize>`.
1 parent 4e90b4f commit 52cf021

5 files changed

Lines changed: 99 additions & 53 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
88
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
99

1010
## Unreleased
11+
- Breaking: `Done` now calls a callback instead of decrementing an `Arc<AtomicUsize>`.
12+
- Added `Skippable::skipped` function to check if the inner source was skipped.
13+
- Fixed `Player::skip_one` not decreasing the player's length immediately.
1114

1215
## Version [0.22.2] (2026-02-22)
1316

UPGRADE.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@ The list below only contains required code changes. For a complete list of
66
changes and new features, see [CHANGELOG.md](CHANGELOG.md).
77

88
# rodio 0.22 to current github version
9-
Nothing yet!
9+
- `Done` now calls a callback instead of decrementing an `Arc<AtomicUsize>`.
10+
- To retain old behavior replace the `Arc<AtomicUsize>` argument in `Done::new` with
11+
`move |_| { number.fetch_sub(1, std::sync::atomic::Ordering::Relaxed) }`.
12+
- `Done` has now two generics instead of one: `<I: Source, F: FnMut(&mut I)>`.
1013

1114
# rodio 0.21.1 to 0.22
1215
- _Sink_ terms are replaced with _Player_ and _Stream_ terms replaced

src/player.rs

Lines changed: 65 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -117,47 +117,55 @@ impl Player {
117117
let controls = self.controls.clone();
118118

119119
let start_played = AtomicBool::new(false);
120-
121-
let source = source
122-
.speed(1.0)
123-
// Must be placed before pausable but after speed & delay
124-
.track_position()
125-
.pausable(false)
126-
.amplify(1.0)
127-
.skippable()
128-
.stoppable()
129-
// If you change the duration update the docs for try_seek!
130-
.periodic_access(Duration::from_millis(5), move |src| {
131-
if controls.stopped.load(Ordering::SeqCst) {
132-
src.stop();
133-
*controls.position.lock().unwrap() = Duration::ZERO;
120+
let sound_count_clone = self.sound_count.clone();
121+
122+
let source = Done::new(
123+
source
124+
.speed(1.0)
125+
// Must be placed before pausable but after speed & delay
126+
.track_position()
127+
.pausable(false)
128+
.amplify(1.0)
129+
.skippable()
130+
.stoppable(),
131+
move |src| {
132+
if !src.inner().skipped() {
133+
sound_count_clone.fetch_sub(1, Ordering::Relaxed);
134134
}
135-
{
136-
let mut to_clear = controls.to_clear.lock().unwrap();
137-
if *to_clear > 0 {
138-
src.inner_mut().skip();
139-
*to_clear -= 1;
140-
*controls.position.lock().unwrap() = Duration::ZERO;
141-
} else {
142-
*controls.position.lock().unwrap() =
143-
src.inner().inner().inner().inner().get_pos();
144-
}
145-
}
146-
let amp = src.inner_mut().inner_mut();
147-
amp.set_factor(*controls.volume.lock().unwrap());
148-
amp.inner_mut()
149-
.set_paused(controls.pause.load(Ordering::SeqCst));
150-
amp.inner_mut()
151-
.inner_mut()
152-
.inner_mut()
153-
.set_factor(*controls.speed.lock().unwrap());
154-
if let Some(seek) = controls.seek.lock().unwrap().take() {
155-
seek.attempt(amp)
135+
},
136+
)
137+
// If you change the duration update the docs for try_seek!
138+
.periodic_access(Duration::from_millis(5), move |src| {
139+
if controls.stopped.load(Ordering::SeqCst) {
140+
src.inner_mut().stop();
141+
*controls.position.lock().unwrap() = Duration::ZERO;
142+
}
143+
{
144+
let mut to_clear = controls.to_clear.lock().unwrap();
145+
if *to_clear > 0 {
146+
src.inner_mut().inner_mut().skip();
147+
*to_clear -= 1;
148+
*controls.position.lock().unwrap() = Duration::ZERO;
149+
} else {
150+
*controls.position.lock().unwrap() =
151+
src.inner().inner().inner().inner().inner().get_pos();
156152
}
157-
start_played.store(true, Ordering::SeqCst);
158-
});
153+
}
154+
let amp = src.inner_mut().inner_mut().inner_mut();
155+
amp.set_factor(*controls.volume.lock().unwrap());
156+
amp.inner_mut()
157+
.set_paused(controls.pause.load(Ordering::SeqCst));
158+
amp.inner_mut()
159+
.inner_mut()
160+
.inner_mut()
161+
.set_factor(*controls.speed.lock().unwrap());
162+
if let Some(seek) = controls.seek.lock().unwrap().take() {
163+
seek.attempt(amp)
164+
}
165+
start_played.store(true, Ordering::SeqCst);
166+
});
167+
159168
self.sound_count.fetch_add(1, Ordering::Relaxed);
160-
let source = Done::new(source, self.sound_count.clone());
161169
*self.sleep_until_end.lock().unwrap() = Some(self.queue_tx.append_with_signal(source));
162170
}
163171

@@ -279,7 +287,7 @@ impl Player {
279287
pub fn clear(&self) {
280288
let len = self.sound_count.load(Ordering::SeqCst) as u32;
281289
*self.controls.to_clear.lock().unwrap() = len;
282-
self.sleep_until_end();
290+
self.sound_count.store(0, Ordering::Relaxed);
283291
self.pause();
284292
}
285293

@@ -294,6 +302,7 @@ impl Player {
294302
if len > *to_clear {
295303
*to_clear += 1;
296304
}
305+
self.sound_count.fetch_sub(1, Ordering::SeqCst);
297306
}
298307

299308
/// Stops the sink by emptying the queue.
@@ -361,6 +370,23 @@ mod tests {
361370
use crate::math::nz;
362371
use crate::{Player, Source};
363372

373+
#[test]
374+
fn test_immediate_length_changes() {
375+
let (player, mut source) = Player::new();
376+
377+
player.append(SamplesBuffer::new(nz!(1), nz!(1), vec![2.0, 3.0]));
378+
player.append(SamplesBuffer::new(nz!(1), nz!(1), vec![1.0, 0.5]));
379+
assert_eq!(player.len(), 2);
380+
assert_eq!(source.next(), Some(2.0));
381+
382+
player.skip_one();
383+
assert_eq!(player.len(), 1);
384+
assert_eq!(source.next(), Some(1.0));
385+
386+
player.clear();
387+
assert_eq!(player.len(), 0);
388+
}
389+
364390
#[test]
365391
fn test_pause_and_stop() {
366392
let (player, mut source) = Player::new();

src/source/done.rs

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,32 @@
1-
use std::sync::atomic::{AtomicUsize, Ordering};
2-
use std::sync::Arc;
31
use std::time::Duration;
42

53
use super::SeekError;
64
use crate::common::{ChannelCount, SampleRate};
75
use crate::Source;
86

9-
/// When the inner source is empty this decrements a `AtomicUsize`.
7+
/// When the inner source is exhausted, this source calls a callback once
8+
/// with the mutable reference to the inner source.
109
#[derive(Debug, Clone)]
11-
pub struct Done<I> {
10+
pub struct Done<I, F>
11+
where
12+
F: FnMut(&mut I),
13+
{
1214
input: I,
13-
signal: Arc<AtomicUsize>,
15+
callback: F,
1416
signal_sent: bool,
1517
}
1618

17-
impl<I> Done<I> {
18-
/// When the inner source is empty the AtomicUsize passed in is decremented.
19-
/// If it was zero it will overflow negatively.
19+
impl<I, F> Done<I, F>
20+
where
21+
F: FnMut(&mut I),
22+
{
23+
/// When the inner source is exhausted, this source calls a callback once
24+
/// with the mutable reference to the inner source.
2025
#[inline]
21-
pub fn new(input: I, signal: Arc<AtomicUsize>) -> Done<I> {
26+
pub fn new(input: I, callback: F) -> Done<I, F> {
2227
Done {
2328
input,
24-
signal,
29+
callback,
2530
signal_sent: false,
2631
}
2732
}
@@ -45,30 +50,33 @@ impl<I> Done<I> {
4550
}
4651
}
4752

48-
impl<I: Source> Iterator for Done<I>
53+
impl<I, F> Iterator for Done<I, F>
4954
where
5055
I: Source,
56+
F: FnMut(&mut I),
5157
{
5258
type Item = I::Item;
5359

5460
#[inline]
5561
fn next(&mut self) -> Option<I::Item> {
5662
let next = self.input.next();
5763
if !self.signal_sent && next.is_none() {
58-
self.signal.fetch_sub(1, Ordering::Relaxed);
5964
self.signal_sent = true;
65+
(self.callback)(&mut self.input);
6066
}
6167
next
6268
}
6369

70+
#[inline]
6471
fn size_hint(&self) -> (usize, Option<usize>) {
6572
self.input.size_hint()
6673
}
6774
}
6875

69-
impl<I> Source for Done<I>
76+
impl<I, F> Source for Done<I, F>
7077
where
7178
I: Source,
79+
F: FnMut(&mut I),
7280
{
7381
#[inline]
7482
fn current_span_len(&self) -> Option<usize> {

src/source/skippable.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@ impl<I> Skippable<I> {
3030
self.do_skip = true;
3131
}
3232

33+
/// Returns true if the inner source was skipped.
34+
#[inline]
35+
pub fn skipped(&self) -> bool {
36+
self.do_skip
37+
}
38+
3339
/// Returns a reference to the inner source.
3440
#[inline]
3541
pub fn inner(&self) -> &I {

0 commit comments

Comments
 (0)