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

Stream dropped but audio callback still being called #771

Open
morajabi opened this issue Apr 3, 2023 · 11 comments · May be fixed by #869
Open

Stream dropped but audio callback still being called #771

morajabi opened this issue Apr 3, 2023 · 11 comments · May be fixed by #869

Comments

@morajabi
Copy link

morajabi commented Apr 3, 2023

I added a log where I drop the stream:

drop(stream);
info!("stream dropped");

and you can see I'm still getting samples from cpal:
CleanShot 2023-04-04 at 01 21 58@2x

seems like after updating to 0.15 it appeared, let me downgrade and check.

Update: yes it doesn't happen in 0.14.

@est31
Copy link
Member

est31 commented Apr 4, 2023

@morajabi would it be possible to do a git bisect? Then we know where to look.

@morajabi
Copy link
Author

morajabi commented Apr 4, 2023

@est31 I haven't used git bisect before and it seems like I need a focused commit on this change? My last commit contains other changes as well, I took out changes to our audio module that fixed the issue and these are that:
Cargo.toml
CleanShot 2023-04-04 at 13 37 06

build_input_stream
CleanShot 2023-04-04 at 13 36 40

To test if it drops stream properly I created the stream and dropped it immediately. In 0.15 it continued calling my data callback. After downgrading to 0.14, it stopped immediately.

Sorry about posting screenshots — if you're unable to recreate or it's unclear, I will try to create a reproduction repo later this week as I'm a bit busy atm.

@ZerNico
Copy link

ZerNico commented Apr 9, 2023

I can confirm, it happens in for me too on 0.15, it doesn't happen on 0.14 or when using the default input device on 0.15. I only tested on macOS Ventura 13.3 btw. If you need any more information, tell me, I am happy to help.

Here is a minimal reproduction.

use std::{time::Duration, thread};
use cpal::traits::{DeviceTrait, HostTrait, StreamTrait};

fn main() {
    let host = cpal::default_host();
    
    let mut input_devices = host.input_devices().expect("Failed to get input devices");
    let device = input_devices
    .next()
    .expect("Failed to get default input device");

    /*
    Works fine with this
    let device = host
        .default_input_device()
        .expect("Failed to get default input device");
    */

    let config: cpal::StreamConfig = device
        .default_input_config()
        .expect("Failed to get default input config")
        .into();

    let input_data_fn = move |data: &[f32], _: &cpal::InputCallbackInfo| {
        println!("data len: {}", data.len());
    };

    let input_stream = device
        .build_input_stream(&config, input_data_fn, err_fn, None)
        .expect("Failed to build input stream");

    input_stream.play().expect("Failed to play input stream");

    thread::sleep(Duration::from_secs(2));
    drop(input_stream);
    thread::sleep(Duration::from_secs(5));
}

fn err_fn(err: cpal::StreamError) {
    eprintln!("an error occurred on stream: {}", err);
}

@ZerNico
Copy link

ZerNico commented Apr 9, 2023

After some testing I found out that 0af6a1c is the commit that introduced the bug.

@cenzovit
Copy link

Also running into this. Was extraordinarily confused by the callback still firing even when I had manually dropped the stream and ended the thread that was running my audio stream logic...

@Ralith
Copy link
Contributor

Ralith commented Oct 13, 2023

Is this macos-only?

@cenzovit
Copy link

cenzovit commented Oct 13, 2023

Not sure about other systems, but I ran into the issue on macos as well.

@ZerNico
Copy link

ZerNico commented Oct 13, 2023

Could be only on MacOS. I only tried it there

@cenzovit
Copy link

I am a little surprised more people haven't run into this issue since it seems it was introduced a year ago. I am torn because the bug is pretty bad, but the functionality the change was trying to introduce is important (triggering the error callback on device disconnect).

In my use case, I need to resample the input stream to a specific sample rate. To do so, I need to know the sample rate of the input stream. This becomes an issue when the input stream magically switches devices / config without triggering any callbacks. This occurs when using the default device in cpal 0.15 and also seems to occur when using any device that disconnects in cpal 0.14. It would probably be more ideal if cpal also exposed an easy way to determine the device and config information from the input stream itself; however, having the callbacks trigger correctly is at least manageable.

@cenzovit
Copy link

For this issue specifically, just from reading over the code, I am fairly certain the stream not closing is due to the cloning of the stream when setting the disconnect listener:

let stream_copy = stream.clone();

@ZerNico
Copy link

ZerNico commented Mar 22, 2024

I think I might have an idea, why this does not work. Because stream_inner is actually kept alive inside the callback of the disconnect listener (AudioObjectPropertyListener) the drop on it does nothing. At least that is an idea I got from testing around now. Even though the callback never get's called the drop works fine, if I remove the stream_copy.pause(); there. If it's added back the bug occurs again.

I really have no idea, how one might fix this, though.

Edit: actually just implementing the Drop trait and then pausing should work I suppose

@ZerNico ZerNico linked a pull request Mar 22, 2024 that will close this issue
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

Successfully merging a pull request may close this issue.

5 participants