Implement custom sounds #341

nc7s · 2025-03-09T16:34:27Z

Resolves #337.

Each commit is self contained (hopefully) and comes with explanation in its message (mostly). But to sum it up, notable changes are

A new [sound.collection] config section to specify custom sounds, which can be used as play-sound(name=custom-name); section name could be adjusted
A default-sounds feature to allow removing, well, default sounds
play-sound(path=...) for "one-off" settings

Canop · 2025-03-12T20:22:07Z

src/sound/sound_config.rs

+            for (name, path) in collection {
+                // TODO: Do we really want to panic here? Or should it be
+                // silenced?
+                crate::sound::play_sound::add_sound(name, path).unwrap();


Why would we want to have a panic ? That's not how bacon should behave

Indeed, hence the TODO.

Do we silence it, or warn about it? If warn, where should the warning go?

Ahem. Do you think bacon should warn about sounds that failed to load?

To allow custom sounds, we need a mutable sound list. This is achieved through an once_cell::Lazy<RwLock<HashMap<&str, Sound>>>. It's a global static because the sound list is basically global in each run. RwLock is suitable for our situation where writes are rare and reads are often. HashMap is to ease retrieving sounds by name. Names and bytes are leaked because they are basically static in each run.

With custom sounds, users might want to reduce executable size by removing the default sounds. Make them an optional feature.

With custom sounds, there's no way to know their durations beforehand, like for default sounds. rodio has the `Source::total_duration()` trait method, which is implemented on its `Decoder`, so that is used. But it returns an `Option<Duration>`, i.e. reading is not guaranteed. Thus we ignore the interrupt for such sounds, for now.

…eature

nc7s · 2025-03-31T17:49:48Z

Hmm, I seem to have mistakenly assumed it's OK to only listen for interrupt if the duration is known. The sound is then not played at all, not yet sure why.

But instead of trying to "fix" acquiring duration, I think it's reasonable to just set a maximum duration: it's a notification sound, after all, it shouldn't last longer than a few seconds.

c-git · 2025-03-31T19:20:04Z

Hmm, I seem to have mistakenly assumed it's OK to only listen for interrupt if the duration is known. The sound is then not played at all, not yet sure why.

But instead of trying to "fix" acquiring duration, I think it's reasonable to just set a maximum duration: it's a notification sound, after all, it shouldn't last longer than a few seconds.

I use the default sounds and have no issue but when I used to do custom sounds (in the previous framework I used when I was using python) they were several seconds long. It was actually an audio report of the outcome. It would tell me if it passed or failed and the error count. I know the sounds won't be doing that but just pointing out that if we allow custom duration is not necessarily safe to assume it will be short. I haven't had time to look at the code myself. Is it possible to send to a separate thread or something like that if it's blocking something else so the user can choose any sound they like? If it's too complicated maybe it's not worth it but I just wanted to point out that I don't think it's a safe assumption. Alternatively we could ask the use for a max duration to play from the sound. Maybe it's their favourite song and they don't want to crate a cropped version but they don't actually want the whole 5 minutes.

nc7s · 2025-04-01T04:18:17Z

Alternatively we could ask the use for a max duration to play from the sound.

Sure could do, add an option duration to [sound.collection] entries and maybe the play-sound action.

Maybe it's their favourite song and they don't want to crate a cropped version but they don't actually want the whole 5 minutes.

Agreed, which is basically the source of my idea of a default duration/cutoff. But then again,

not necessarily safe to assume it will be short

It's a notification sound, when a job is done - there is a reason why the pre-picked sounds are short, with the longest being 2 seconds (2, success) and the shortest being 0.25 seconds (store-scanner-beep), and the longer ones mostly just have some hardly audible fade-in and -out. Notifications happen a lot. A lot. Sounds gonna overlap if long yet not cut off. There's gonna be cutoff anyway, so why not set a "standard" one? Ability to set custom duration could be offered if the user feel like it, as said above.

.. But this all comes from the lack of assured ability to measure duration. It's baffling symphonia couldn't. I'm experimenting with it to see if this can be fixed. Could even go as far as suggesting to replace mp3 with ogg - mp3 is not an open format, even if its patents expired; compared, ogg (container) and vorbis/opus (audio stream) are open formats. Still, being able to set the duration for a sound seems useful.

nc7s · 2025-04-01T06:57:44Z

The culprit of rodio being unable to read duration should be RustAudio/rodio#696, fixed in RustAudio/rodio#697, merged early last month. I've asked if they could release it to avoid a git+rev dependency.

nc7s · 2025-04-01T07:03:34Z

FWIW, if we are to not disturb the user listening to some music, duration.unwrap_or(Duration::MAX) would allow playing something with the duration up to practically infinity. It could even be used to get rid of the duration field of embedded sounds.

c-git · 2025-04-01T13:42:12Z

I wouldn't over index on it. I might be the only person who sets long running tasks in bacon and wants a longer sound.

nc7s force-pushed the custom-sound branch from 9dd54f9 to 3a99f9c Compare March 10, 2025 12:55

Canop reviewed Mar 12, 2025

View reviewed changes

nc7s force-pushed the custom-sound branch from 3a99f9c to a85a71f Compare March 31, 2025 17:15

nc7s added 6 commits April 1, 2025 01:16

add default-sounds feature

0f53f56

With custom sounds, users might want to reduce executable size by removing the default sounds. Make them an optional feature.

support play-sound(path=...)

b3acc3e

add [sound.collection] and load it on SoundConfig application

70706ed

update docs and default-prefs with custom sounds and default-sounds f…

bb6e7bc

…eature

nc7s force-pushed the custom-sound branch from a85a71f to bb6e7bc Compare March 31, 2025 17:17

PREVIEW: fix reading duration with upcoming rodio fix, Duration::MAX

5c64cc6

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement custom sounds #341

Implement custom sounds #341

nc7s commented Mar 9, 2025 •

edited

Loading

Canop Mar 12, 2025

nc7s Mar 13, 2025

nc7s Mar 31, 2025

nc7s commented Mar 31, 2025

c-git commented Mar 31, 2025

nc7s commented Apr 1, 2025 •

edited

Loading

nc7s commented Apr 1, 2025 •

edited

Loading

nc7s commented Apr 1, 2025 •

edited

Loading

c-git commented Apr 1, 2025

Implement custom sounds #341

Are you sure you want to change the base?

Implement custom sounds #341

Conversation

nc7s commented Mar 9, 2025 • edited Loading

Canop Mar 12, 2025

Choose a reason for hiding this comment

nc7s Mar 13, 2025

Choose a reason for hiding this comment

nc7s Mar 31, 2025

Choose a reason for hiding this comment

nc7s commented Mar 31, 2025

c-git commented Mar 31, 2025

nc7s commented Apr 1, 2025 • edited Loading

nc7s commented Apr 1, 2025 • edited Loading

nc7s commented Apr 1, 2025 • edited Loading

c-git commented Apr 1, 2025

nc7s commented Mar 9, 2025 •

edited

Loading

nc7s commented Apr 1, 2025 •

edited

Loading

nc7s commented Apr 1, 2025 •

edited

Loading

nc7s commented Apr 1, 2025 •

edited

Loading