tesselode / kira

Library for expressive game audio.
https://crates.io/crates/kira
Apache License 2.0
865 stars 45 forks source link

Slice and loop region #102

Closed Roms1383 closed 2 months ago

Roms1383 commented 2 months ago

Hi, I'm currently exploring ways to expose current_duration + total_duration over both StaticSoundData and StreamingSoundData<T>, and there's something that keep getting me confused:

shouldn't the slice and loop_region be internally modified ? (without touching public API)

Context

As far as I understand, if we e.g. have:

Even more, if we follow-up with the same example and:

Then as far as I can tell we have a bug because on one side:

Worse, both regions do not even overlap. So when it gets played, does it:

Suggestions

Naive solution

As far as I understand, and please correct me if I'm wrong, but:

This way it is guaranteed to always have a correct slice with a consistent duration and interface does not require any breaking change.

More elaborate solution

Unless you purposely want to be flexible and let crate users being able to specify both a slice which can contains a smaller loop_region to have a sound that starts playing e.g. from 1s to 9s while looping on the last 3s for example. But in that case, shouldn't the loop_region always be a smaller region contained inside the current slice, but it's not currently validated by the setters.

Good-enough solution

If no change at all is preferable, at the very least it feels like the API should expose:

Current state for downstream users

Last but not least it could be a concern left to downstream crate consumers, but there's something particularly annoying in the current state of the API.

We can perfectly calculate duration(s) ourselves for StaticSoundData by doing something like:

impl SomeTrait for StaticSoundData {
   fn total_duration(&self) -> Duration { self.slice(None).loop_region(None).duration() }
}

Except the downside is that StreamingSoundData<T> requires a self instead of a &self for StaticSoundData, forcing every method which needs the duration of a T: SoundData to automatically require self to be agnostic over both types.

Thanks for your time :)

Roms1383 commented 2 months ago

Now I also realize that looping sound, and some streaming sounds, can be considered as having an Infinite duration.

But it could still be useful to expose a way to know what is the looping_region portion of audio duration. And for streaming sound that never ends, Infinite would also be a plausible way to express it.

Roms1383 commented 2 months ago

I'm willing to open a PR if it can helps, but I need your feedback first to be sure to not go in the wrong direction.

tesselode commented 2 months ago

I think you're misunderstanding the slice feature. Slicing a sound is meant to be semantically like you created a brand new sound with a portion of the original sound's audio data. It's not actually copying any audio data, but you could think of it like it is. As such, the loop region is relative to the slice. Say you have a sound with data points A through F:

A B C D E F
0 1 2 3 4 5

And then we take a slice of the sound from times 2-5 (inclusive):

C D E F
0 1 2 3

If we set the loop region from 1-2 (inclusive), we'll hear D and E repeatedly, even though those were at positions 3 and 4 in the original sound.

Going back to your example where you have a slice of 6..7 and a loop region of 2..4, that loop region is out of bounds, but not in the way you think it is. You'd be trying to loop a region after the end of the sound, since the sliced sound is only 1 second long and you're trying to play a portion 2 seconds in. (I believe in this case, the sound wouldn't play at all, but I'd have to check what the behavior is because I haven't made any promises about what that behavior should be.)

Roms1383 commented 2 months ago

Thanks for the follow-up, this explanation actually clarify everything (and would be worth added in the docs).

I've opened PR #103 to expose ways to retrieve durations.