Enable multithreaded consumers by changing Rc<RefCell<_>> to Arc<RwLock<_>>

[pfnsec]

First off, this library is great!

I wanted to propose a change that would allow pitch-detection to integrate with my current audio projects.

Consider the following example:

use std::sync::{Arc, RwLock};

use cpal::{traits::{HostTrait, DeviceTrait, StreamTrait}, Data};
use pitch_detection::detector::{mcleod::McLeodDetector, PitchDetector};

pub fn main() {
    let host = cpal::default_host();
    let device = host.default_input_device().unwrap();

    println!("Input device: {:?}", device.name());

    const SAMPLE_RATE: usize = 44100;
    const SIZE: usize = 512;
    const PADDING: usize = SIZE / 2;
    const POWER_THRESHOLD: f32 = 5.0;
    const CLARITY_THRESHOLD: f32 = 0.7;

    let detector = McLeodDetector::<f32>::new(SIZE, PADDING);
    let detector = Arc::new(RwLock::new(detector));
    let audio_detector = detector.clone();

    let config = device
        .default_input_config()
        .expect("Failed to get default input config");
    println!("Default input config: {:?}", config);

    let err_fn = |err| eprintln!("an error occurred on stream: {}", err);
    let data_fn = move |data: &Data, _info: &cpal::InputCallbackInfo| {
        let audio_detector = &mut *audio_detector.write().unwrap();
        let pitch = audio_detector
            .get_pitch(data.as_slice().unwrap(), SAMPLE_RATE, POWER_THRESHOLD, CLARITY_THRESHOLD);
        if let Some(pitch) = pitch {
            println!("{:?}", pitch.frequency);
        }
    };

    let stream = device
        .build_input_stream_raw(
            &config.into(),
            cpal::SampleFormat::F32,
            data_fn,
            err_fn,
        )
        .unwrap();

    stream.play().unwrap();

    loop {
        std::thread::sleep(std::time::Duration::from_secs(10));
    }
}

Using pitch-detection v0.3.0, we're unable to access the detector from our audio callback closure:

Rc<RefCell<Vec<f32>>> cannot be sent between threads safely

With this patch, the locking and refcount intrinsics are replaced with thread-safe equivalents, allowing our CPAL example to compile!

I've tested it on my local machine and it works correctly. All tests pass as well, and I've made sure to update the docstrings where appropriate. Let me know if you have any feedback.

[alesgenova]

Hi, thank you for the contribution!

I looked into it a little, and wanted to check a couple of things before merging.

I wonder if changing the type to Arc<RwLock<>> is overkill and worth the extra run time cost (but I don't know Rust very well, maybe the impact will be negligible).

If I understand it correctly, In this use case the detector doesn't really need to be shared across threads (i.e. all the computation is done in a single audio thread provided by cpal). Yet the problem is that the detector is created on the main thread, captured by the lambda, and finally moved to the audio thread deep into the cpal code.

Ideally we could find a way to create the detector on the audio thread and get a ref to it every time the lambda is executed.
I'm not sure if this is possible to achieve without changing the cpal API (for example you could pass an additional lambda to initialize some type of context, and then pass a getter to such context as a parameter of the data_callback).

A trivial way would be to create a new detector each time the data_callback is executed, but this is not a good option since we would be allocating/deallocating the buffers for each invokation.

If there is no way around it, I'll be happy to merge the PR, but first I'll need to make sure that the library would still work for example when it's built for WASM.

What do you think?

[alesgenova]

I think I was able to figure it out.
rust provides the macro thread_local! to define thread local global variables.
The example below builds and runs just fine with plain v0.3.0, and if I understand the way thread_local works, the detector should be created lazily the first time LOCAL_DETECTOR.with() is invoked on a thread.

use std::cell::RefCell;

use cpal::{
    traits::{DeviceTrait, HostTrait, StreamTrait},
    Data,
};
use pitch_detection::detector::{mcleod::McLeodDetector, PitchDetector};

pub fn main() {
    let host = cpal::default_host();
    let device = host.default_input_device().unwrap();

    println!("Input device: {:?}", device.name());

    const SAMPLE_RATE: usize = 44100;
    const SIZE: usize = 512;
    const PADDING: usize = SIZE / 2;
    const POWER_THRESHOLD: f32 = 5.0;
    const CLARITY_THRESHOLD: f32 = 0.7;

    thread_local!(static LOCAL_DETECTOR: RefCell<McLeodDetector::<f32>> = RefCell::new(McLeodDetector::<f32>::new(SIZE, PADDING)));

    let config = device
        .default_input_config()
        .expect("Failed to get default input config");
    println!("Default input config: {:?}", config);

    let err_fn = |err| eprintln!("an error occurred on stream: {}", err);
    let data_fn = move |data: &Data, _info: &cpal::InputCallbackInfo| {
        LOCAL_DETECTOR.with(move |detector| {
            let pitch = detector.borrow_mut().get_pitch(
                data.as_slice().unwrap(),
                SAMPLE_RATE,
                POWER_THRESHOLD,
                CLARITY_THRESHOLD,
            );
            if let Some(pitch) = pitch {
                println!("{:?}", pitch.frequency);
            }
        });
    };

    let stream = device
        .build_input_stream_raw(&config.into(), cpal::SampleFormat::F32, data_fn, err_fn)
        .unwrap();

    stream.play().unwrap();

    loop {
        std::thread::sleep(std::time::Duration::from_secs(10));
    }
}