#Better Audio

1 messages · Page 11 of 1

unborn tulip
#

hi hi, i'm moon! i work at processing. @faint wigeon is a close collaborator. I'm going to be folding bevy_seedling into our project. i'm not an audio expert, but i do work with audio regularly as an artist, and also have some experience with high performance audio (though it was done in swift). would love to help in any way.

After reading through the better audio in 2026 doc, I think I'll likely be able to add to documentation as I go. libprocessing's goals with audio will hopefully lead to some upstream contribution, and i'm also interested in the work of folding creek into firewheel

slate scarab
#

Wow, that's so exciting! Processing has a such a special place in my heart.

slate scarab
#

If you have any questions about bevy_seedling, please fire away! I'd be happy to help, there.

unborn tulip
#

thank you so much! also i've been really enjoying reading your code, it's so good! i will reach out with questions (:

toxic ingot
slate scarab
#

I'm curious what you think @dusky mirage. I've made a few nodes that can panic under the current API, such as the ircam hrtf node.

dusky mirage
#

Yeah, we need to figure out if the Firewheel context being !Send would be a problem for Bevy. If it is, then it might be worth spinning up a "clack plugin hosting" thread.

toxic ingot
slate scarab
toxic ingot
#

I've got a clap plugin node processing

#

I want to include a clap plugin in the repo so I can create tests. Is it fine license wise to pick like a plugin with an MIT license and include the clap bundles with they dynamic libraries? If so, would I include a copy for every platform?

#

Maybe I use NIH-plug to make a bunch to test different channel configs and such

#

If I do it in repo it'll just be per platform

dusky mirage
#

@toxic ingot Just use core::error::Error and core::fmt for no_std.

slate scarab
#

Thank goodness for core::error::Error stabilizing.

dusky mirage
#

Oh, did that not used to be stable? That sounds like a pain.

slate scarab
#

Yep, only in the last year I think, which is why we saw, for example, thiserror 2.0 roll out.

#

okay no, the year before
i may be old now

toxic ingot
slate scarab
#

It works for both

#

core is always available.

#

std often simply re-exports from core.

limpid mason
#

The std error is just a reexport of core error AFAIK

toxic ingot
#

gotcha

limpid mason
#

(There is a lint "forcing" you to always use the "lowest level" import, so you have maximum no-std comparability)

toxic ingot
toxic ingot
#

If I were to go the route of making a bunch of crates for clap plugins to test the host with, where would that best go?

#

All of them as members of the workspace?

dusky mirage
# toxic ingot All of them as members of the workspace?

Actually, maybe it would be better if it was in a separate repository. I don't want to pollute the workspace with too many dependencies. And once Firewheel is upstreamed into Bevy, we definitely don't want to be pulling in nih-plug into the bevy workspace, even if it is just a test dependency.

#

I actually wonder if Clap support would be best as a 3rd party node anyway. That is a quite a large test surface for the bevy team to maintain. What do you think @slate scarab and @celest whale ?

#

Though clap is technically a well defined standard, so maybe it fits the criteria to be included in bevy itself?

slate scarab
#

Yeah, to be clear a third-party crate is still highly valuable here! Not being included in Firewheel isn't the same as not being important.

celest whale
dusky mirage
toxic ingot
#

How does bevy feel about just the mere loading of arbitrary dynamic libraries? I feel like all sorts of issues can arise when plugins are malformed or misbehave

#

You may be right that it's better of as third party support if firewheel is getting upstreamed

dusky mirage
celest whale
toxic ingot
#

Honestly it works out cause I feel like I could have the test plugins as part of that repo without as much issue

dusky mirage
#

I actually designed Firewheel to be compatible with Clap's threading model from the get go. If we upstream, we need to make sure that the !Send requirement for the Firewheel context stays, and that the ordering of "deactivate processor, send processor to the main thread, and then drop the node on the main thread" cleanup strategy in Firewheel stays.

toxic ingot
#

Nice! Part of what I was going to look into next was if the drop ordering was going to be an issue cause I read somewhere in the clack docs that if you drop the PluginInstance before the processor then the memory would leak

#

or something like that

celest whale
dusky mirage
#

Ah yeah, a test would be a good way to enforce that.

unborn tulip
#

Does bevy_seedling run on top of bevy v0.19.0-dev ? processing is building on top of bevy main branch. i could help maintain an unstable latest branch of bevy_seedling if that's helpful

slate scarab
deft jasper
slate scarab
#

I’d be happy to turn this into a branch on the seedling side.

deft jasper
#

shouldn't be a tough bump. Nothing so far has been drastic, but if you want to take it over at some point just lmk

#

yeah I just ran cargo update and its still fine

#

so, nothing to do atm

dusky mirage
river marsh
dusky mirage
#

Uhh, I haven't decided yet.

river marsh
#

my instinct is to stay in control of that code

#

id like to know your original reason for not using an existing crate

unborn tulip
dusky mirage
deft jasper
dusky mirage
river marsh
#

i am not shy about it either :3

dusky mirage
#

That being said, the safety in Firewheel's audio buffer types isn't too hard to reason about. It's mainly just "the constructor ensures the invariants are met" kind of deal.

river marsh
#

the crate merging is gonna be a lot of churn, i'd like to hold off on that until we're reasonably certain that is the way forward. can we leave that for last?

#

theres a few experiments i want to do

dusky mirage
#

Ok, sure. That makes sense.

toxic ingot
toxic ingot
slate scarab
#

Okay @unborn tulip we have a bevy-0.19 branch up tracking Bevy main. Thanks Chris!

toxic ingot
#

Is it required that patches be fixed-length things with finite numbers of fields? I'm working on clap parameters now and those can differ from plugin to plugin. If I have something like a HashMap in my AudioNode then will I have to patch the entire map at once?

# 
#[derive(Debug, Clone, PartialEq)]
#[cfg_attr(feature = "bevy", derive(bevy_ecs::prelude::Component))]
#[cfg_attr(feature = "bevy_reflect", derive(bevy_reflect::Reflect))]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub struct ClapPluginParams {
    mapping: HashMap<String, f32>,
}

// Something like this?
// impl Patch for ClapPluginParams {
//     type Patch = ();
// 
//     fn patch(data: &ParamData, path: &[u32]) -> Result<Self::Patch, PatchError> {
//         todo!()
//     }
// 
//     fn apply(&mut self, patch: Self::Patch) {
//         todo!()
//     }
// }

/// A node that hosts a CLAP plugin
#[derive(Diff, Patch, Debug, Clone, Copy, PartialEq)]
#[cfg_attr(feature = "bevy", derive(bevy_ecs::prelude::Component))]
#[cfg_attr(feature = "bevy_reflect", derive(bevy_reflect::Reflect))]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub struct ClapPluginNode {
    params: ClapPluginParams,
}
#

My initial idea was to populate the map in info()...

slate scarab
#

The only requirement is that patches must be expressed as NodeEventType. You do not need to derive Diff or Patch.

#

That is, they can be implemented manually.

toxic ingot
#

Hmm, so I could manually and dynamically create a "patch" for a specific key of the map, I guess as long as elements aren't added or removed later

slate scarab
#

Yes. Since it includes a string, you'll likely need the Custom variant to express it, but it shouldn't be onerous or anything.

#

or Any I suppose

toxic ingot
#

hmm okay thanks

unborn tulip
toxic ingot
#

@slate scarab Is this sensible to you?

#[derive(Debug, Clone, PartialEq)]
#[cfg_attr(feature = "bevy", derive(bevy_ecs::prelude::Component))]
#[cfg_attr(feature = "bevy_reflect", derive(bevy_reflect::Reflect))]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub struct ClapPluginParams {
    // Mapping between CLAP parameter ID and that parameter's value
    mapping: HashMap<u32, f32>,
}

impl Patch for ClapPluginParams {
    type Patch = (u32, f32);

    fn patch(data: &ParamData, path: &[u32]) -> Result<Self::Patch, PatchError> {
        match data {
            ParamData::F32(value) => Ok((
                *path
                    .iter()
                    .next_back()
                    .ok_or_else(|| PatchError::InvalidPath)?,
                *value,
            )),
            _ => Err(PatchError::InvalidData),
        }
    }

    fn apply(&mut self, patch: Self::Patch) {
        assert!(
            !self.mapping.insert(patch.0, patch.1).is_none(),
            "Param map keys should not change"
        );
    }
}

impl Diff for ClapPluginParams {
    fn diff<E: EventQueue>(&self, baseline: &Self, path: PathBuilder, event_queue: &mut E) {
        for param in &self.mapping {
            param.1.diff(
                baseline
                    .mapping
                    .get(param.0)
                    .expect("Param list shouldn't change at runtime"),
                path.with(*param.0),
                event_queue,
            )
        }
    }
}

/// A node that hosts a CLAP plugin
#[derive(Diff, Patch, Debug, Clone, PartialEq)]
#[cfg_attr(feature = "bevy", derive(bevy_ecs::prelude::Component))]
#[cfg_attr(feature = "bevy_reflect", derive(bevy_reflect::Reflect))]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub struct ClapPluginNode {
    params: ClapPluginParams,
}
slate scarab
#

yes this looks reasonable
the expect would scare me a little but it's probably fine

toxic ingot
#

It's mostly a user convenience thing. If I remove the expectation that the map shouldn't change theoretically the user could populate the map themselves if they knew all the parameter IDs for the plugin they're using

#

I might have to instantiate it twice, once to query the params and fill the map and another for the actual processing

#

Is there a reason we couldn't have AudioNode::info take a mutable self?

slate scarab
#

I do feel like there's a lot of opportunity to rework this part, to be honest. Sorry to be vague, but my intuition is that we can make cases like these way easier with a few tweaks. I may get my thoughts down here at some point.

toxic ingot
#

hmm yeah, although this idea of the construction of the processor of a node setting up data on the node it was created from seems pretty specific to this case where there's this double indirection of parameters happening

slate scarab
#

Since we're gung-ho on Firewheel upstreaming, we may be able to author an API that allows way more flexibility during node construction.

#

That facilitates the ECS side of things a touch more.

#

And consequently makes something like this easier. We'll see, though.

toxic ingot
slate scarab
#

Correct. Although that kind of bidirectional data flow is not something I'm too excited about.

toxic ingot
#

In that you it doesn't particularly interest you as a feature or that it causes issues?

slate scarab
#

It's quite tricky. Our diffing naturally allocates frequently, so it's not really acceptable to call from the audio thread.

#

In the general case.

toxic ingot
#

I see

slate scarab
#

I see the appeal of course, but I'd really like to avoid it if we can.

toxic ingot
#

makes sense

steep dove
#

instead of constantly hitting the allocator?

slate scarab
#

Hm, maybe, although keep in mind you'd have to send that data back to the main thread.

#

Unless you just mean in general!

#

Most patches won't actually allocate (beyond pushing it to a list of patches), but a number of edge cases will trigger allocations.

steep dove
#

and when do the edge cases that cause allocations be dropped?

#

are they contained within the diffing algorithm?

#

or do they outlive it

slate scarab
#

A basic patch typically looks like this (pseudocode):

struct Patch {
    data: ParamData,
    path: ParamPath,
}

// On simple, flat data like this, the param path will not allocate
// and the param data will be simple floats:
struct Params {
    a: f32,
    b: f32,
}

// But for nested data, the path will grow beyond a single index
// and begin allocating.
struct NestedParams {
    a: Params,
    b: Params,
}

// Types that can't be directly represented in the `ParamData` enum
// will also require allocation (for the `Any(Box<dyn Any>)` variant)
struct AnyParams {
    a: ArcGc<str>,
}
#

We diff by recursively descending types, comparing field-by-field, pushing path indeces as we descend and patches when we hit changed leaves.

#

The ParamPath allocations are probably the trickiest bit, since we frequently clone the whole path type for nested structs.

steep dove
slate scarab
#

Ah, but to clarify, this is (currently) never called in the audio thread, and should never really be a problem for performance in the ECS. There are very few audio nodes after all.

steep dove
#

ah, ok, i saw "allocates frequently" and this part of my brain buzzed

#

bevy needs to start getting better at using pooling allocators for non-ecs stuff

slate scarab
#

Yeah I suppose I should say could allocate frequently depending on the shape of the type, though often does not.

#

We could also avoid a lot of incidental allocations by using a linked list on the stack for the parameter paths. However, I suspect this would end up a fair bit slower, so I don't think it's worth considering.

steep dove
slate scarab
#

