Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#77: Add in pause after specific interval, "skip" silences #71

Closed
wants to merge 3 commits into from

Conversation

jw1540
Copy link
Contributor

@jw1540 jw1540 commented Mar 9, 2021

PR adds a crude implementation on skipping silences. It basically checks when the average power for the currently playing buffer drops below an arbitrary value, and then increases the rate. Once the power returns above the value, the rate is reduced back to 1.0.

Pausing after a time just runs a timer for that period and then calls pause. Perhaps it should also observe the playing status and remove itself if the audio file ends prior to the timer completing?

@tanhakabir
Copy link
Owner

Awesome thanks! I'll look into this tonight

@tanhakabir tanhakabir self-requested a review March 9, 2021 21:55
@tanhakabir
Copy link
Owner

So something I want to make sure is that the player itself is pretty light and extensible in terms of manipulations being attached to the engine. Meaning I want the player at the core to be almost like AVAudioEngine that can stream.

But I also love your feature suggestions and think others would find super valuable! I'm looking now into extracting out these to be in their own namespace of "Extra Features" which wouldn't be as tightly coupled to the core engine source code but still give the same functionality. I'll co-commit these changes so that your changes exist in the adjacent namespace!


let meterLevel = self.scaledPower(power: avgPower)
if meterLevel < 0.6 {
self.changeRate(to: 1.5)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to use hardcoded 1.5 for speeding up to skip silences as they might have the audio rate set to 4 or something by default.

There's 2 options here:

  1. We change this to add to the current rate by some appropriate factor. I'm think this rate increase for skipping silences should be less at higher rates so we'd need to determine some sort of ratio.
  2. We assume that at rates above some number, like 3, the skip silences feature wouldn't be noticeable/helpful and then we could just restrict the rate change to a lower range of original rates. This makes it easy to just add some constant to the current rate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a really good point that I hadn't considered. I like both suggestions and think a mix of the two would be best? Applying some appropriate factor definitely seems the right way to go and if the rate increases beyond to a point where the skip would be negligible it just switches the tap off?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created an issue for this #82

@tanhakabir
Copy link
Owner

I think I'd need to expose a point for taps like how we have one for audioModifiers to be fully extensible.

@jw1540
Copy link
Contributor Author

jw1540 commented Mar 10, 2021

@tanhakabir I wonder if it's worth pulling these out into two separate PR's? That way we can add a small feature increment whilst thinking a little more about how to approach skipping silences.

@tanhakabir
Copy link
Owner

@jw1540 yeah if you're able to separate them out that would be great!

@tanhakabir
Copy link
Owner

Hey @jw1540 what resources did you use for skipping silences? I want to document that in the code for future reference

@tanhakabir
Copy link
Owner

Asked for documentation in #81

@tanhakabir tanhakabir closed this Mar 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants