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

PannerNode always crashes #369

Closed
ggadwa opened this issue Oct 9, 2023 · 8 comments
Closed

PannerNode always crashes #369

ggadwa opened this issue Oct 9, 2023 · 8 comments

Comments

@ggadwa
Copy link

ggadwa commented Oct 9, 2023

Machine: AMD, 16 GB, Windows 10 Home, Rust 1.7, web-audio 0.33.0

I knew web audio from a javascript 3d development environment I made, so thank you for this project! While working in rust, calling this anywhere in my code:

let panner = ctx.create_panner();

Will eventually cause (usually when it's called a second time):

thread 'cpal_wasapi_out' panicked at 'called Option::unwrap() on a None value', C:\Users\ggadwa.cargo\registry\src\index.crates.io-6f17d22bba15001f\web-audio-api-0.33.0\src\render\graph.rs:261:14

Note the panner is not connected to an output or connected from an input source, just calling create_panner() causes this. Note that if I instead use Panner::new with options the same thing happens, but more immediately (just when I call it the first time, or at least in my testing.)

Further notes: Creating a source, connecting it to an output, and playing it within the same code work perfectly; but within the same code creating a panner node will crash it, so it seems related specific to the panner node.

As I am new at Rust, it is possible this is some mistake on my part, i.e., a lifetime/de-allocation problem.

@orottier
Copy link
Owner

Hi @ggadwa, thanks for flagging this issue with us.
It definitely sounds like an issue in our library.
However I'm not directly able to reproduce it, could you share the full code snippet you are running?

@ggadwa
Copy link
Author

ggadwa commented Oct 10, 2023

Just in case it's a conflict in the libs I made this example by taking apart my game so the windowed loop remains, and then gathered a bit from one of your examples. Hit the button while this runs once and it'll play a sound and keep running, hit it the second time and it'll crash (for me, at least.) Three files, the cargo, the main, the lib. Hope this helps!

=== cargo ===

[package]
name = "webaudio_test"
version = "0.1.0"
authors = [""]
edition = "2021"

[lib]
crate-type = ["cdylib", "rlib"]

[dependencies]
cfg-if = "1"
anyhow = "1.0"
bytemuck = { version = "1.13.1", features = [ "derive" ] }
nalgebra = "0.32.3"
env_logger = "0.10"
pollster = "0.3"
log = "0.4"
tobj = { version = "3.2", features = ["async"]}
num-traits = "0.2"
wgpu = "0.17.1"
web-audio-api = "0.33.0"
winit = "0.28"
instant = "0.1"
serde_derive = "^1.0.156"
serde = { version = "^1.0.156", features = ["derive"] }
serde_json = "1.0.100"
rand = "0.7"

[dependencies.image]
version = "0.24"
default-features = false
features = ["png"]

[build-dependencies]
anyhow = "1.0"
fs_extra = "1.2"
glob = "0.3"

=== lib.rs ===

use winit::{event::*, event_loop::{ControlFlow, EventLoop}, window::{Window, WindowBuilder}};
use web_audio_api::{context::{BaseAudioContext, AudioContext}, node::{AudioNode, AudioScheduledSourceNode}};
use std::f32::consts::PI;

pub async fn run() {
env_logger::init();

let event_loop = EventLoop::new();
let window =
    WindowBuilder::new()
        .with_title("atom")
        .with_inner_size(winit::dpi::LogicalSize::new(1024.0, 768.0))
        .with_resizable(false)
        .build(&event_loop).unwrap();

// the audio context
let ctx = AudioContext::default();

// taken from webaudio example
let length = ctx.sample_rate() as usize;
let sample_rate = ctx.sample_rate();
let mut buffer = ctx.create_buffer(1, length, sample_rate);
let mut sine = vec![];

for i in 0..length {
    let phase = i as f32 / length as f32 * 2. * PI * 200.;
    sine.push(phase.sin());
}

buffer.copy_to_channel(&sine, 0);
        
// run the window loop
event_loop.run(move |event, _, control_flow| {
    match event {
        Event::WindowEvent {
            ref event,
            window_id,
        } if window_id == window.id() => {
            match event {
                WindowEvent::MouseInput {
                    button,
                    state,
                    ..
                } => {
                    if *state==ElementState::Pressed {
                        println!("sound");
                        let panner = ctx.create_panner();

                        
                        let src = ctx.create_buffer_source();
                        src.set_buffer(buffer.clone());
                        src.set_loop(true);
                        src.connect(&ctx.destination());
                        src.start();
                    }
                },
                WindowEvent::CloseRequested => *control_flow = ControlFlow::Exit,
                _ => {}
            }
        }
        Event::RedrawRequested(window_id) if window_id == window.id() => {
        }
        Event::RedrawEventsCleared => {
            window.request_redraw();
        }
        _ => {}
    }
});

}

=== main.rs ===

use webaudio_test::run;

fn main() {
pollster::block_on(run());
}

@orottier
Copy link
Owner

Thanks again.
I have managed to shrink the snippet to this, and now I can reproduce the error:

    let context = AudioContext::default();
    let panner = context.create_panner();
    drop(panner);
    println!("sleep");
    std::thread::sleep(std::time::Duration::from_secs(1));
    let mut panner = context.create_panner();
    println!("sleep");
    std::thread::sleep(std::time::Duration::from_secs(1));

For some reason, the drop and sleeps are necessary for the panic condition to occur.
I will have a look at it now, the PannerNode has some special treatment going on (the AudioListener is not added to the audio graph when no panners are present, for performance) and I suspect it has something to do with it.

@ggadwa
Copy link
Author

ggadwa commented Oct 10, 2023

Sorry for the big snippet, I wasn't sure what was potentially the problem. Note that in my game code, I am creating (and updating) a listener, and that code doesn't crash anything, without the panner create, if that helps at all.

Thanks for this library, and glad I could help with a bug report!

@orottier
Copy link
Owner

No worries, your snippet helped me narrow down the search.

I have actually found the issue: whenever a new AudioPanner is added, we connect the AudioListener to it in the audio graph to make sure the panner renderer has access to the listener position/orientation. But we don't account for the panner to go out of scope and drop from the audio graph. The listener will still have a connection to it which can no longer be resolved. This makes the application crash. I'm working on a fix now

@orottier
Copy link
Owner

I have prepared a fix in #370
@b-ma is there anything you would like to add to the discussion?
If not, I will probably merge this tomorrow and put out a new release

@b-ma
Copy link
Collaborator

b-ma commented Oct 11, 2023

Hey, no nothing particular to add on my side, except thanks @ggadwa for the feedback

If not, I will probably merge this tomorrow and put out a new release

ok (I might need some little help to figure out how to fix ircam-ismm/node-web-audio-api#37 then)

orottier added a commit that referenced this issue Oct 12, 2023
Fix #369 - AudioListener would have a connection to a dropped PannerNode
@orottier
Copy link
Owner

I have released version 0.34 just now.

@ggadwa there are some breaking changes in this release. Mainly most audio nodes must be mutable now, this is simply done changing

- let src = ctx.create_buffer_source();
+ let mut src = ctx.create_buffer_source();

@b-ma sorry I missed your call for help over at the node bindings. I will have a look. It will be a bit tricky indeed

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

No branches or pull requests

3 participants