No, u32 for each field (so we don't support paths into hashmaps for example).

#

a typical Diff / Patch derive looks like

pub struct LowPassNode {
    pub frequency: f32,
}


impl Diff for LowPassNode {
    fn diff<E: EventQueue>(
        &self,
        baseline: &Self,
        path: PathBuilder,
        event_queue: &mut E,
    ) {
        self.frequency
            .diff(&baseline.frequency, path.with(0u32), event_queue);
    }
}

pub enum LowPassNodePatch {
    // The `Patch` for a leaf like `f32` is just itself
    Frequency(<f32 as Patch>::Patch),
}

impl Patch for LowPassNode {
    type Patch = LowPassNodePatch;

    fn patch(
        data: &ParamData,
        path: &[u32],
    ) -> Result<Self::Patch, PatchError> {
        match path {
            [0u32, tail @ ..] => Ok(LowPassNodePatch::Frequency(
                <f32 as Patch>::patch(data, tail)?,
            )),
            _ => Result::Err(PatchError::InvalidPath),
        }
    }

    fn apply(&mut self, patch: Self::Patch) {
        match patch {
            LowPassNodePatch::Frequency(p) => {
                <f32 as Patch>::apply(&mut self.frequency, p)
            }
        }
    }
}

So you can see how we avoid allocations in Patch, which we typically run on the audio thread, while Diff runs free with them. The with method clones the path and appends the provided index.

river marsh
#

@dusky mirage i've run into the same dependency cycle thing with rtgc depending on bevy_platform. I think we have to include that in the upstreaming, it seems small enough. Does that sound reasonable or do you have a better solution in mind?

dusky mirage
slate scarab
#

Yes all my comments were addressed and it looks good to me!

river marsh
dusky mirage
river marsh
#

ty!

#

Is there anything else touching core or nodes that you wanna land before upstreaming do you think @dusky mirage

dusky mirage
river marsh
#

okay cool, that sounds good

dusky mirage
#

I'm going to be spending some time with my nephew today, so I'll get to it tomorrow!

river marsh
#

i may look at getting the version upgrades done as PRs

dusky mirage
river marsh
#

can you go over these? i can make some fix PRs

slate scarab
#

Aw, sorry, I could have redirected you to the simple-backend branch. It's much better.

river marsh
#

i did most of this back in february

slate scarab
#

Notably, it removes the horrendous context facade trait and removes the plugin generics in favor of resources as you mentioned you preferred int he past.

river marsh
#

ooooh

#

will this be hitting main?

#

whats the holdup

slate scarab
#

Firewheel publishing a new version with these changes, mainly

river marsh
#

i see

slate scarab
#

Though I could track git I suppose.

river marsh
#

so 0.19 -> firewheel release with all the audit cleanups and other changes -> seedling cleanups -> upstreaming?

slate scarab
slate scarab
river marsh
#

i think some of the things i found can be addressed now and dont have to wait

slate scarab
#

Yes I agree.

river marsh
#

ill look at that branch and see how it affects plans

#

ty

slate scarab
#

Damn I was hopnig no one would notice the duplicated code in queue.

#

You're too clever vero.

river marsh
#

lol

#

why do you want duplicated code

slate scarab
#

It's slightly different, but I had trouble factoring out the identical bits.

#

So I just kept it.

river marsh
#

i see

#

im more interested in deduping the effect normalization

#

its identical from what i can tell

#

not sure if the borrows are hard to deal with

slate scarab
#

Ah I see. I'm just taking a cursory glance now, I'll take a deeper look later today.

river marsh
#

no rush

slate scarab
#

EntityHashSet isn't ordered by insertions, right?

#

That is, it's not an IndexSet or equivalent.

#

Sorry, I'll make notes on the PR.

river marsh
#

ahhhh

#

didn realize that was a requirement

slate scarab
#

Yeah since the order of effects can have real differences on the processing.

river marsh
#

right yeah duh

slate scarab
#

Well, the other important point is that I wanted to use iter_many_unique_mut for the convenience trait, so a mere Vec<Entity> wouldn't satisfy the safety requirements.

river marsh
#

i just saw EntitySet and assumed ordering wasnt needed

slate scarab
#

I probably should have just made a SystemParam for this purpose though.

river marsh
#

can you update the docs to specify order is important

slate scarab
#

I'll add a note there.

river marsh
#

or maybe call it something that doesnt imply unorderednees

dusky mirage
#

Ok, I got the fixed-resample and symphonium dependencies updated!

#

Also while doing this, I realized it would make sense to add "OffsetSeconds" and "OffsetSamples" variants to EventInstant, where the event happens after the given amount of delay from the instant the Firewheel processor receives the event. This would be useful, for example, ensuring that a sequence of sampler events are triggered with the lowest latency possible without potentially cutting off the beginning of the first sample event. https://github.com/BillyDM/Firewheel/blob/7b8cbad500e7fe670050cbe9fe97c3795ec406c1/crates/firewheel-core/src/clock.rs#L45

#

I haven't done that yet, I just made a todo note.

dusky mirage
dusky mirage
slate scarab
dusky mirage
buoyant relic
#

Hey guys. Thanks for your work on audio! I have a quick question.
I was trying to work with bevy_kira_audio recently and just stumbled upon seedling/firewheel today. Skimming through the repos, seems like seedling is currently hard depended on symphonia for decoding? (symphonium can be opted out for firewheel but seedling is using it for SampleLoader). I was going to use ffmpeg-next for video decoding in my app, so I was thinking of using it to decode audio as well to get rid of the extra symphonia dependency. Please do tell me if anyone has any info on this approach.
Additionally, I'll need to add an additional unconventional codec as well, which can be done with symphonia codec registry (if sticking with symphonia), but seedling's SampleLoader is hard coding the default registry. (But well, I can technically workaround this by creating another loader I guess)

slate scarab
river marsh
#

symphonium feature gate was one of the audit comments i left iirc

buoyant relic
buoyant relic
#

I created 2 issues #102 and #103

GitHub
Symphonium integration should be feature-gated · Issue #102 · Cor...

Currently, seedling is hard depending on symphonia through firewheel-symphonium for audio decoding. We should make it possible to opt out of symphonia when disabling all the decoding features (wav,...

GitHub
Support adding new formats and codecs to SampleLoader? · Issue #10...

Currently, feature flags are being used to enable codecs in symphonia and the default symphonia codec registry is being used for decoding audio. If users want to support other codecs, they have to ...

dusky mirage
#

Hmm, the author of audioadapter still hasn't published the no_std fix.

#

@slate scarab Should I just go ahead and disable the no_std test in the CI for now and hope they get it updated before it is fully upstreamed into Bevy?

river marsh
#

sounds fine to me, we can always fork if its undermaintained. its not a really big crate right

#

oh its a relatively new crate

#

like a few months

dusky mirage
#

It sounds like they plan on publishing it. Maybe they just forgot over the weekend.

dusky mirage
#

Ugh, now GitHub CI is borking itself. https://github.com/BillyDM/Firewheel/actions/runs/23498327792/job/68385787138?pr=122 (This is the entire contents of crates/firewheel-graph/src/lib.rs:

#![cfg_attr(not(feature = "std"), no_std)]

pub mod backend;
mod context;
pub mod error;
pub mod graph;
pub mod processor;

#[cfg(feature = "unsafe_flush_denormals_to_zero")]
mod ftz;

#[cfg(feature = "scheduled_events")]
pub use context::ClearScheduledEventsType;
pub use context::{ActivateInfo, ContextQueue, FirewheelConfig, FirewheelContext, FirewheelFlags};

extern crate alloc;
#

Oh wait, maybe it's just because updating the CI in a PR doesn't change the currently running CI.

dusky mirage
#

No, that wasn't it. I'll try deleting the CI cache.

#

@slate scarab Are you ok with the API for bypassing nodes?

slate scarab
dusky mirage
#

You're welcome! I just keep thinking of little things I want to improve.

river marsh
#

yay audioadapter did the thing!

slate scarab
#

weeeeee

dusky mirage
dusky mirage
dusky mirage
#

Turns out I needed to fix no_std support for the other crates in the audioadapter repository as well.

slate scarab
#

alas
we shall continue to champion no std for the ecosystem in the footsteps of our dear leader bushrat (may he rest in peace)

slate scarab
#

Okay @dusky mirage, the bypassing is looking good from my perspective in terms of API! It should work great.

One thing that I think I'm observing is that bypass events scheduled in the past appear to be dropped. Is this expected? Ought I mitigate it with better scheduling?

rapid umbra
dusky mirage
slate scarab
dusky mirage
#

Hmm, it uses the exact same scheduler code that other events use, so I'm not sure why they could be getting dropped.

slate scarab
#

i can create a repro in a bit

dusky mirage
#

Though I'll go ahead and merge the bypass PR so I can work on other things without worrying about merge conflicts.

#

One change I want to make is to rename the "epsilon" constants and argument names for Volume, since they aren't technically being used as "epsilon" values. The only part that actually uses them as epsilon values is the smoothing filter, but I ended up using a different constant for that anyway.

dusky mirage
#

Ok, I added the ability to clamp graph inputs to silence if it is below a certain threshold.

Though note while this is functionally a noise gate, it is not a good noise gate and increasing the threshold may cause audible clicking. We should probably create a dedicated noise gate node at some point.

slate scarab
#

mhm

dusky mirage
#

There is one more thing I want to experiment with, and that is ability for nodes to opt-in to in-place buffer processing (where the input buffers and the output buffers are the same to improve performance in some cases). I'm not sure I will actually add support for that in the compiler, but it might be nice to have the functionality in place on the node author side so it is ready if we ever decide to add it.

fossil anvil
#

(the notable exception is no_mangle symbols, but rust-libm shouldn't be doing that)

dusky mirage
#

Hmm, I'm not sure. Still, it would be preferrable if libm just wasn't included in the dependency tree.

slate scarab
#

yes that's true

#

no need to compile it on std

fossil anvil
#

right, it's not ideal, but it is a per-clean-build cost, not a cost on the users of the binary, which is significantly different

dusky mirage
dusky mirage
#

After testing to make sure it works, I think that is all I wanted to do before upstreaming! (besides merging the crates)

river marsh
#

yay!

#

okay i will pick this up soon

#

is seedling using latest firewheel yet?

#

i think theres some stuff to look at fixing in seedling still actually

slate scarab
#

I can merge that tonight.

#

This includes the node fallibility and bypassing changes in particular.

river marsh
#

sick

#

tysm

dusky mirage
#

Firewheel now has a total of 24628 lines of Rust code! (excluding comments)

#

21313 lines of Rust code excluding the examples

#

That is significantly more than rodio (at 12341 lines of Rust code excluding examples and benchmarks). But hey, audio is important!

slate scarab
#

also, believe it or not, also more than seedling 🤭

dusky mirage
#

Ok, so the author of the audioadapter decided that a major version bump with breaking changes is probably the best move. (The only reason libm is required is because of a single square root operation in a less commonly used trait, and they want to change it to where it doesn't need that.)

This does mean we will also have to wait until rubato updates to the new audioadapter version, but they seem fairly keen on updating it too.

vocal dock
#

square root?

#

just use the fast inverse squareroot algorithm and inverse that

#

or even better

#

square the argument and do the fast inverse squareroot twice

slate scarab
#

nice

river marsh
#

dont normalize twice for no reason...!

dusky mirage
barren hare
#

With 5 iterations... how decadent...

#

I like how their initial guess seems to use a similar approach to Q_rsqrt, but makes the Evil Bit Level Hacking™ less obscure than in the C version

vocal dock
slate scarab
#

oaky seedling master is up to date with firewheel main

bold sequoia
# dusky mirage They actually ended up using newton's method: <https://github.com/HEnquist/audio...

This is going to be slower on many platforms, Q_rsqrt was around 3x slower than using the built-in sqrt method (which will emit the sqrt instruction + extra newton iterations on x86 with the correct features enabled) when I last tested it. It might be faster to use the bit-twiddling on the generic x86 target (which IIRC doesn’t have the sqrt instruction) but maybe worth using cfg(target_feature) to switch. If you use .sqrt it can also be trivially vectorised. YMMV of course though, check the decomp and llvm-mca

#

ARM also has a vectorised sqrt instruction

#

but I haven’t benchmarked it

dusky mirage
bold sequoia
river marsh
#

<3

river marsh
#

@slate scarab once you review and merge this, the only thing remaining before I do take 2 of the upstreamening PR is

  • TIMELINE_ID yeet
  • update_2d_emitters_effects et al deduplication (if its actually doable cleanly)
  • moving presets into examples (maybe)
#

id like it if you could do the TIMELINE_ID yeet

slate scarab
#

Yes yes! That should be no problem.

#

I'm not super happy to leave the presets behind since even basic projects will be compelled to include a bit of boilerplate.

#

that could certainly be fine of course

river marsh
#

okay

slate scarab
#

i think there are some places where i make things too implicit

river marsh
#

i think presets will be bsn eventually

#

so i dont really mind where we put them until then ig

slate scarab
#

as soon as we can represent graphs in bsn directly that would be nice

river marsh
#

yeah

slate scarab
#

Yes, it does throw away the vec allocation and create a new one for the arced slice, but subsequent clones become cheaper. These are typically cloned at least once as well, so I'd still consider it an overall win.

river marsh
#

oh thats nifty

#

i like it

dusky mirage
#

The new version of audioadapter with the no_std fixes have been released!

tender fiber
buoyant relic
#

Hey @slate scarab . I made a draft PR to support adding custom symphonia codecs https://github.com/CorvusPrudens/bevy_seedling/pull/109 . This, however, would require changing SymphoniumLoader to accept non-static registry and probe, which I don't think would be a problem in general? Limiting to only static registry kinda makes little sense for SymphoniumLoader::with_codec_registry_and_probe.
Maybe how the codecs config is handled in this PR can be improved. But tell me what you think

GitHub
Support adding custom `symphonia` codecs and formats by WataNekko ...

Resolves #103.

slate scarab
#

Hm, seems like the really tricky part is managing the enabled extensions.

#

We can also just leak anything that needs to be static. I don't think it'll matter.

buoyant relic
#

AssetLoader::extensions actually take &self so I just store the extensions in the loader itself

slate scarab
#

oh 😅 i am a silly bird

buoyant relic
#

well i thought the extensions was tricky as well until i realized it takes &self

#

not sure about the design however

slate scarab
#

i can take a closer look later today
the main thing that worries me is deferring the preregistration until PreStartup -- that means if, for whatever reason, someone attempts to load an audio asset before that system, it'll simply fail

#

but i think we could do it in the plugin without sacrificing the flexibility at all

buoyant relic
#

ye i was not sure about the prestartup

#

I just assumed all the asset loading should probably be after prestartup anyway (or i hope so)

#

i didnt put it in the plugin because then the user would have to insert the config before adding the plugin to have the loader preregistered with the extensions correctly

slate scarab
#

that's true, but that will only affect people registering codecs, which has to be far fewer than people who may want to load assets in pre startup for whatever reason

#

keep in mind you can also always replace the asset loader, so if you want to add more codecs afterwards, we could always just reinsert it

buoyant relic
#

true that

#

then i’ll put it into the plugin later

#

im just not sure about the behavior of preregister and registering a loader with different extensions, or reregistering another loader after that (for documentation)

slate scarab
#

any assets loaded before the extension is registered (or pre-registered) should fail

#

but otherwise it should be straightforward -- once the loader is replaced with more extensions, those files should be loaded correctly

buoyant relic
#

Technically, I guess seedling SampleLoader can take a static ref to the registry but then we'd have to delegate adding the default feature codecs to the user

slate scarab
#

Will that actually work in Bevy? If it's not static, we can't keep the loader in the world.

buoyant relic
#

wouldnt be a problem for me honestly, but just wouldnt be as flexible of an API (force user to work with static)

buoyant relic
#

the PR I made yesterday work with non-static fine

#

the loader is stored in the SampleLoader as well (along with the extensions)

buoyant relic
#

using static registry would just be SymphoniumLoader<'static>

slate scarab
#

Because it's constructed in the loader on every invocation. I think the intent is that we don't do that now that I'm looking at it.

buoyant relic
slate scarab
#

Not sure, but since there's a resampler cache it looks like it's supposed to be persisted. I still agree that they maybe shouldn't be required to be static.

buoyant relic
#

if we store the SymphoniumLoader as well then we should use static i guess. otherwise, self referential stuff

#

ye im not sure what the resampler cache is doing

#

ah. i guess otherwise we allocate new resamplers on every AssetLoader::load

#

hmm

slate scarab
#

Yes, which indeed seems not optimal!

buoyant relic
#

then maybe i should rethink the codecregistry reference

#

would caching the loader even work tho

#

we'd need some lock since SymphoniumLoader is not thread safe

#

and decode needs &mut self

slate scarab
#

Yeah that's probably why we did that huh.

buoyant relic
#

shame. i dont think we can optimally cache this unless Symphonium change the caching

dusky mirage
buoyant relic
#

hmm. for bevy, if we REALLY wanna optimize this then i guess we can ignore the symphoniumloader cache and just use decode_with_resampler to provide the resampler from some thread safe cache

#

but idk if this thread safe cache should be in symphonium or seedling

#

either way, if we want to optimize this then symphonium needs some changes

dusky mirage
dusky mirage
#

We could also add a trait the SymphoniumLoader that looks like this:

pub trait ResamplerCache: Default {
    fn resampler_mut(&mut self, key: ResamplerKey) -> &mut PacketResampler<f32, Sequential<f32>>;
    fn clear_cache(&mut self);
}
#

Though actually, it would probably need to be like this to get around mutex lifetimes and whatnot. fn with_resampler_mut(&mut self, key: ResamplerKey, f: FnOnce(&mut PacketResampler<f32, Sequential<f32>>));

slate scarab
#

We'll need some sort of interior mutability for the asset stuff (on the bevy side at least) since assets are loaded with a &self signature.

dusky mirage
#

Ok

#

I can easily add interior mutability to SymphoniumLoader. It makes sense since the mutability is only used for the cache.

slate scarab
#

Hm, not sure it's best that way tbh -- it might be better to do it in bevy since the loading is an async function. That is, we may want an async-aware mutex rather than a blocking one.

#

Not totally sure what we want exactly though.

dusky mirage
#

Hmm, I could also make it to where you pass in the resampler cache as an argument.

slate scarab
#

oh that might work blobthink

dusky mirage
#

Hmm, though on the other hand, it might be useful to sometimes have multiple copies of the same cache so you can load audio files in parallel.

slate scarab
#

which i suppose we can do if we pass the cache to it right

dusky mirage
#

@slate scarab How do you feel about this api?

pub trait ResamplerCache {
    fn with_resampler_mut(&mut self, key: ResamplerKey, f: &mut dyn FnMut(&mut PacketResampler<f32, Sequential<f32>>));
}

/// The default implementation.
pub struct DefaultResamplerCache {
    resamplers: HashMap<ResamplerKey, PacketResampler<f32, Sequential<f32>>>,
}

impl ResamplerCache for DefaultResamplerCache {
    fn with_resampler_mut(&mut self, key: ResamplerKey, f: &mut dyn FnMut(&mut PacketResampler<f32, Sequential<f32>>)) {
        let resampler = self.resamplers.entry(key).or_insert_with(|| {
            PacketResampler::new(
                key.channels,
                key.source_sample_rate,
                key.target_sample_rate,
                ResamplerConfig {
                    quality: key.quality.into(),
                    ..Default::default()
                },
            )
        });
        (f)(resampler);
    }
}

/// We don't actually need the `SymphoniumLoader` struct anymore since
/// `symphonia::default::get_codecs()` and `symphonia::default::get_probe()`
/// are actually just lazy statics.
///
/// Alternatively, we could add separate functions like `decode_with_registry()`.
pub fn decode(
    probed: ProbedAudioSource,
    config: DecodeConfig, // A new config struct I've added (contains `target_sample_rate`)
    codec_registry: &CodecRegistry,
    probe: &Probe,
    #[cfg(feature = "resampler")] cache: &mut dyn ResamplerCache,
) -> Result<DecodedAudio, LoadError> {
}
slate scarab
#

ya this seems good :3
although im not the biggest fan of changing signatures based on features!
i'd rather have two functions, one with the cache and one without

dusky mirage
slate scarab
#

oh nice! i can double check in a bit BongoBlob

buoyant relic
# dusky mirage What does it take to make the cache thread safe? If it's as simple as changing t...

i actually had to use a thread safe cache recently. basically ye, we need interior mutability since we work with &self. you either wrap RwLock<HashMap> or use a specialized concurrent map. From what I researched, dashmap is a popular crate for this, which split the map into multiple shards, each with their own rwlock to reduce contention. I dont know if we wanna add a dependency for this tho. Either way, RwLock should be used rather than Mutex cuz read is far more than write in this case.

#

i like the trait approach to delegate this to user. but should it actually be &dyn Cache to make it safe to share between thread?

slate scarab
#

Yeah probably not worth the dep unless it's already in the tree.

dusky mirage
#

So do you think it's good to publish the new symphonium version now?

slate scarab
#

I haven't had the opportunity to review if you'd like that (still working!)

dusky mirage
#

Ok, I'll wait then.

#

Actually, I wonder if it would be worth adding an option to encode resampled data into a 16 bit format with dithering to reduce memory usage? That shouldn't be hard to add.

slate scarab
#

I think we'd be better served by focusing on lazily decoding while playing (for sounds where it makes sense) -- this will be orders of magnitude less memory rather than just halved!

That is, I'm not sure a 16-bit option will be that useful overall. But I could be wrong.

dusky mirage
#

Fair. For music and ambiance, lazily decoding definitely makes sense. But for sound effects, it makes sense to store them in memory anyway, and doing this will halve that memory.

slate scarab
#

hm, possibly so

dusky mirage
slate scarab
#

oaky :3

buoyant relic
# dusky mirage <@164224139316428800> Ok, what do you think of this new api? <https://codeberg.o...

i just had a look at this.

rwlock should probably be used instead of mutex like i mentioned. because cache usually has more read than write (write to cache once and read the cached value many times to reuse). using rwlock: the first time a resamplerkey is used, we obtain exclusive ref for a single thread to the map to insert the resampler. subsequent use of the same resampler would obtain shared ref from the lock, which can be shared concurrently between multiple reading threads. using mutex here would unnecessarily get exclusive ref to a cached resampler every time, which blocks all other threads while they just need to read the same value without modifying as well.

and ResamplerHashMap should probably be HashMap<Key, Arc<Resampler>> so that we can: get ref to map value, clone the resampler arc, and release the map lock right away. this way, we dont hold the lock until the resampling is done. otherwise, with mutex its even worse, each thread locks until its done resampling, essentially making the concurrent resamplings sequential. with rwlock its a lil less bad, a single write thread blocks other threads but multiple read threads block other write threads. but yeah, using arc would prevent holding the map lock while resampling.

and rethinking this, do we actually need the cache api at all? cant the api just require passing in the resampler? and the user can choose to cache or not cache, or cache with async thread safety for example. currently the Cache trait api would not work with async locks anyway, since with_resampler_mut would probably need to be async for that case. i think this can just be left to the caller. symphonium should just provide a simple default cache impl for getting the resampler, while the api just requires the resampler.

#

cuz honestly it depends on how you cache it. for example, wrongly caching with the mutex and holding the lock through resampling would probably be slower than allocating a new resampler everytime without cache. each application probably has different strategy for optimizing

#

and for single thread apps, u wouldnt need cache with locks at all (while the Cache trait assumes thread safe locks with interior mutability &self)

dusky mirage
slate scarab
#

cant the api just require passing in the resampler?
this does leave open the possibility for incorrectness -- you'd have to make sure to pass the correct resampler in

#

and ResamplerHashMap should probably be HashMap<Key, Arc<Resampler>>
the resamplers requires mutable access to resample!

#

it's definitely a tricky problem space

buoyant relic
buoyant relic
#

then quite tricky cuz allocating the resampler every time may be faster than locking throughout resampling (dont take my words for this tho, only benchmarking would tell)

buoyant relic
#

so i think sharing cache between concurrent threads doesnt make sense in this case if each thread needs to modify the resampler state

#

maybe we should scrap the thread safe cache

dusky mirage
#

We could have a per-thread cache.

dusky mirage
#

Though we could also just experiment to see if just creating a new resampler every time is fast enough.

#

Hmm, though the default FFT resampler does seem to allocate quite a bit of data.

pub fn new(fft_size_in: usize, fft_size_out: usize) -> Self {
    // calculate antialiasing cutoff
    let cutoff = if fft_size_in > fft_size_out {
        calculate_cutoff::<f32>(fft_size_out, WindowFunction::BlackmanHarris2)
            * fft_size_out as f32
            / fft_size_in as f32
    } else {
        calculate_cutoff::<f32>(fft_size_in, WindowFunction::BlackmanHarris2)
    };
    debug!(
        "Create new FftResampler, fft_size_in: {}, fft_size_out: {}, cutoff: {}",
        fft_size_in, fft_size_out, cutoff
    );
    let sinc = make_sincs::<T>(fft_size_in, 1, cutoff, WindowFunction::BlackmanHarris2);
    let mut filter_t: Vec<T> = vec![T::zero(); 2 * fft_size_in];
    let mut filter_f: Vec<Complex<T>> = vec![Complex::zero(); fft_size_in + 1];
    for (n, f) in filter_t.iter_mut().enumerate().take(fft_size_in) {
        *f = sinc[0][n] / T::coerce(2 * fft_size_in);
    }

    let input_f: Vec<Complex<T>> = vec![Complex::zero(); fft_size_in + 1];
    let input_buf: Vec<T> = vec![T::zero(); 2 * fft_size_in];
    let output_f: Vec<Complex<T>> = vec![Complex::zero(); fft_size_out + 1];
    let output_buf: Vec<T> = vec![T::zero(); 2 * fft_size_out];
    let mut planner = RealFftPlanner::<T>::new();
    let fft = planner.plan_fft_forward(2 * fft_size_in);
    let ifft = planner.plan_fft_inverse(2 * fft_size_out);
    fft.process(&mut filter_t, &mut filter_f).unwrap();
    let scratch_fw = fft.make_scratch_vec();
    let scratch_inv = ifft.make_scratch_vec();

    FftResampler {
        fft_size_in,
        fft_size_out,
        filter_f,
        fft,
        ifft,
        scratch_fw,
        scratch_inv,
        input_buf,
        input_f,
        output_f,
        output_buf,
    }
}
dusky mirage
#

Also, nevermind on the dithering to 16 bit thing, it turned out to be more complex than I thought to integrate.

#

Though I also realized that it may be possible to cache decoders too, not just the resamplers. Though I might save that for a later date.

I could rename ResamplerCache to SymphoniumCache, and then add decoder caching later.

slate scarab
#

Yeah maybe some benchmarking is in order. I can do some testing today ferrisOwO

buoyant relic
#

another approach i thought of is maybe the cache is a pool of resamplers. if a specific resampler needed is already in used by another thread then we create a new one, otherwise just take 1 of the available one in the pool. but idk if that extra complexity even pays off 🙂 maybe per thread cache is enough

#

again, i think maybe leaving this decision to the user is better idea (as in the main api doesnt require cache, just a resampler). each system has its own quirks and only benchmarking for a specific application would tell for that system.

slate scarab
#

Okay so on my machine, I'm seeing some 23us of overhead for reconstructing the resampler every time:

cache metrics/without cache/44100
                        time:   [880.14 ns 885.73 ns 892.82 ns]
cache metrics/with cache/44100
                        time:   [865.29 ns 868.97 ns 873.92 ns]
cache metrics/without cache/48000
                        time:   [29.967 µs 29.998 µs 30.036 µs]
cache metrics/with cache/48000
                        time:   [6.0377 µs 6.0496 µs 6.0608 µs]
#

This is for resampling a 44.1kHz 1ms wav file.

#

Note that for longer files, and especially vorbis, this overhead becomes essentially zero.

vorbis cache metrics/without cache/48000
                        time:   [2.9538 ms 2.9566 ms 2.9598 ms]
vorbis cache metrics/with cache/48000
                        time:   [2.9365 ms 2.9392 ms 2.9421 ms]

That's 2.9ms for a ~2 second ogg vorbis at 44.1kHz on my machine. Just thought I'd reinforce that decoding even short files ahead of time isn't a great idea for performance!

slate scarab
#

Still a touch ugly but at least there are way fewer systems and they're now more robust.

#

With that, I think master's pretty close to addressing everything in your first audit pass.

river marsh
#

yayyyy

#

tysm

slate scarab
#

ofc!

dusky mirage
#

@slate scarab Ok, I was busy this Easter weekend, but I'm back at it now! What do you think of this new API? I made it so caching is optional, and that the default cacher is single-threaded only unless you enable the new "multithreaded-cache" feature. The new multithreaded caching logic can now have up to a set number of concurrent caches at a time.

slate scarab
#

oh nice! i can look soon :3

slate scarab
#

Hm, the multithreaded setup ended up being a lot of machinery, huh!

One thing that could make this easier in the context of Bevy is relying on thread locals. That should allow us to work with !Sync types and help avoid a ton of synchronization overhead.

In that world, I imagine simply taking a mutable reference to a concrete cache type might be more straightforward. In our asset loader, for example, we could do something like:

thread_local! {
    static CACHE: RefCell<ResamplerMap> = const { RefCell::new(ResamplerMap::new()) };
}

CACHE.with(|cache| {
    let mut cache = cache.borrow_mut();
    
    /// ... then use the cache via a mutable reference
});

The only downside I'm aware of is the fact that we effectively leak these caches, particularly if the target sample rate changes a few times. I think in practice this won't make too much of a dent, and it should be the best performing option.

#

My intuition is that we can land on a nice API here without necessarily needing a trait -- a simple &mut ResamplerHashMap may be good enough!

In any case, if you'd like to continue down the current trajectory, I'd actually recommend splitting out the default cache into two types (just like my recommendation for the signatures)! In my opinion, it's more ergonomic for folks to switch between, say, a LocalResamplerCache and a ResamplerCache than to adjust Send/Sync-ness via features. (You could of course feature-gate one or both still!)

#

Sorry apparently we have completely opposite schedules for this work 😅 I keep missing you during the day.

dusky mirage
slate scarab
#

Yeah for the library itself that might be controversial. But as consumers of the API in bevy_seedling, I think we can probably justify it blobthink

#

And in bevy_seedling can always decide to use the multi-threaded cache approach if we decide the thread locals are no bueno. That is, making the API take a mutable reference to a cache doesn't prevent us as users from doing that.

dusky mirage
slate scarab
#

Oh I just mean it could take a &mut ResamplerHashMap right? The caches could have a get_mut() or something that returns a ResamplerHashMap I think.

dusky mirage
#

Oh, you mean keep the trait?

slate scarab
#

hold on just a sec -- sorry for not being clear haha

slate scarab
#

oh im back

#

oaky

slate scarab
#

I could imagine an API like this (pseudocode):

let mut local_cache = LocalCache::new();
let resampler_map: &mut ResamplerHashMap = local_cache.get_mut();

let local_result = symphonium::decode(
    probed,
    decoder_config,
    target_sample_rate,
    Some(resampler_map),
    None,
);

let sync_cache = SyncCache::new();
let resampler_map: Arc<Mutex<ResamplerHashMap>> = sync_cache.get();

let sync_result = symphonium::decode(
    probed,
    decoder_config,
    target_sample_rate,
    Some(&mut resampler_map.lock().unwrap()),
    None,
);

does that make any sense?

slate scarab
dusky mirage
slate scarab
#

This allows for the same solution you proposed, no? sync_cache.get() could have slots the same as in your implementation.

#

Ah, sorry the exact type names may not match up.

dusky mirage
#

Oh ok, I see what you are saying now.

slate scarab
#

One question is if you can reasonably get the ResamplerKey with such an approach, I think.

#

I suppose it doesn't matter, really. With the trait approach, we could store a

thread_local! {
    static CACHE: LocalSymphoniumCache = const { LocalSymphoniumCache::new() };
}
#

With that in mind, I think your trait approach is probably totally fine.

slate scarab
dusky mirage
slate scarab
#

Slots could be claimed by atomic flags, could they not?

#

In any case, it seems annoying.

dusky mirage
#

I mean I guess we could just do the thread local approach and have a separate hash map for each thread. Though that approach will potentially leak memory, and there is no way to create a "clear cache" method for it.

slate scarab
#

Give that it's a Arc<Mutex<HashMap<ResamplerKeyInner, Arc<Mutex<PacketResampler<f32, Sequential<f32>>>>>>> (wowie haha), why can't you simply

drop(resampler_cache);
return resampler;
#

(You should be able to return a MutexGuard<T>, although maybe the lifetime is weird.)

slate scarab
dusky mirage
#

Hmm, though actually, I suppose there is no reason that we couldn't just have the multithreaded cache be the only version. It would be functionally the same as a standard hash map if you create one per thread. The mutexes would just never encounter contention.

slate scarab
#

I suspect the overhead will be noticeable, though I can't back this up without benchmarking.

dusky mirage
#

I thought mutexes had very little overhead when there is no contention?

slate scarab
#

Hm, don't we expect some contention when extracting the resamplers?

#

Certainly it's a shorter operation than holding a lock while resampling! I can't deny that.

dusky mirage
#

Yeah, I suppose that is a good point, I don't know how efficient my implementation actually is.

slate scarab
#

I think I could get a bencharmking test going tonight!

dusky mirage
#

Though actually, it might be possible to achieve the same effect using a RwLock and atomic flags instead of mutexes.

dusky mirage
slate scarab
#

ya if it's not saturated (which is surely most of the time) then you don't need a writer lock

dusky mirage
#

Ok, I made the outer mutex into a RwLock!

buoyant relic
#

btw, how does the thread local approach leak memory? i keep seeing you guys mentioning it but dont see how

fallow current
#

Thread local basically puts ownership outside structs and methods, something you do not drop. So if you put collections into thread locals you actively have to think about how large it can get and what you do to limit it.

I like t o use bevy's Parallel because it does that itself when dropped, also I have an per-instance thread local unlike std::thread_local!.

#

weird that according to the docs Parallel does not impl Drop, the wrapped type certainly does. Maybe rust docs do not include implicit Drop impls

buoyant relic
#

is it just like a static variable then? wouldnt it be roughly the same if the caches in the loader dont get cleared and just stayed there? cant we just have a fn clear_cache for the thread local caches?

fallow current
#

not sure how you could clear cache of another thread as each only sees their own

#

could do the nuke variant and first spawn as many thready as physically possible and have each check if there is something to clear

#

it is basically not "a static variable" but "a static variable exclusive to each thread"

#

maybe if the thread terminates this drops the local but idk

buoyant relic
#

okay. i was under the impression that the struct is dropped when the thread terminates. and if the thread still lives then it can clear its cache. thats why i dont get why everyone keeps saying leak.

https://doc.rust-lang.org/std/thread/struct.LocalKey.html
According to the doc, only some caveats about destruction exist, but all of those from what i read seems to imply the process gets terminates anyway and the OS would reclaim all memory?

LocalKey in std::thread - Rust

A thread local storage (TLS) key which owns its contents.

slate scarab
#

Unless I'm mistaken.

buoyant relic
#

yeah if the thread sticks around then it can clear its own cache, no?

#

thats why i dont get why theres leaks

slate scarab
#

It's not like we can signal to every thread "hey clear this" in user space.

#

Bevy will just choose a thread for any particular asset loading task.

buoyant relic
#

but then its the same if we store a multithreaded cache in the loader. the cache would eventually gets big without clearing

slate scarab
#

The thread locals are opaque -- each thread sees a different value and we have no control over which thread is running.

buoyant relic
#

hmm. no, im just confused about “leaks”. cuz as i see it, thread local doesnt produce leaks

slate scarab
#

Bevy is kinda leaky in general, to be honest. There's no shrink method for the world like in flecs. So this wouldn't be the worst leak ever. But the user could not at any point clear up the caching we're doing in the asset loader if we use thread locals.

#

Given there are only so many threads, I doubt it would end up being a real problem. I don't think the resamplers are that huge either.

#

So that's why I think it's acceptable for a Bevy app.

buoyant relic
#

then when would we clear the cache for a multithreaded cache that we have control over? thats what i mean by isnt it the same thing as the thread local

slate scarab
#

When the sample rate changes, for example!

#

Otherwise we'll have essentially useless resamplers sticking around.

buoyant relic
#

hmm. okay i kinda see it now

#

but well, honestly im not sure. thats why i said before i dont know if the multithreaded cache complexity is worth the trade off, or maybe thread local is enough

slate scarab
#

I still suspect a thread local will perform better, with the stickiness being the only downside imo.

buoyant relic
#

can we, like, register an event handler for the thread local cache, to be cleared when sample rate change occurs?

#

i guess that would require some locks for the thread local cache, which idk would make sense or not 😅

buoyant relic
fallow current
#

thread locals need at most a RefCell, not a full blown locking container

#

nothing needs to be concurrent

#

because you cannot access the value of another thread

#

thread locals are like [RefCell<T>; NUMBER_OF_THREADS] and you (current thread) only get one specific &RefCell<T> out of it

buoyant relic
#

ye. i was replying to my idea of registering an event handler to clear cache, which would happen on another thread and would need lock in that case (which sounds stupid but in the end, it just acts like a spread out concurrent maps)

fallow current
#

when T gets large, because it is a vector of bytes for example, and the thread is now owned by something that does not care for it, you basically leaked the memory until bevy closes

#

if your event handler logic does not grasp this thread again, it has no chance to clear it

slate scarab
#

unless the thread local is using a Sync primitive that you share upon creation
in which case.....

#

why hehe....

buoyant relic
fallow current
#

use thread locals that contain all an Arc of the same Mutex 😎

tender fiber
#

mpsc to send messages between threads is also possible.

buoyant relic
#

we need the event handler logic (clearing cache) to be run on the individual threads. a message wouldnt do anything if we cant control exactly what gets run on what thread (in the bevy thread pool)

#

but hmm. is there a way to schedule a task to run on all threads in the bevy thread pool?

#

or maybe a message works like: the main thread broadcast a sample rate change message to all threads. then a thread running the get resampler logic would first check and clear cache if theres a message

#

is this over engineering? 😂

tender fiber
#

Over engineering would be both.

dusky mirage
#

@slate scarab Ok, for the sake of getting this moving, how do you feel about the API now? (I made it so there is now a Cache and a "SyncCache".) We can always improve the implementation of the Sync cache later. https://codeberg.org/Meadowlark/symphonium/pulls/3

slate scarab
#

Yeah, sorry for my sluggish pace on this (again)! I think this is a reasonable step forward, and should allow us to do whatever we want in user space as well.

dusky mirage
#

Ok, so do you think it's good to publish now?

slate scarab
#

I haven't integrated it yet, which is usually where I find issues. But it's easy enough to bump this crate's version, right? So I wouldn't worry too much I think!

#

Or does it interact with firewheel-symphonium? That might be annoying blobthink

dusky mirage
#

Yeah, the API has changed, so I would need to update that as well.

slate scarab
#

I should be able to do a quick integration this evening, though I can't guarantee it'll happen tonight. But again, don't feel the need to wait on my behalf. I guess we're still blocked on other stuff in general for the upstreaming in any case.

dusky mirage
slate scarab
#

yay!!

dusky mirage
# slate scarab yay!!

Actually, now that I think about it, I'm not sure it really makes sense to have the load_audio_file convenience methods in firewheel-symphonium. Do you think it would be better just to have users use the symphonium methods directly?

#

That way all of the configuration options would be available to them.

slate scarab
#

mm ya it's probably totally fine to not have too many layers of convenience

dusky mirage
#

Ok, I'll change that then!

dusky mirage
#

Alright, it is changed!

dusky mirage
slate scarab
#

oh yay!!

#

im excited to add metrics to bevy_seedling with this BongoBlob

dusky mirage
#

Let me know if the API works for you!

slate scarab
#

yes yes i think it should work well!

dusky mirage
#

So do you think it's good to merge into main?

slate scarab
#

from my cursory review I'd say so

dusky mirage
slate scarab
#

Oh, I didn't realize you had made a PR in Firewheel! Today should actually be a great day to review both of these in depth!
-# after taxes bleghhhh

dusky mirage
#

Oh whoops, well I already merged the performance profiling one since it had merge conflicts with the other one I had to fix.

slate scarab
#

Oh no worries, I can provide feedback in an issue or a followup if anything comes up

dusky mirage
#

I have also been having a freezing issue with CPAL input on my system (which is why I disabled it in the visual_node_graph example for now).I need to look into that.

dusky mirage
#

Glad I don't have to do any debugging! 😅

#

Also dang, the convolver node is very slow when compiled in debug mode (it is using about 32% CPU time in debug mode, and about 6% CPU time in release mode).

I guess adding this didn't help:

# The convolver is very slow without full optimizations
[profile.dev.package.fft-convolver]
opt-level = 3
#

OH, FFTConvolver has a generic, so I guess it gets compiled inside the firewheel-nodes crate. That's annoying.

#

(I'm already compiling with opt-level=2 for all dependencies in debug mode, so I guess the convolver really needs opt-level = 3 for some reason.

slate scarab
#

yeah i always just throw 3 on there even for testing

dusky mirage
#

Ok, yeah, adding this fixed it:

[profile.dev.package.firewheel-nodes]
opt-level = 3
dusky mirage
slate scarab
#

waow!

dusky mirage
#

But I do think I learned more about how taxes work. I also realized I should keep better track of my income and business-related expenses.

slate scarab
#

(i like freetaxusa, it's pretty much always better for taxes that aren't complicated and is currently not particularly predatory)

dusky mirage
#

Oh, cool. Thanks for the recommendation!

slate scarab
#

although state filing is still some money, if you happen to need that
so
that might also eat up any returns hehe

dusky mirage
#

Ok, well I guess I could get $10 from federal tax returns.

dusky mirage
slate scarab
#

yay!!

dusky mirage
#

Oh wait, it got rejected because I don't have an "IP PIN". Ehh whatever, I don't need the 10 bucks that bad.

slate scarab
#

oh

#

can you give it to me

#

i dont need it either

rapid hedge
#

What’s the state of the art in terms of reading MIDI controllers into Bevy?

#

I assume using midir in PreUpdate?

#

not sure which channel is best to ask this in but I feel like this one contains the biggest number of people that may have done this before hehe

river marsh
#

yeah thats what ive done

limber kernel
#

please tell me u are making a game controller with ur midi keyboard

rapid hedge
#

But I'm considering writing a crate that translates midi input into BEI actions

#

That way you should be able to do that

steep dove
#

I literally know so many people doing this with bevy

rapid hedge
rapid hedge
limber kernel
#

oh are you gonna do some dmx stuff too?

rapid hedge
#

But I can see adding it anyways

hushed widget
#

I had some fun with midi controllers and making music with Bevy a few ...years ago and going through Bevy for the audio was too slow, so I had to bypass Bevy for the midi->audio part but was still sending the midi event to Bevy for screen effects

hearty bear
slate scarab
#

But in terms of the impact to bevy_seedling, it's all good!

#

If the only goal or purpose behind it is to implement some traits and provide some inherent methods, you could convert the inherent methods into a new extension trait. That may or may not make sense here, but it's certainly an option.

#

I suppose you're stuck if you need to implement some traits blocked by the orphan rule. I didn't cehck for that.

dusky mirage
#

Though I agree the naming is a bit confusing. Maybe I should think of a better name.

slate scarab
#

it's kind of a tough spot to be fair

#

couldn't symphonium implement traits from firewheel-core? what role is firewheel-symphonium serving here? just curious!

dusky mirage
slate scarab
#

Optionally depending on a core crate from Bevy doesn't seem too problematic to me personally. I think there's an unnecessary stigma there! It wouldn't need to depend on all of bevy -- just the low-level audio crate.

dusky mirage
#

Though now that I think about it, it might not be a bad idea to create a "sample resource" crate, similar to what audioadapter does for buffers.

slate scarab
#

Correct me if I'm wrong.

dusky mirage
#

The problem is that whenever bevy updates, we will have to update symphonium as well.

#

It's the same cyclic dependency problem that led us to decide to upstream Firewheel in the first place. (And also why I removed the rtgc dependency).

slate scarab
#

ugh

barren hare
dusky mirage
#

Ok, I may have been overengineering a bit and creating a more fleshed out "sample-resource" crate. But I realized when trying to make an abstraction for sample resources that can be streamed from disk/network, the sample resource would have to be !Sync for that to work. That can be done, but it does mean we will need to think of how to handle that in the sampler node. I'm thinking of a sort of "SampleResourceHandle" trait that "spawns" a new resource that is then sent to the audio thread.

dusky mirage
#

How exactly are assets handled in bevy again? How is a handle to say, a texture or a material implemented?

slate scarab
deft jasper
dusky mirage
#

@slate scarab Hmm, I'm realizing this is actually kind of a hard problem to solve generically. But here is roughly the idea I have:

// Send + Sync + Clone
//
// For resources that already have all data loaded into memory, the struct
// implementing this trait will just contain an `ArcGc<dyn SampleResource>`.
pub trait SampleResourceHandle {
    // We might be able to turn the second item into an `OwnedGcUnsized`
    // in Firewheel.
    //
    // Resources that already have all data loaded into will will return `None`
    // for the first item.
    fn spawn_resource(&self) -> (Option<Box<dyn SampleCacheWorker>>, Box<dyn SampleResource>);
}

/// A "server" responsible for streaming samples from disk/network and then
/// sending them to the corresponding `SampleResource`.
///
/// This is technically a "Future" (I think). I'm not sure how futures/async
/// is handled in bevy.
///
/// Send + !Sync + !Clone
pub trait SampleCacheWorker {
    /// Tick the server.
    ///
    /// (Maybe this could be async?)
    fn tick(&mut self);
}
# 
/// Send + !Sync + !Clone
//
// For resources that already have all data loaded into memory, the struct
// implementing this trait will just contain an `ArcGc<dyn SampleResource>`.
pub trait SampleResource {
    /// Fill the buffer
    fn fill_buffer(&self, buffer: &mut [f32], start_frame: u64, channel: usize) -> usize;

    /// Returns caching information about this resource, like the optimal cache
    /// block size and the maximum supported playback speed.
    ///
    /// Resources which already have all data loaded into memory return `None`.
    fn caching_info(&self) -> Option<CachingInfo> {
        None
    }

    /// Returns `true` if the given range in the sample resource is in memory and
    /// ready to be read from.
    fn range_is_ready(&self, range: Range<u64>) -> bool {
        let _ = range;
        true
    }

    /// Request the corresponding `SampleCacheWorker` to cache the given region
    /// of frames.
    ///
    /// The sampler node will automatically request to cache regions based on
    /// the current state (i.e. the start of a "loop range" will be cached).
    fn cache_region(&self, region: Range<u64>);

    /// Request the corresponding `SampleCacheWorker` to delete the previously
    /// requested range.
    fn delete_cache_region(&self, region: Range<u64>);

    /// Notify the sample resource the speed and the direction that the playhead
    /// is moving. This can be used to help the `SampleCacheWorker` make optimal
    /// streaming decisions.
    fn notify_playhead_behavior(&self, is_playing: bool, is_playing_backwards: bool, speed: f64) {
        let _ = behavior;
    }
}
#

Though another hard problem is that it would be optimal to also "pre-emptively" request to cache regions before the sampler node actually receives the event to change the playhead position and/or start playing.

slate scarab
#

yes yes okay!! today i can vet back to this finally
gimme a sec

dusky mirage
dusky mirage
slate scarab
slate scarab
#

Because you'd want some variation on a ringbuffer structure or similar?

dusky mirage
slate scarab
#

mhm mhm

slate scarab
#

For example

//! streaming_loader.rs
struct StreamingCacheWorker(/* */);
struct StreamingSampleResource(/* */);

// this is a touch different because there's no handle struct,
// but i'm sure you get the idea
fn stream_file(path: String) 
    -> (StreamingCacheWorker, StreamingSampleResource);

consumers could freely chose whether they want to even perform type erasure or not

#

In other words, it may not be worth fully abstracting over streaming vs in-memory resources. A worker trait and expanded sample resource trait may be enough.

dusky mirage
slate scarab
#

Yeah part of my thinking there is that we (in bevy_seedling) may want direct control over that particular bit of abstraction, given how it interacts with the asset system in Bevy.

slate scarab
#

Oh by the way feel free to merge the symphonium update! The dependency issue is worse than newnewtypes hehe

#

@dusky mirage
if that was your only holdup

dusky mirage
#

Though yeah, I kind of also realized that having a generic "sample resource" crate doesn't make all that much sense because it depends a lot on how the specific sampler engine is implemented.

#

(Also yay, tornado watch time 🌪️ )

dusky mirage
slate scarab
#

oh yes this makes sense nodcathyper

dusky mirage
#

Though we still need to figure out the !Sync problem for sample resources, but that could be done in a later PR.

#

Ok, merged!

frozen iron
#

Hey all, what are the recommendations for using bevy_seedling (main) in web builds? Forgive me if this is exactly what you've been talking about up above.

I am targeting main by finding the latest bevy_seedling git commit and then copying its embedded firewheel rev, e.g.:

firewheel = { git = "https://github.com/BillyDM/Firewheel", rev = "f1e8219" }

bevy_seedling = { git = "https://github.com/CorvusPrudens/bevy_seedling.git", rev = "a47482ef69d8d4c7d85cba0e003ea36030af627f", default-features = false, features = [ ... ]

This used to build but now I get:

error[E0277]: `*mut u8` cannot be sent between threads safely
    --> /home/ejs/.cargo/git/checkouts/bevy_seedling-854d7bc153e83485/a47482e/src/platform/cpal.rs:123:17
     |
 123 |     mut stream: ResMut<CpalStream>,
     |                 ^^^^^^^^^^^^^^^^^^ `*mut u8` cannot be sent between threads safely
     |
     = help: within `platform::cpal::CpalStream`, the trait `Send` is not implemented for `*mut u8`
... (chain of reasons) ... 
 789 | /     impl_platform_host!(
 790 | |         WebAudio => WebAudioHost,
 791 | |         #[cfg(feature = "audioworklet")] AudioWorklet => AudioWorkletHost,
 792 | |         #[cfg(feature = "custom")] Custom => super::CustomHost
 793 | |     );
     | |_____- in this macro invocation
note: required because it appears within the type `firewheel::firewheel_cpal::cpal::Stream`

(etc)

(Same thing without the webaudio feature, fwiw.)

#

Oh, I'm silly. I tried building seedling examples the same way and it works. Huh.

slate scarab
#

Oh we might have a web build issue in that case blobthink

#

i believe @sly glen is working on a PR that should allow us to resolve this issue, coincidentally.

Looks like cpal modifies the properties of its stream depending on the platform.

frozen iron
#

I think I should have deleted my original message before I went off to dinner, because I found I could build (many) examples and run them with bevy run web. So I must have some threading thing on my end.

#

Though I see that spatial_basic_3d doesn't build anymore (it doesn't have the SeedlingPlugin->s rename... 😉

slate scarab
frozen iron
# slate scarab Hm, but does it work with the `web_audio` feature?

I did try building my project on my end with web_audio on and off -- same build result. (FWIW, I never did get audio to work on a thread under web in the first place... just wanted to see what recent changes had.)

OTOH, building bevy_seedling examples with -Fweb_audio doesn't build either -- for different reasons -- making me wonder if cargo is really pulling the right commits. (There, it complains that cpal has configured out audioworklet since atomics isn't in the target... wut?)

#

Let me keep digging...

slate scarab
#

which you'll get with bevy build web -U multi-threading

#

if you want it to be easy 🤭

frozen iron
frozen iron
#

🤷 I'm not sure what's going on -- certainly something on my end. I tried loading bevy_seedling with extra dependencies, and once I add my own repo as a dependency, the errors erupt and break the example builds too. So, ignore me for now!

frozen iron
#

I am still trying to figure this out. I was able to get my base repo and a very simple example to build for wasm32 again, but something™ I've added in my app (which uses that repo) is doing something else to make rustc angry.

(I meant to reply to myself -- no response needed still 🙂

clear wasp
dusky mirage
slate scarab
#

ohhh i can take a look today!!

dusky mirage
#

@slate scarab Thinking about the sampler node, do we even need a type-erased SamplerResource? I know you're not a fan of generics on nodes, but if the details of the sampler node are hidden from the user in bevy_seedling, maybe it wouldn't matter much?

#

Though on second thought, it would make sampler node pools have more complicated generics.

slate scarab
#

I think it would be a bit of a UX regression if you had to make pools ahead of time with the knowledge of whether they support streaming or in-memory etc

#

That's to say -- the type erasure significantly improves overall composability!

dusky mirage
#

Ok, I'll keep the type erasure then.

dusky mirage
#

@slate scarab Ok, I removed the sample parameter from SampleNode. Let me know what you think of the API!

I does hurt the ergonomics a bit, but I can live with it.

slate scarab
slate scarab
#

the user-facing API is completely unchanged :>

#

@frozen iron did you ask again about #87, by the way? I thought I saw that earlier.

I should be able to investigate soon again! Thank you for updating the repro.

#

Also, if you have the time, feel free to try building again with the latest master (bfd2f33). We landed a change that should remove the fragile platform-dependent Send-ness.

frozen iron
frozen iron
slate scarab
#

yay!!

#

Yes I did some testing yesterday and it seems like there may be a panic occuring once we actually use the worklet host. Though even with the audioworklet feature, cpal won't choose the worklet host by default. So on main it should just run on the main thread and work correctly.

#

If suboptimally.

limber kernel
#

@dusky mirage random q but wondering what your rationale was on including effect nodes in firewheel vs. seedling vs. another crate. The delineation feels a bit arbitrary and confusing to me currently but i might just not be getting something 😅

dusky mirage
frozen iron
# slate scarab Yes I did some testing yesterday and it seems like there may be a panic occuring...

Hmm, I am having little luck so far with the web build (after fixing other issues).

I can get audio running with seedling examples, but my own programs are just silent. Unfortunately it's sooo painful to experiment here since every minor change I make is another 5 minute build.

I notice this in both the seedling example and my own project's logs:

INFO /home/ejs/.cargo/git/checkouts/firewheel-238d422ce3aa55d4/f1e8219/crates/firewheel-cpal/src/lib.rs:328 Attempting to start CPAL audio stream...
sound_chain.js:1570 INFO /home/ejs/.cargo/git/checkouts/firewheel-238d422ce3aa55d4/f1e8219/crates/firewheel-cpal/src/lib.rs:515 Starting output audio stream with device "Some(DeviceId(WebAudio, "default"))" with configuration StreamConfig { channels: 2, sample_rate: 44100, buffer_size: Fixed(1024) }

I am wondering how the bevy_seedling examples build properly for web -- they seem to require atomics and such, and bevy run --example .... web just works there. But on my end, I also have to explicitly pass -U multi-threading. I don't see any .cargo/config.toml or any other uses of web-specific enablements in these repos.

I'm probably still far in the weeds due to having to make a lot of patch overrides and such in order to use dev versions of things. I might go back to native again where I belong :-p

slate scarab
#

In 0.7, we used our own crate for multi-threaded audio processing. On main, we now rely on cpal for this. In my in-progress work, it automatically switches the host to AudioWorklet, which is the multi-threading context. WebAudio is actuall just main-thread processing, and should work regardless of your nightly settings.

#

That is, it should not be silent.

#

they seem to require atomics and such
They don't for basic functionality! You can always use main-browser-thread audio processing, it's just a bad idea if you want to have reliable, glitch-free audio.

#

I'll let you know once I've sorted out the new multi-threading setup. Nonetheless, you shouldn't be having silent audio! Make sure you at least click on the window, though. The browser doesn't let the audio context start without an interaction.

frozen iron
frozen iron
slate scarab
#

oh yay!!

dusky mirage
slate scarab
#

i will test soon :3

dusky mirage
#

I'm not sure what I was thinking when I added that silence detection code. 😅

slate scarab
#

happens hehe 🤭

dusky mirage
#

I was also getting a weird echoing bug with the input, but it turns out it was just a bug with the EasyEffects software I'm using for system-wide EQ.

hasty marsh
#

folks, I'm the author of Unhaunter (game, http://www.unhaunter.com, FOSS) and I really really need a good audio engine, Rodio is setting me back a lot. A quick look around, it seems we're enabling people to use Firewheel via bevy_seedling.

If my understanding is correct, I can attempt to remove rodio and put bevy_seedling, then I could try to help by testing.

Question: With proper work, do we expect that basic audio features are currently production ready? meaning, that I could release a stable version of the game eventually (<6 months) with firewheel and at least handle the basics: playing samples once, looping, basic volume control. Without substantial bugs with that simple usage.

If that's the case I think I will be trying.

slate scarab
#

Bugs do occasionally crop up of course, but the core behavior has been solid for quite a while now.

winged cedar
#

is bevy_seedling coming out in 0.19?

slate scarab
#

If you mean "will it be a part of Bevy itself in 0.19," then no, not quite yet!

#

I'm somewhat optimistic about a 0.20 timeline though.

celest whale
slate scarab
#

So exciting!

#

@dusky mirage I've been itching to publish a new release for a little bit, but we're (in theory) quite close to rc for 0.19. I think we should probably wait until then to do the publishing rounds.

#

Well, unless you want to do it all twice, that is.

#

There's definitely some advantage to publishing now.

dusky mirage
#

Oh yeah, that being said, how do you feel about bumping the rust edition of Firewheel to 2024? There will probably be a lot of new clippy warnings to fix. 😁

winged cedar
slate scarab
slate scarab
#

Also it might be nice for Rustweek hehe 🤭

dusky mirage
celest whale
slate scarab
#

@dusky mirage Okay now I've totally reversed my opinion haha. I'll work today on getting bevy_seedling up to speed for a new release. I'll let you know if it's ready, and then I'd be happy to help unblock anything if necessary for a Firewheel release too!

compact sphinx
#

avian has adopted this practice - separating the bevy release version and the feature version

dusky mirage
slate scarab
#

Obviously that's very much up to your discretion. If you're not ready for a new release soon, that's okay!

#

But it would be really helpful for me if you happen to have the capacity for it sometime this week.

dusky mirage
#

Yeah, I should have the time soon. Not today though, family is over!

slate scarab
slate scarab
#

(The feature enables multi-threaded audio processing, which would allow us to remove my firewheel-web-audio crate.)

dusky mirage
slate scarab
#

yay!!

dusky mirage
#

Though if we can't use bevy_platform::time::Instant::now() with the audioworklet feature, then that means we also can't use any profiling features in that target as well (unless there is some way for an audioworklet to get delta time between two points in time?)

slate scarab
#

yeah that’s true — fundamentally we’re not allowed to get the time

#

in the worklet

#

In practice that should be fine — I believe there’s a way to get coarse usage from the web audio API. Fine-grained profiling can be left to the desktop.

slate scarab
#

Note that it's very specific, though -- it's only in the multi-threaded worklet that we can't get the time. You can get it on Wasm when it's running on the main browser thread.

#

Which, with cpal, you can switch between at runtime.

#

I could help make a tiny wrapper over bevy_platform::time::Instant::now() that returns an Option<Instant> if you'd like.

#

We can trivially detect on Wasm whether we'll be able to get the time or not at runtime.

dusky mirage
#

And the reason we aren't using the CPAL timestamps is that there is no real way to turn them into an Instant. CPAL timestamps only tell you how much time has passed since "some undefined point around when the stream was started", which is useless for synchronizing events with the main thread. (It can be used to accurately find the delta time between process calls and the amount of delay until the buffer is sent to the audio device).

#

(And even then the ALSA backend seems to have a bug where even delta time isn't working properly.)

slate scarab
#

well that's a bit tricky huh

#

Is it possibly worth maintaining a separate backend so we can maintain this capability?

#

I suppose that might be what we have to do until and unless cpal provides the capability we need.

#

The real nice part about cpal is that we can easily switch between multi-threaded and single-threaded at runtime, which is great in case multi-threaded is disallowed (often happens if the right HTML headers aren't supplied).

dusky mirage
#

Well we don't really need delta time for anything. The only thing I used it for is to detect output overflows, but even then I've worked around that using Instant.

Really, the CPAL API has some strange design decisions. The correct way to do this is to just have the OS tell you if there was a overflow or not (this is what RtAudio does).

slate scarab
#

but we do want timestamps that we can correlate with the main thread, right?

#

and cpal, at least currently, explicitly disallows this using only their APIs

dusky mirage
#

We only need a single Instant describing the start of the process block.

slate scarab
#

ya okay so in that case, I won't try to unify cpal with multi-threaded just yet

#

I'll keep firewheel-web-audio kicking for a bit

hasty marsh
#

sorry to make noise here but.... an initial test ino bevy_seedling and the behavior of VolumeNode is unexpected:

            commands.spawn((
                MenuSound::default(),
                SamplePlayer::new(asset_server.load("music/unhaunter_intro.ogg")).looping(),
                sample_effects![VolumeNode {
                    volume: Volume::Decibels(-64.0),
                    smooth_seconds: 0.5,
                    ..default()
                }],
                MusicPool,
            ));

I would expect this to start playing at -64dB (almost inaudible). What seems to happen is that it starts at 0dB and it's lerp'ed to -64dB in 0.5 seconds.

It is even more unexpected if you omit smooth_seconds, because I can hear a "clack" sound, which actually is the lerping doing the same in 15ms.

I would expect that the smoothing applies when changing the value after the entity is spawned, not for the moment we spawn it - because also, the selection of 0dB as starting point is subjective.

#

If you want me to open bugs, or totake conversation elsewhere, I'm happy to 😄

slate scarab
#

No worries! I believe this is the same issue as #87. That is, we may need a way of immediately setting values in certain circumstances.

#

To be clear, the VolumeNode is just a handle to some processor in the audio thread that's always there. When you spawn a new entity with parameters, those parameters are then applied to some existing node. If the processor happens to be at zero dB, and you assert a smoothing of 0.5s, then it'll have to lerp from wherever it is to the new value (-64dB).

#

The default value is your choice! It's whatever you provide to the pool entity, though MusicPool is spawned for you by default.

#

However, that default won't necessarily matter after the first time a player is assigned, since the value going forwards will remain wherever it happens to be.

hasty marsh
#

do the pools by default have a VolumeNode spawned?

#

bevy_seedling - src/pool/mod.rs:679

fn populate_pool(
    q: Query<
        (
            Entity,
            &SamplerConfig,
            Option<&PoolSize>,
            Option<&SampleEffects>,
            Option<&EffectId>,
        ),
        (
            With<PoolLabelContainer>,
            With<PoolMarker>,
            Without<PoolSamplers>,
        ),
    >,
    mut effects: Query<&EffectId>,
    default_pool_size: Res<DefaultPoolSize>,
    mut commands: Commands,
) -> Result {
    for (pool, config, size, pool_effects, effect_id) in &q {
        if effect_id.is_none() {
            commands.entity(pool).insert(VolumeNode::default());
        }

This is probably setting the pool with a VolumeNode default. So when we create the new one, we are changing it, so it triggers.

slate scarab
#

But otherwise it's entirely up to you!

#

Pools will only grow when they run out of capacity (and haven't reached their capacity limit). When they grow, the effects provided at pool definition are cloned into the new slots. Thus, those values serve as defaults.

hasty marsh
#

question: do you see the problem from the "user" perspective? (user meaning, the dev that's using these crates)

slate scarab
#

I'd like to think so! It's something that needs solving, though the best solution isn't totally clear to me.

hasty marsh
#

can confirm that using a custom pool with volume close to zero, does in fact solve my specific problem here:

#[derive(PoolLabel, Debug, Clone, PartialEq, Eq, Hash)]
pub(crate) struct BGAudioPool;

pub(crate) fn create_pools(mut commands: Commands) {
    commands.spawn((
        SamplerPool(BGAudioPool),
        sample_effects![VolumeNode {
            volume: bevy_seedling::prelude::Volume::Decibels(-120.0),
            ..Default::default()
        }],
    ));
}

However I don't think this is the desired behavior.

#

And we will see what happens for sound effects, if they would also lerp either from gain 0.0 or from gain 1.0.

slate scarab
#

We'll need to figure out something a bit more robust in bevy_seedling itself blobthink

hasty marsh
#

yes, it works for that particular problem because that plugin in my game expects to start sounds muted and fade them in. But it would give problems for sound effects in other plugins. I'm following perfectly 😉

#

also it sounds like if I don't provide any VolumeNode in the Pool then I do not have default.... nope hah. I tested. The pool must have the effects we need. So I'm forced to a default initial value.

slate scarab
hasty marsh
#

yeah, same

#

I wish I could explain better but I'm ramping up on this just today

slate scarab
#

Say you have a pool with one slot. The default value will be applied once, on startup. If you play a sound in it, and the last volume it was at is 0dB, then the next sound queued in that pool will start at 0dB. This "default" specified by the pool won't matter -- it will still have to sweep from 0dB to -60dB if you want the next sound to start silent.

#

That's why I say it's fragile. It will start failing as you play more sounds.

hasty marsh
#

wait a second, there's only ONE VolumeNode for the WHOLE pool?

#

I thought I was spawning a VolumeNode per sound

slate scarab
#

No, that's why I specified "a pool with one slot."

#

Each slot in the pool has its own set of effects.

hasty marsh
# 
pub struct DefaultPoolSize(pub RangeInclusive<usize>);

impl Default for DefaultPoolSize {
    fn default() -> Self {
        Self(4..=32)
    }
}

so there is a size, and then the sounds pick a slot there?

slate scarab
#

Right, spawning a sound means that it looks for a slot in the pool!

hasty marsh
#

ok, I see now why you call it a Pool, it's like a ThreadPool... you have "workers" there. Told you I'm ramping up as we speak 😛

slate scarab
#

yes!!

#

Right so that's why we have to figure out how to cleanly transition from one value to the next whne a new sound is spawned.

#

Often, this doesn't matter. But if you have a large smoothing time, it's problematic.

hasty marsh
#

ok, so - can a slot of the pool play more than 1 sound at a time?

slate scarab
#

No, they're limited to just one at a time

hasty marsh
#

ok, in a ThreadPool, if one work unit does "i=5", you wouldn't expect that the next worker would have "i==5" - in any programming language. Even if you reuse the same threads, the context starts new.

I think that in here, when a sound starts playing, it needs to be "reset" to what that sound wants. It doesn't make sense to smooth from whatever the other sound used. So if I have a smoothing of 0.1 second, and I am playing random sounds at different volume levels... it will come out all randomly and weird

#

Maybe this is obvious to you already

slate scarab
#

Yes I'm not suggesting that this is ideal!

#

The other problem is that it's not necessarily so trivial.

#

You noticed the click when you set the volume smoothing to zero, didn't you?

#

We have to account for that sort of thing. Not all parameter can simply flip instantaneously.

hasty marsh
#

no, but I know what you are talking about, I know a bit about DSP

slate scarab
#

In principle we should be able to get away with very little or even no volume smoothing when switching samples. That is, if the input is silent, then there shouldn't be any clicking.

hasty marsh
#

yeah, but think: A pool slot can only play 1 sound at a time. If we start to play a new sound, by definition, it wasn't playing any sound before (or was going to fully stop). If it stops, that means Output = 0.0 (center). If we start straight away at full volume, if the entering sample sound has a high sample - that's intended and should be played.

slate scarab
#

However, that depends on your effects! If there's any energy left over from a previous voice, such as from a reverb, an instant volume change would click.

hasty marsh
#

wait, I'm trying to follow

#

you're assuming Reverb -> VolumeNode

slate scarab
#

Yes, which is something you can do!

hasty marsh
#

and I was assuming Input -> VolumeNode -> [Effects ] -> Output

slate scarab
#

The VolumeNode is just an effect like any other

hasty marsh
#

of course, sure

slate scarab
#

That is, you can arrange them however you like

#

Of course, we can consider this an acceptable compromise.

#

Just don't place volume nodes in a position where they'll cause clicking.

hasty marsh
#

I mean... for this particular problem, even a bool for configuring this is acceptable; reset_on_new_sound: true - or something like that.

But overall I think we're going to hit a ton of problems with this architecture

#

like, I might want to fire a ton of samples with different reverb settings... and I might want to change dynamically the reverb... it's going to go mad

slate scarab
#

is it?

hasty marsh
#

in the sense that developers are not going to expect this behavior

slate scarab
#

The behavior that I'm suggesting we change, or other parts of it?

hasty marsh
#

I'm referring on how the sample effect work, on the slots of the pool.... I still can't formulate this, I'm still absorbing info

#

and my game is a bit special what I want to do, and trying to figure out how everything maps

slate scarab
#

A big reason for pooling like this is that it provides excellent performance characteristics. It scales very well! Note we have talked about permitting more dynamic management as well.

hasty marsh
#

could we dedicate the slot to 1 sound, including the tail from any effects? if any tail, any smoothing, is dedicated within the sound+slot, then it will do what people expect. Otherwise... I kind of expect a bit of chaos from people trying to understand how to use this

slate scarab
#

If the implementation details matter, then we have work to do!

hasty marsh
#

let me give you a practical example

slate scarab
#

(I'll have to return in a bit.)

hasty marsh
#

This is my game Unhaunter. Imagine there are sounds when a door is opened or closed, or weird sounds coming from different places of the house.

Now, we compute the position of the sound and the traversal needed to reach the player/camera/mic. Depending on this:

  • We set the Volume for the sound (with distance, you hear it less)
  • We set the Panning for the sound (Depending of if it is at the left or right...)
  • We set the Reverb for the sound (At the distance, we hear more reverb, more wet, less dry) (Also room size, we can estimate current room size you're in, and set it for the reverb)
  • We set a lowpass for the sound (Depending on the walls and bounces needed to reach your ears)

Imagine this working with bevy_seedling on a naive implementation - each sound would overlap settings of the other. And sound effects are short (2 seconds) but reverb tails can be >5.0s. This could cause a nightmare to a lot of people to understand why it doesn't do what I tell it to.

So... the expectation is - if I spawn the sound with effects that I can configure per-entity, I expect them to be isolated from other entites that played before. In practice... you're saying they're not.

image.png
#

(note that I'm not saying that I can't make it work, I could make it work - but people coming in will get surprised by the behavior )

slate scarab
#

While I gave it as an example earlier, reverb is not typically calculated per-sound. If you did want this, there are many solutions! These would naturally be hand-managed, because reverb tails are not exactly trivially analyzable.

#

That's to say, I'm not sure I agree that it's particularly problematic. We could absolutely offer a special mode or player for sounds + effects that truly must be isolated! But so far, the primary issue I've seen others run into with this architecture is the smoothing problem.

hasty marsh
#

agreed, and I need to learn more

slate scarab
#

Panning, volume, and lowpass won't pose problems -- for example, in the worst case we could simply have a "clear state" trigger that cases them to clear themselves as if they were newly created!

#

In that way, it should behave identically to newly created, dedicated voices for sounds.

hasty marsh
#

if there are any effects that can add tail, if we can reserve the expected tail time it would probably solve most of the issues too

slate scarab
#

We could certainly do that! We already have a somewhat clever silence detection scheme for the graph.

hasty marsh
#

as for me I'd be happy with a forced fadeout at +X time and that's it. Even if I have to specify it and calculate it myself, still would be happy. Just saying, in case it's easier than silence detection. Because some effects could have feedback that could ring infinitely - not sure what effects do you have, and if people can add effects externally.... but certainly effects could ring.

slate scarab
#

Yep, you can easily add your own!

hasty marsh
#

that's really cool! 😄

#

it's a bit late here, will have to go

hasty marsh
limber kernel
hasty marsh
#

@limber kernel oh yeah it is - and I've been cooking on closed doors for almost a year. Multiplayer will be included on the next release and uses room codes, players do not need to host servers or open ports! (Also it's a FOSS game)

dusky mirage
hasty marsh
#

always happens

dusky mirage
#

Do you have a test project I could use to debug this?

hasty marsh
#

not really, sorry - I was testing with my game Unhaunter, my migration to firewheel... and the stuff to reproduce this wasn't even comitted entirely because it is so broken

#

would need to create a sample project or something but my feeling is that should be trivial to reproduce

dusky mirage
hasty marsh
#

I believe you don't even need spatial audio to reproduce, all you need is to try to reproduce a sample that begins loud, request it at a very low volume, with some smoothing and voila

dusky mirage
#

Ok, I'll try that first.

dusky mirage
#

Doing this I also realized it would be much more helpful if NodeID output the type of node when debugged. That probably won't be hard to do, just need to store a static str.

slate scarab
#

I guess there isn't a public API for that

dusky mirage
#

Ok, turns out the diffing logic in the pools was wrong.

But while I'm at it, I think the API of node pools can be improved now that we have the ability to bypass nodes.

slate scarab
#

Not yet published

#

As-is, it might not have sugar for pools, I'll have to double check.

#

Oh looks like I need to update the scheduling logic on bypassing as-is.

#

(We have a resource for opting into scheduling, and it's off-by-default. Looks like the bypassing doesn't check it currently.)

dusky mirage
#

Actually, if you're ok with FX chains only being serial chains of nodes, then we might not even need the FxChain trait at all. It could be possible to just let the user pass in a list of arbitrary nodes as the fx chain.

slate scarab
#

oh actually bevy_seedling rolls its own pool managment 😅

#

sorry if i've never communicated that

dusky mirage
#

Ah, ok.

slate scarab
#

I can investigate this today too! I might be able to spot some expectation my pools are breaking

dusky mirage
#

Well it turns out VolumePanChain::set_params and SpatialBasicChain::set_params is implemented incorrectly.

#

I'll just push a fix for that real quick.

slate scarab
#

I didn't think about silence-detection interacting with immediate parameter setting by the way. That would be very convenient, and indeed I'd expect that to be working as-is!

#

Though I wonder if that's enough. A pool that's reached capacity has to kick currently-playing samples out.

#

In which case I think there won't be the opportunity for a block of silence

dusky mirage
dusky mirage
slate scarab
#

@dusky mirage, as an aside, one thing we could do is basically auto-bypass nodes that aren't mentioned on spawn. For example:

commands.spawn((
    SamplerPool(MyPool),
    // this pool has a chain of two effects
    sample_effects![
        SpatialBasicNode::default(),
        FastLowPassNode::default(),
    ],
));

// spatial and low-pass nodes are bypassed
commands.spawn((
    MyPool,
    SamplePlayer::new(server.load("my_sound.wav")),
));

// spatial is bypassed
commands.spawn((
    MyPool,
    SamplePlayer::new(server.load("my_sound.wav")),
    sample_effects![
        FastLowPassNode::default(),
    ],
));

// neither are bypassed
commands.spawn((
    MyPool,
    SamplePlayer::new(server.load("my_sound.wav")),
    sample_effects![
        SpatialBasicNode::default(),
        FastLowPassNode::default(),
    ],
));

However, this has the slight issue of being very verbose. It would be a bit annoying to provide parameters for spatial sounds every time you spawn them!

dusky mirage
#

Hmm, I personally don't mind it. It's more explicit.

slate scarab
#

We could get fancy with it too, though it may not appeal to you 🤭

For example, every time you play a sound in a pool with an effect that it doesn't have, well.... we could add it. In combination with automatic bypassing, this would allow people to freely add effects that, in practice, would only rarely cause recompilation.

I've heard several complaints that having to specify all the effects up-front doesn't feel good.

dusky mirage
slate scarab
#

ya true

dusky mirage
slate scarab
#

It's essentially opt-in flexibility. If you don't want this recompilation, just create the pool with all the effects you want.

dusky mirage
#

It definitely feels more "bevy-y".

slate scarab
#

I'll think about this blobthink
it might be a good rustweek task

wary bridge
#

completely unrelated but curious if you've messed around with bsn for seedling yet

dusky mirage
#

Ok, I won't go crazy with changing the API of pools then.

There are two small things I thought of though:

  1. We could add a Configuration type to the FxChain trait, similar to the Configuration type in the AudioNode trait. That way you wouldn't need to store node configurations in the struct itself.
  2. It might make sense to add first_node: impl FnOnce(&mut ContextQueue) and fx_chain: impl FnOnce(&mut FxChainState<FX>, &mut FirewheelContext) arguments to AudioNodePool::sync_worker_params(), like there is in AudioNodePool::new_worker().
slate scarab
#

You can certainly do smaller-scale stuff now in BSN, like spawning sample players! That should work great

#

I suppose I should ensure that it'll all work smoothly

wary bridge
#

I see no reason why we couldn't also support a world.spawn_scene_list (with an accompanying ScenePatchList asset type) if you really want this to be flat.

this bit does exist in bsn currently

dusky mirage
slate scarab
#

I haven't kept up on naming in BSN

wary bridge
#

I'm not exactly sure about the entity name reference part of it, but the rest of it yes

#

worth trying out tho

slate scarab
#

Connections would just be a collection of references (or Entity, however it ends up resolved), and I'm sure there's some way to express giving them a list in BSN.

wary bridge
#

if only graphs were easier to notate with text

slate scarab
#

yaaa

#

no worries we'll make an editor for it soon enough :>

wary bridge
#

audio editor, call it the Auditor

dusky mirage
slate scarab
dusky mirage
#

nice!

slate scarab
#

We could probably go up to 7.1 even, though at some point it really gets into more bespoke territory imo

frozen iron
# slate scarab <@714940838781911051>, as an aside, one thing we could do is basically auto-bypa...

It would be a bit annoying to provide parameters for spatial sounds every time you spawn them!

Yes, and it's also annoying to specify the effects when they're unchanged too. 🙂

For example, every time you play a sound in a pool with an effect that it doesn't have, well.... we could add it. In combination with automatic bypassing, this would allow people to freely add effects that, in practice, would only rarely cause recompilation.

Yes, please!

I think this particular example would go the wrong direction in terms of usability. It would be / is surprising to have to redundantly mention the effects that were already set up in the pool. That would mean, every time I made a slight change to a pool, I'd have to edit every call site where I spawn a sample and copy the effects in. Yuck!

Like other commenters have said, the SamplerPool API seems like it's providing a slightly different metaphor than it seems to be, and maybe that's where the repeated confusion comes from. The obvious benefit of a pool is limiting the maximum number of simultaneously-playing sounds. That makes sense and works as expected.

But I think when new users (myself included, when I was one) see the example code spawning a SamplerPool and effects, we assume that those effects would be provided by the pool automatically when I associate a new SamplePlayer with that pool.

I.e. in this imagined workflow, in any place in my code I should be able to spawn (MyPool, SamplePlayer), and have those reverb/volume/etc. effects from MyPool applied wholesale to that new SamplePlayer . I only need to set up those details once.

Think of specifying the effects in a pool as being similar to required components. Required components do not limit freedom: I would expect that if I spawned (MyPool, SamplePlayer) but wanted a different VolumeNode or SpatialBasicNode, I would just add them along with the SamplePlayer and it would override the "defaults" from the pool.

Similarly, I should be able to add additional effects that might be specific to a certain use case, without needing to jump through hoops (i.e. making a new pool, adding the effects to the original pool and fixing up all the other call sites, etc). As you've mentioned, this is like the DynamicBus metaphor, but that has less flexibility with limiting the total number of sounds.

The quoted example is saying "if you don't mention an effect component, it's bypassed." But I assert the opposite feels much more "right": if I don't want an effect, I should be able to explicitly add some other component/effect that does this (i.e. using whatever future bypass/disablement state you guys decide on). I don't want to touch every call site whenever I change the pool. (I.e. at some point I want a Doppler shift node which automatically applies to any samplers assigned to my "Sfx" pool.)

Like others here have said, architecting the Bevy-side API around the optimization-friendly requirements -- i.e. requiring an alignment of effects between the SamplerPool and any SamplePlayer -- is not intuitive and might even be necessary for peak performance in most use cases. There is (IMO) ample processing power to rebuild some node graphs a few times a second as needed.

I would expect optimizations to be addressed behind the scenes (say, treating the SamplerPool/effects components as metadata that Seedling uses to maintain a set of common effect chains corresponding to the actual runtime behavior of the code). If and when users get to the scale where they need to care, they can opt in to a more complex but optimized spawning pattern.

I hope I expressed my opinions clearly and am representing other users accurately here!

hasty marsh
#

I feel that I am crashing here where I do not belong because I kind of recently started to follow this work closely, but I do have quite opposite opinions

#

Not specifying an effect in the SamplePlayer, but having it in the SamplerPool, it is surprising that the effect is applied. Without spending a few hours reading on this, one would expect that sample_effects at the Pool level are bus-like, and are applied after. Which I know it is not the case.

And then, the Pool effects being "defaults" is a kind of a lie because you could add this at the pool level:

sample_effects![VolumeNode { smooth_seconds: 1.0, ..default() }]

And then at the SamplePlayer level, if someone does:

sample_effects![VolumeNode { volume: Volume::Decibels(-10.0), ..default() }]

Guess what's not conserved as the default? the smooth_seconds, because Rust forces us to rewrite it.

So only uses them as defaults when I forget them.

I could see devs that want a LowPass for a particular sample:

  1. They try to add lowpass to a sampleplayer,
  2. Fails to run because the pool doesn't have lowpass - then it adds it to the pool
  3. It runs, now suddenly all sounds in the game weirdly sounds muffled because the defaults for that effect might contain a cut at 1kHz for example.
frozen iron
# hasty marsh Not specifying an effect in the SamplePlayer, but having it in the SamplerPool, ...

I feel that I am crashing here where I do not belong

Oh, don't worry, I'm the same kind of drop-in user. But I think we do belong here, unless told otherwise 😉 If Seedling + Firewheel are upstreamed into Bevy, all the consumers of a "better" audio API need to be happy.

but I do have quite opposite opinions

Maybe in some regards -- but I think we are in agreement about the desired workflow in general, where the pool provides the effects for subsequent sample players in that pool. I did lazily use the term "defaults" above when I meant more to say "template" when referring to the way the effects provided by the SamplerPool "should be" applied to samples. IIUC this is what you also mean by "bus-like"...?

I do see the point about the use of ..default() not quite acting as an incremental override. That's an interesting observation. I wonder if these engine-level defaults with well-defined constants should be Option<...> in the node, where Seedling fills these in unless explicitly provided? That might make them slightly less surprising.

hasty marsh
#

bus-like means that you put the effect on the pool itself, and it doesn't go to the workers. If you have a pool of 30 workers, one think is to put a Echo effect on each worker, and another is to get one (1) Echo effect after downmixing everything into one stereo pair. Not sure what's the current way to do this in bevy_seedling, but imagine: EchoeyEffectpool -> [Echo effect] -> SamplePool; you could put the effect just once after all samples have been downmixed.

slate scarab
# 
commands
    .spawn(SamplerPool(MyPool))
    .chain_node(ExpensiveEffectNode::default());

:>

hasty marsh
#

exactly, thanks @slate scarab ! - other people is initially going to feel that this would be what this code does:

commands.spawn((
    SamplerPool(MyPool),
    // this pool has a chain of two effects
    sample_effects![
        SpatialBasicNode::default(),
        FastLowPassNode::default(),
    ],
));
#

And then imagine they learn that no, these are defaults per sample. Great. Then they do:

commands.spawn((
    SamplerPool(MyPool),
    // this pool has a chain of two effects
    sample_effects![
        ReverbNode::default(),
    ],
));

And suddenly all samples played through the pool now have reverb, even though it is not declared. They could have needed to add it because they needed it for 1 sample.

slate scarab
#

I think the docs do a good job of clarifying the behavior here! There will always be some level of implicit behavior -- after all, the ECS is doing quite a bit in the background to synchronize with the audio thread.

#

It's a touch tricky to balance the concerns of performance/reliable audio and pure ease-of-use. To some extent, they're in conflict.

#

Some people prefer explicitness, others implicitness.

#

I'll have to think about this a bit blobthink

#

Certainly, a number of people would prefer declaring effects on-the-fly.

hasty marsh
#

think how does this work when coming from Rodio - because seems we're aiming to replace it, so if it finally replaces, we will get a ton of people attempting to use this with basically zero knowledge, just a skim look into the migration docs

slate scarab
#

If they're coming from rodio, the migration should be quite easy since its capabilities are so limited 🤭
(At least, as exposed through bevy_audio to be fair.)

hasty marsh
#

by the way, how is WASM support? because Unhaunter targets WASM+Linux+Windows, so it is a must here.... I heard... nightly compiler? and some weird headers that will give me trouble with the current GitHub pages deployment?

slate scarab
#

Depends on what you want.

#

If you don't care about multi-threading, then there are no particular requirements.

#

However, that means you're processing audio on the main browser thread, which is a recipe for clicks and stuttering!

#

If you want to process audio on a dedicated thread, that does require nightly.

#

I'd recommend giving the Bevy CLI a try if you haven't! It makes this process very easy.

hasty marsh
#

you're telling me that it can be easily enabled / configured into "dumb single thread" or "multi thread"? that sounds amazing to me

slate scarab
#

You do have to mess with the plugin a bit in 0.7, but as soon as the next version is published (hopefully soon, within a few days), it should "just work" when you enable seedling's web_audio feature.

hasty marsh
#

is the default the old boring single main thread for WASM?

slate scarab
#

Yeah, similar to bevy_audio today

hasty marsh
#

purrrfect

slate scarab
hasty marsh
#

that's excellent, and let me tell you why: It holds my migration to bevy_seedling, from committing to it. Knowing that I could get bevy_seedling + Firewheel running, and even released without having to deal with the multithread problems, that I can defer that for later and it opens the door for it... that's exactly, precisely what I need

slate scarab
#

Oh nice!

hasty marsh
#

oh wait, and itch.io does support the damn header that I would be missing in GitHub pages? huhuh... that would make an excuse to do itch.io releases

slate scarab
#

It does, yes

#

It's a little checkbox in the game settings page!

#

Looks like pages doesn't support custom headers

hasty marsh
#

okay - to be honest I'm worried a bit with all the story on the Pool workers architecture, the smoothing problem overall. It's highly likely that I will stress test the architecture before committing to it

slate scarab
# slate scarab Certainly, a number of people would prefer declaring effects on-the-fly.

Another thing we could consider is allowing everything to be done on-the-fly. Right now, dynamic pools exist, but you can't route individual sounds to any particular place.

We could use essentially the same concept to allow you to spawn sounds with bespoke effects and to route them anywhere. We could cache these workers and reuse them if a new job comes in with the same shape, which we expect to happen very frequently.

hasty marsh
#

what if instead of sample_effects![], we make the user create a custom Struct with the effects they want, then the presence of the Struct tells you what to cache. And the Struct has its own defaults, like, it's very clear.

#

although I' more worried on the smoothing problem

#

@slate scarab @dusky mirage - any concern if I try to run bevy_seedling + Firewheel directly from git-master branch? (not for release or production, just internal testing)

slate scarab
#

You would lose expressiveness that we've built into the API thus far

hasty marsh
#

oh because they're components?

slate scarab
#

Yeah, and you can do all sorts of stuff with them as individual entities (sample_effects! describes a set of related entities)

#

For example, bypassing them (AudioBypass on master), adding configurations, etc.

#

The upcomping BSN may also inform some of our API decisions here. sample_effects! are in a sense rather scene-like.

#

BSN patches are a very natural API for them -- that's essentially what you noted earlier. You can't "patch" the values within sample_effects, since you have to replace an entire node when specifying it.

#

But, at least as far as I understand, this would be easy to achieve with a scene.

frozen iron
dusky mirage
hasty marsh
#

yeah, I tried and... bevy_seedling gets error when updated to the main branch

#

was just trying to see if I can follow the development more closely

dusky mirage
#

Oh yeah, I should add an option to add/remove nodes in the sampler pool API. I'll get to that soon.

#

Yeah, Firewheel moves pretty fast. There are just a lot of small things that keep cropping up that require tweaks to the API. 😁

#

But we are getting really close to settling on a final API.

frozen iron
#

I apologize for belaboring under incorrect assumptions in my last night's confusing message. I just RTFM 😅 : and realized Seedling already claims to do what I want and was arguing toward. Great!

So, what set me off esp. rudely meddling in a semi-private thread? If I may explain, it was this line from the message I quoted:

one thing we could do is basically auto-bypass nodes that aren't mentioned on spawn

But I admit I might not know what "bypass" means in this context. Bypass where and how? I should have just asked that.

If you take "bypass" to mean what it means,"going past", removing from a chain, it sounds like "effectively remove the effect from this <something>"? What's "something"? I assumed it was the effects chain in use for the selfsame SamplePlayer. (If so, given the proposed code and comments, and again assuming "bypass" means "remove", it looked like the effect declaration pattern would behave in a subtractive ... uh... "repeat the same components declared in the pool, or lose them (if "bypass" means remove!)" manner now?)

Where am I going wrong?

hasty marsh
#

sorry if I explain something you already know. Bypass in Audio means to disable an effect without removing the effect. Imagine a "Wet" knob where 100% is enabling the effect at 100% and 0% is forwarding the source input to the output verbatim. This is what we mean with "bypass" to simply route the input to the output.

frozen iron
#

OK, it's exactly what I thought then, thanks!

slate scarab
frozen iron
slate scarab
#

Oh I should note we're finally putting some more effort into resolving that again! So we should make some great headway in the next couple days.

frozen iron
#

Yes, I saw 🙂

#

You know what. I'm not sure I mentioned this, but there's an important aspect to the way I (was) using Seedling. I was using SpatialScale(Vec3::splat(10.0)) (though probably I shouldn't). That accentuates the issue in #87. If I use the expected SpatialScale of Vec3::ONE, the effect isn't noticeable.

I found a good workaround. I just spawn the pool with PoolSize(0 ..= N). I.e. ensure it's empty and doesn't preallocate anything with any assumption of the default spatial scale. Starting from an empty pool, the first sound is spawned and Seedling appears to sample the DefaultSpatialScale resource or SpatialScale component as expected.

hasty marsh
hasty marsh
#

I added ItdNode (Interaural time difference node), plus some frequency cutoff. I also had reverbs precomputed from before... omg it sells it so well

hasty marsh
#

@slate scarab I'm scared. I'm very very tempted of merging bevy_seedling into my main dev branch. Very awesome. But I'm not sure if I will regret this later.

#

I'm not particularly afraid of API migrations, after all I've been migrating since Bevy 0.12 - but I'm not sure what I'm signing into right now

#

the smoothing bug, I worked around it

slate scarab
hasty marsh
#

I decided yesterday and merged the feature towards my dev and alpha branch. It is set to be in the next release

#

a friend of mine tested it yesterday on PowerPC, and worked perfectly, zero issues - and he got very impressed

toxic ingot
#

Is there an in-built way in firewheel for instances of nodes of the same type to communicate? I'm taking a crack at a network node and I need to deliver network package to a specific node on a machine among potentially many

slate scarab
#

Though note the potential execution order!

toxic ingot
#

You mean the execution order of instances of the same node type may be anything?

slate scarab
#

It depends on their order in the graph

toxic ingot
#

ah

#

Awesome, thanks. I'll take a look

slate scarab
#

it's a DAG, so the order will follow the graph's flow

#

(Though you can have "coordinator" nodes that have zero inputs and outputs. These are always processed first.)

toxic ingot
#

And this proc store is unique to the type?

slate scarab
#

No, it's analogous to storing resources in a Bevy world

toxic ingot
#

Ah, it stores arbitrary data with type as the key

slate scarab
#

right

toxic ingot
#

gotcha

slate scarab
#

So you could create a queue within some type stored in there

#

We may want to provide access to the ID in ProcInfo, though. Currently I'm not sure you can get at that information within the node itself.

#

Oh, right you get access to it at construction.

toxic ingot
#

I guess I could also identify it myself and store that as part of processor state

#

Is NodeID stable? Can I control it or rely on it to stay between graph recompiles?

slate scarab
#

ya

dusky mirage
#

The one thing I am a little iffy about in regards to esoteric platforms is that Firewheel uses 64 bit atomics in some places, which isn't supported on all platforms. We do use portable-atomic, but that does mean we lose out on the realtime safety on platforms that don't support it natively.

Though now that I think about it, it would be pretty simple to create a small wrapper that uses a triple buffer instead on platforms that don't support 64 bit atomics.

hasty marsh
#

In my experience with this friend, PowerPC not only supports a lot of stuff... it seems to support plenty of things that x86_64 does not. So instead of if PowerPC supports X, I would worry if the compiler+libs support PowerPC properly...

toxic ingot
#

@slate scarab I want to pass configuration data into the node that is required, where Default doesn't make sense. In the case of the Network node, for something like steam networking messages, it requires that the steam client API is initialized so the handle can be passed into the node. Should I just panic in the case someone tries to pass in no configuration? Should there be a way for nodes to specify that configuration is required (and thus the configuration type wouldn't have to implement Default), or is there some other solution I'm missing?

oak walrus
toxic ingot
# oak walrus Are you looking to use the Steam Networking API inside of a Firewheel node? I wo...

Yeah true. I figured I'd need to separate out the node and the network I/O and communicate via a channel or something eventually, but wanted to get something working. Do you think it would make sense to keep track of if there are any networking nodes and spin up a thread to handle network I/O coming through channels from the nodes if there are > 0 of them? If that's the case, and that thread is initialized from the existence of at least one networking node, that networking node still needs to have the handle to the Steam Networking API and the default config is still a problem? Not sure

oak walrus
#

That's how I/O is generally handled in audio engines, yes, use a separate thread and a ring buffer (not a std channel, as those can reallocate and/or go through mutexes) to push data in and out. So the question still stands, but it is to make the nodes aware of the ring buffer instead

toxic ingot
#

How would that thread be started? Does it make sense to start and stop it with the existence of instances of the audio nodes or should it be outside that? If it's the former then the audio node will still need to be configured with the steam API handle, even if just to start the thread if it's the first network node, right?

oak walrus
#

I don't know the specifics of the Steam API but you should be fine starting the thread regardless of the presence of any node in the audio graph, at least at first, threads aren't too much overhead anyway and it'll probably spend its time yielding back to other processes because it has nothing to do

slate scarab
#

We should also probably relax the bounds on the configuration blobthink
FromWorld could be very useful.

toxic ingot
#

Oh. I can just make the configuration take an option to the handle with None as default and fail node creation if it’s none lol

#

I had it in my head that it had to be the straight value in the config and the default would get in my way

toxic ingot
#

Is it crazy to start a thread per node to handle network IO?

dusky mirage
#

@slate scarab I added a method to make it easier to add/remove nodes from an audio node pool! https://github.com/BillyDM/Firewheel/commit/806496cae6bec7a84c6bd38c637d6426a9927423

While creating it, I also figured it would be handy to have a "try_modify_graph" method that reverts all changes if an error occurs.

I also realized I forgot to actually add the logic to drop nodes when their respective node processors were successfully deactivated.

dusky mirage
dusky mirage
toxic ingot
#

Hmm, thinking in the context of games, biggest might be something like 100 players all using voice chat, so 200 nodes/threads?

dusky mirage
#

Ok yeah, that would be an awful lot of threads.

#

I would create a single thread and have all nodes connect to that.

toxic ingot
#

hmm yeah ok

dusky mirage
#

An official "voice chat" plugin is something we should definitely add at some point. Though I personally don't know enough about network programming to do it myself.

toxic ingot
#

absolutely

#

I'm doing this mostly so that we can get closer to that. Voice chat is one of those features that I feel like should be a solved issue in game dev but there aren't great options (even in other engines) that aren't cloud-based.

#

Bevy having easy-to-integrate p2p voice chat would be amazing

dusky mirage
#

(PowerPC users rejoice!)

dusky mirage
dusky mirage
#

@slate scarab Alright, now I think Firewheel is in a ready enough state! I might be able to help with the upstreaming process tomorrow. (Depends on whether or not my nephew decides to come over tomorrow.)

slate scarab
#

I think we're probably at a good point to publish soon -- ideally we'll publish a new version before Bevy's next version, and then another after/during the rc period! That can help give some folks a smoother migration process.

I'm reworking firewheel-web-audio now and should be able to get that going tonight, which is the main thing I'd like to resolve before another version. I'd love to PR in the optional Instant fetching for profiling too!

#

RustWeek might be a great time for us to nail down some things we want as well, so the week after might be a good time to get back on upstreaming ^v^

dusky mirage
#

And it's possible that the fix to FX chains I committed recently fixes the issue.

slate scarab
#

I resolved this by disabling scheduling by default in my in-progress work. Ideally it would work anyway, but it seems like the order of messages ends up with some arriving after the sample starts playing.

#

So yeah that should be good -- I'll be merging these changes in so people can test!

slate scarab
pine pelican
#

I don't know if this is selfish but before upstreaming, should "critical" / "regression" bugs be fixed? This may be subjective (and selfish), but I'd like to see the seamless bug fixed, plus the "[sound sometimes plays quietly](#1378170094206718065 message)" bug (can't remember if it was deeemed to be the same issue as this). I guess my thinking is that both of these are regressions on current bevy_audio and even though they could be fixed after upstreaming, fixing first might be a consideration?

slate scarab
#

Yep, definitely!

hasty marsh
dusky mirage
slate scarab
dusky mirage
#

Hmm, what exactly do you mean by "seamless playback"? Firewheel should be capable of sample-accurate playback events.

slate scarab
#

Not totally sure about the root cause, but I'm looking to address it today as well.

#

Certainly in principle it should be easy to get seamless playback!

slate scarab
#

Works great in my testing!

slate scarab
dusky mirage
dusky mirage
slate scarab
#

yay!

dusky mirage
#

@slate scarab Actually, do you have any need for SamplerState::clear_finished()? It would be simpler if we just had SamplerState::finished().

slate scarab
#

I don't think it matters for me at least. If the play field is mutated in such a way that playback should start (and complete) again, we won't be looking for the old ID anyway.

dusky mirage
#

Yeah, that's what I was thinking. I'll remove it so the logic is easier on my end.

dusky mirage
slate scarab
#

Hm, I'm actually still observing zeros! Let me dig into it a bit here as well.

slate scarab
#

Oh, because SamplerState has an Option<...> at the top level, meaning that it can become desynchronized if you clone it out once after a node is added. Since node construction is deferred until compilation (if I'm not mistaken), the inner value remains None until that point.

dusky mirage
#

Oh yeah, that makes sense. Should be simple to fix, I'll do that tomorrow!

slate scarab
#

Oh do you think it's possible?

#

I think it's set up this way now because of the channels that are created in the construct_processor call

#

right?

dusky mirage
#

Yes, I just got to create the channel when the node is added to the graph. (I just store the processor part in a temporary field wrapped in an Option).

slate scarab
#

It would definitely be super convenient for me if it could be done! It's helpful to be able to clone the state around just after the node is added.

dusky mirage
#

I should probably do that for the triple buffer node too.

dusky mirage
dusky mirage
#

(Essentially, set the initial node capacity really low in the Firewheel config and see if node profiling still works when adding a bunch of new nodes.)

toxic ingot
#

Is there a resampler in firewheel I can use just for my own node? Opus only accepts certain sample rates

dusky mirage