SolarLiner / bevy-kira-components

Experimental Bevy <-> Kira integration crate
https://solarliner.dev/bevy-kira-components/bevy_kira_components/
MIT License
9 stars 6 forks source link

Expose playback_rate to AudioFileSettings #20

Closed mgi388 closed 4 months ago

mgi388 commented 5 months ago

Summary

playback_rate exists in:

https://github.com/tesselode/kira/blob/ed69848baf290c2def2b58412b4b0a035c435fd7/crates/kira/src/sound/static_sound/settings.rs#L24

And in:

https://github.com/tesselode/kira/blob/ed69848baf290c2def2b58412b4b0a035c435fd7/crates/kira/src/sound/streaming/settings.rs#L22

But there's no way to set it in bevy-kira-components.

Implementation proposal

Expose it in AudioFileSettings:

/// Settings available to the user when instantiating an audio file.
#[derive(Debug, Component, Deserialize, Serialize)]
pub struct AudioFileSettings {
    /// By default, sounds will start playing right away when inserted. Setting this to `true`
    /// prevents that.
    pub start_paused: bool,
    /// Volume at which the audio will play at.
    pub volume: f64,
    /// The playback rate of the sound.
    ///
    /// Changing the playback rate changes both the speed and the pitch of the
    /// sound.
    pub playback_rate: f64,
    /// Panning (in 0..=1) for the sound, where 0 is hard left, and 1 is hard right.
    pub panning: f64,
    /// Optionally loop a region of the sound (given in seconds)
    pub loop_region: Option<Region>,
    /// Only play a specific region of the file
    pub play_region: Region,
    /// Play the file in reverse (not available for streaming sound files)
    pub reverse: bool,
    // pub start_time: StartTime, // TODO: Implement with serializable types
}

As with volume, just support fixed and so expose it as an f64/Fixed.

Callers could then do this:

commands.spawn(
    AudioFileBundle {
        source,
        settings: AudioFileSettings {
            loop_region,
            volume,
            playback_rate: 0.25, // play at 25% of the usual speed
            ..default()
        },
        ..default()
    },
);

Looks like a pretty tiny PR unless I'm missing something, let me know if you're happy with this approach and I can submit it.

SolarLiner commented 5 months ago

Yeah that is would be a pretty small PR to implement -- expose the value, and integrate it in the settings structs when creating the asset.

mgi388 commented 5 months ago

I already have the changes locally and I’ve been using them. It seems a pretty tiny change. I’ll make the PR when I get a chance. Hopefully I can also update an example or add one if it’s appropriate.