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

3.6.0: AudioChannel.Speed has no effect #1503

Closed
ivan-mogilko opened this issue Jan 16, 2022 · 8 comments · Fixed by #1511
Closed

3.6.0: AudioChannel.Speed has no effect #1503

ivan-mogilko opened this issue Jan 16, 2022 · 8 comments · Fixed by #1511
Labels
context: audio context: script api type: bug unexpected/erroneous behavior in the existing functionality
Milestone

Comments

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Jan 16, 2022

In 3.6.0 setting playback speed in script (using AudioChannel.Speed property) has no effect whatsoever.

The script channel's speed value is being set correctly, which in turn sets OpenAL's source AL_PITCH param.
But apparent problem is that mojoAL is missing pitch implementation completely. It has it applied to a data structure, but I found no code that would use this value.

@ivan-mogilko ivan-mogilko added type: bug unexpected/erroneous behavior in the existing functionality context: audio context: script api labels Jan 16, 2022
@ericoporto
Copy link
Member

Probably related issue there is icculus/mojoAL#2

@ivan-mogilko
Copy link
Contributor Author

For the future reference, mentioned in that issue, people in another project added their own pitch impl to their copy of mojoal:
supertuxkart/stk-code@33459e8

@ivan-mogilko
Copy link
Contributor Author

ivan-mogilko commented Jan 16, 2022

On the other hand, it might be that OpenAL's "pitch" by concept is not a direct equivalent to the playback speed.
In this, and any case, we may also implement resampling in the engine code, and pass altered sound data into mojoal.

@ericoporto
Copy link
Member

MojoAL now has a pitch implementation. :)

@ivan-mogilko
Copy link
Contributor Author

ivan-mogilko commented Jan 21, 2022

Aha. Commit for the reference: icculus/mojoAL@47b22d0

EDIT: from the description it seems like this is not an equivalent to Speed though. I will have to test how it works.

@ivan-mogilko
Copy link
Contributor Author

ivan-mogilko commented Jan 21, 2022

So, I tried it out, and mojoAL's Pitch is not what we need for Speed, it's a separate kind feature. As mentioned in the commit description, setting pitch only adjusts the sound samples, but it does not change the sound duration.

When reading OpenAL specification though, here what it states, in a paragraph for AL_SEC_OFFSET:

This value is based on byte position, so a pitch-shifted source will have an exaggerated
playback speed. For example, you can be 0.500 seconds into a buffer having taken only
0.250 seconds to get there if the pitch is set to 2.0.

Unfortunately, the paragraph for AL_PITCH itself does not mention anything about this.

@ivan-mogilko
Copy link
Contributor Author

ivan-mogilko commented Jan 21, 2022

As mentioned by @ericoporto elsewhere one may e.g. pass different frequency into the alBufferData call to get the speed changing result.
UPDATE: unfortunately this particular method does not allow to change the frequency for a playing sound dynamically. Only if you set the different freq at the start.

@ivan-mogilko
Copy link
Contributor Author

I made an experiment with resampler, which changes frequency before passing to OpenAl. So OpenAl thinks that the frequency is same, and sound ends up playing slower and faster; result matches the one of 3.5.1 from what I mat tell.
ivan-mogilko@40af1c2

There are couple of issues with this, first the resampling is done per chunk, so there may be lags in speed change. Also position calculation is based on what openal returns, and so far not precise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
context: audio context: script api type: bug unexpected/erroneous behavior in the existing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants