Add total duration

[Roms1383]

Hi, this is a simple QoL PR to add total_duration method to both StaticSoundData and StreamingSoundData<Error>.

Motivation

To retrieve a StreamingSoundData's total duration whose slice has already been set currently requires to reset its slice to None, which requires ownership and is further complicated by the fact that StreamingSoundData does not implement Clone.

With this change, the total duration can be retrieved from both data, simply requiring shared reference.

[Roms1383]

Any objection to this small addition @tesselode?

[tesselode]

I have the same objections we discussed here.

[Roms1383]

The issue is that my plugin simply cannot operate without it, forcing me to sit on a fork (not too bad, but still).

The main issue coming from my plugin being agnostic over both static and streaming sounds and, since StreamingSoundData<T> is not Clone, it cannot retrieve initial total duration without consuming data, preventing me to use it afterwards.

The difference with traditional usage where a game developer makes its own audio engine and has control over everything is that my audio plugin is used by multiple mod creators independently of each other, and I have no control over what they will do at runtime.

Consider this concrete use case:

• imagine a sound that lasts 10s.
• mod A predefines this sound in YAML with a custom region, from e.g. from 2s to 8s.
• for some reason, later on at runtime mod A wants to play this sound with e.g. a different custom region than previously defined in YAML, effectively overriding it for this specific call to play.
• it calls the scripting method using ext optional settings parameter to do so: https://github.com/cyb3rpsych0s1s/audioware/blob/main/crates/audioware/reds/Ext.reds#L13
• which in my audio plugin :
  • first validate ext settings to make sure e.g. that the region actually fits in audio's duration: https://github.com/cyb3rpsych0s1s/audioware/blob/main/crates/audioware/src/engine/mod.rs#L221 (and its implementation details https://github.com/cyb3rpsych0s1s/audioware/blob/b4c380eb88c894e8741544de429b9f940baf67e7/crates/manifest/src/de/setting.rs#L299).
  • then if valid, it plays the StreamingSoundData afterwards: https://github.com/cyb3rpsych0s1s/audioware/blob/main/crates/audioware/src/engine/mod.rs#L248.

You can see that, as soon as it validates it cannot play it anymore (since validation took ownership).
And I can't realistically skip validation because mod A could make a mistake (especially if working with sounds dynamically from the scripting language), specify an invalid region e.g. from 11s up to 15s so I'm required to check this, lest people might end up having bugs.

In this case, num_frames cannot give me the original (unsliced) duration either, forcing me to rely on slice which, in StreamingSoundData, consumes self.

It's especially mandatory in my case because if gamers start having bugs, or modders do not get the expected auditive outcome, usually the blame is gonna fall back on the native plugin while it could actually just come from a setting incorrectly specified.

[Roms1383]

Now I understand that my use case is niche and might not necessarily fits common usage.
You're also totally right about the mental model, but I still find this request reasonable:

• people who never use it will have it removed by the compiler
• the cost on compile time for 2 additional methods I believe is probably pretty negligible
• it could be renamed to make it clear and preserve the mental model: for example renaming them to initial_duration or real_duration

[tesselode]

Thanks for elaborating on your use case. I can see how you're in a bind with the current API. I have a couple takeaways from this:

1. Maybe the slicing mental model isn't that great

My goal with this mental model was to make it clear that loop regions and seek positions are relative to the start of the slice, but now I'm wondering if it would just be better to make a playback_region field that doesn't change how durations are reported. I'd still have to choose whether loop regions and seek positions are relative to the start of the slice or the start of the original audio, but I can just pick one and document it somewhere.

2. It would be nice if StreamingSoundData was Clone

This should be doable, but it would require a breaking change. As you can probably tell by Kira's history, though, I'm not averse to breaking changes.

[Roms1383]

Please do not make complicated changes for my niche use case, especially not breaking changes.

Your mental model is great, and streaming sound do not need to be Clone.

I was just looking for simple additional methods, but they can sit on my fork it's fine too.

[tesselode]

I thought about it a little more after posting that reply, and the playback_region field I proposed would be almost exactly the same as the existing slice field. It's literally only the duration methods that would change. And I could see the value in having both kinds of duration method.

How do you feel about the name unsliced_duration? Also, would you be willing to add corresponding unsliced_num_frames functions?

[Roms1383]

Ok I changed total_duration to unsliced_duration it's indeed a way better name :)

Regarding unsliced_num_frames I don't get if it's necessary since it'd be basically equivalent to &self.frames.len() (StaticSoundData) or &self.decoder.num_frames() (StreamingSoundData).

Likewise I don't understand the change mentioned for playback_region. Isn't slice already explicit enough ?

[tesselode]

slice is fine, I feel it might be confusing since there's a "slice" concept in Rust already. But I don't think it's worth making a breaking change for, so no need to rename that.

[Roms1383]

any other change(s) missing ?

[tesselode]

nope, this looks good!

[tesselode]

thank you!