Exposed rodio API for skipping first part of a sample #1001

0----0 · 2021-12-18T22:17:49Z

Hi! I just happened to need this little bit of rodio API, and it seemed like the easiest way to get it was to patch ggez. I don't know if you'd consider it too cluttered to expose this in ggez's API, but since it's so tiny, I figured I might as well just submit the pull request and let you decide.

You can read about the rodio method being called here: https://docs.rs/rodio/0.14.0/rodio/source/trait.Source.html#method.skip_duration . Basically, it just skips the first part of a sound source. This can be useful, for example,

If you're processing an audio event "late" due to networking delays, but you want to keep the audio in-sync with the visuals
If you want to play only the latter part of a sound effect
If you want to start playing a piece of music partway in

All the same, it's a bit niche, and these use cases might be better covered by just using rodio directly. Since calling skip_duration wraps the entire sound sample iterator in a SkipDuration iterator, this might add a small overhead to any audio being played, but it's probably not a big deal, and it's consistent with how the ggez interface exposes other rodio features.

PSteinhaus · 2021-12-19T20:55:36Z

I think this is a useful feature.

Honestly, audio has been a bit of a thorn in my side for a while. I haven't actively used it in any projects yet, so I'm probably not the best judge, but it always seemed a bit too limited to me and now that you've published this PR I realize that the thing that annoyed me most was probably not being able to jump forward in sound.

PSteinhaus · 2021-12-19T21:38:12Z

I'm wondering whether we should add a comment somewhere on how elapsed reacts to skip_duration.

Since you added the call to skip_duration before periodic_access the skipped duration isn't included in the duration returned by elapsed. That's how it should be (based on the documentation of elapsed, which says:)

Get the time the source has been playing since the last call to play().

From that documentation it should be clear, that skip_duration has no influence on elapsed, but I think mentioning it in the documentation of skip_duration can't hurt.

0----0 · 2021-12-20T14:07:36Z

From that documentation it should be clear, that skip_duration has no influence on elapsed, but I think mentioning it in the documentation of skip_duration can't hurt.

That seems reasonable! While I was there, I actually remembered another little undocumented gotcha: set_fade_in currently applies before set_skip_duration, meaning that the fade-in applies to the original source, not to the part you actually play. For example, if you fade in 2 seconds, then skip 2 seconds, it will be as if you just skipped 2 seconds--you won't hear the fade-in. When I was originally writing the addition, I thought this was better, but upon reflecting on it, I think it would be better to have the fade-in after the skip.

Pros of fade-in before skip:

If you want "fade-in" to be an effect applied to the base audio source, and you just want to use "skip" for seeking around the audio source for technical/networking reasons, then this is more logical

Pros of skip before fade-in:

Whether for networking or for any other reason, if you start playing any audio source from the middle, then you probably want at least a subtle fade-in to make it less jarring/poppy
Treats the skip as being "Consider this the new start of the audio source," with effects applied on top of that

If we move skip before fade-in, I think there's also a good argument for moving it before speed, so that you don't have to do the funky math of "Okay, I want to start playing the audio from this timestamp, but I'm playing it at 2x speed, so multiply the timestamp by 1/2..."

What do you think? Should I adjust the order of operations while I'm documenting it?

PSteinhaus · 2021-12-20T15:33:00Z

That seems reasonable! While I was there, I actually remembered another little undocumented gotcha: set_fade_in currently applies before set_skip_duration, meaning that the fade-in applies to the original source, not to the part you actually play. For example, if you fade in 2 seconds, then skip 2 seconds, it will be as if you just skipped 2 seconds--you won't hear the fade-in.

Ah, nice catch! Didn't think of that yet.

What do you think? Should I adjust the order of operations while I'm documenting it?

Yes, I think the second option is more intuitive and useful. Go ahead and move it in front of fade in and speed (though I think I read that the number of samples to skip is calculated by rodio taking the current speed into consideration anyway).

The documentation of skip_duration should definitely explain that this function changes the timestamp from which the sound starts playing and that all other effects then apply from that new starting position onwards.

PSteinhaus · 2021-12-20T15:40:01Z

If we wanted to go wild we could also just make two functions and call the first option skip_duration and the second start_from or something like that. But I'm pretty sure that would just needlessly confuse people.

0----0 · 2021-12-22T03:46:52Z

If we wanted to go wild we could also just make two functions and call the first option skip_duration and the second start_from or something like that. But I'm pretty sure that would just needlessly confuse people.

I do think that having two functions would probably be overkill right now, but now that you mention it, start_from might be a more intuitive name than skip_duration for the functionality as we intend. What do you think about having the user-facing function name be something like set_start_from or set_start for now?

PSteinhaus · 2021-12-22T11:29:37Z

Hmm, set_start is a bit more intuitive than set_skip_duration, but it also loses the similarity to the underlying rodio function. I guess I'd still prefer set_start though.

set_start now operates after repeat_infinite but before other audio effects. The docs have been updated to elaborate on these interactions.

0----0 · 2021-12-24T06:03:39Z

I changed the external name to set_start as we discussed, as well as its place in the order of operations so that it occurs before speed and fade_in. However, I left the internal name as skip_duration, matching the underlying rodio function. I noticed set_pitch/speed did something similar, keeping an intuitive user-facing name while using an internal rodio-matching name, so I thought I'd give it a shot for consistency. I also expanded on the interactions in the doc comments. Lemme know if that works!

Exposed rodio API for skipping first part of a sample

77d5c12

Changed set_skip_time to set_start, updated docs

6a6f517

set_start now operates after repeat_infinite but before other audio effects. The docs have been updated to elaborate on these interactions.

PSteinhaus merged commit 650ae29 into ggez:devel Dec 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Exposed rodio API for skipping first part of a sample #1001

Exposed rodio API for skipping first part of a sample #1001

Exposed rodio API for skipping first part of a sample #1001

Exposed rodio API for skipping first part of a sample #1001

Conversation