RustAudio / rodio

Rust audio playback library
Apache License 2.0
1.78k stars 232 forks source link

`Sink::skip_one` doesn't modify the sink's length accordingly #497

Open daniellga opened 1 year ago

daniellga commented 1 year ago

I am creating a new issue since I wasn't able to reopen #488 . I have this problem even after #494 .

#[cfg(test)]
mod tests {
    use rodio::{source::SineWave, OutputStream, Sink, Source};
    use std::time::Duration;

    #[test]
    fn skip_one_test() {
        let (_stream, stream_handle) = OutputStream::try_default().unwrap();
        let sink = Sink::try_new(&stream_handle).unwrap();
        let source = SineWave::new(440.0)
            .take_duration(Duration::from_secs_f32(5.))
            .amplify(0.20);
        let source2 = SineWave::new(440.0)
            .take_duration(Duration::from_secs_f32(5.))
            .amplify(0.20);

        sink.append(source);
        sink.append(source2);

        assert_eq!(sink.len(), 2); // OK as expected

        sink.skip_one();

        assert_eq!(sink.len(), 1); // shouln't this be true? I am getting sink.len() == 2 instead
    }
}
daniellga commented 1 year ago

I mean, I am assuming this should be the behaviour, although not clear in the function's documentation.

dysphoriac commented 1 year ago

im having the opposite problem kinda, when i call sink.skip_one() it skips all the sources in the sink.

daniellga commented 1 year ago

@sisyphus2a are you using the github version? I am pretty sure I was having this problem before... For me it works pretty well except for this issue here...

alset0326 commented 1 year ago

I have same issue too. And after some debug, I think the variable controls.do_skip not restored that cause the issue.

/// sink.rs:88 
.periodic_access(Duration::from_millis(5), move |src| {
    if controls.stopped.load(Ordering::SeqCst) {
        src.stop();
    }
    if controls.do_skip.load(Ordering::SeqCst) {
        let _ = src.inner_mut().skip();
        let mut to_clear = controls.to_clear.lock().unwrap();
        if *to_clear == 1 {
            controls.do_skip.store(false, Ordering::SeqCst);  // <<<< set controls.do_skip
            *to_clear = 0;
        } else if *to_clear > 0 {
            *to_clear -= 1;
        }
    }
    let amp = src.inner_mut().inner_mut();
    amp.set_factor(*controls.volume.lock().unwrap());
    amp.inner_mut()
        .set_paused(controls.pause.load(Ordering::SeqCst));
    amp.inner_mut()
        .inner_mut()
        .set_factor(*controls.speed.lock().unwrap());
})

We can see that controls.do_skip only be set when *to_clear == 1 which cannot come true by calling skip_once(). So we skipped all sources. Maybe not mix skip with clear?