Skip to content

KHR_audio_emitter: Rename pitchSpeed to playbackRate and rename autoplay#248

Merged
aaronfranke merged 1 commit intoomigroup:mainfrom
aaronfranke:audio-playback-rate
Apr 10, 2025
Merged

KHR_audio_emitter: Rename pitchSpeed to playbackRate and rename autoplay#248
aaronfranke merged 1 commit intoomigroup:mainfrom
aaronfranke:audio-playback-rate

Conversation

@aaronfranke
Copy link
Copy Markdown
Member

@aaronfranke aaronfranke commented Apr 9, 2025

Since this property controls the combined pitch and playback speed, I opened PR #239 with the name "pitchSpeed". However, this differs from Web Audio, and multiple people have expressed confusion.

This PR renames the "pitchSpeed" to "playbackRate" to match Web Audio. However, I intentionally kept the description the same, but added even more text to clarify that this does not resample and pitch+speed are locked in together.

This PR also renames autoPlay to autoplay because it seems that this is the dominant casing across the industry.

@aaronfranke aaronfranke force-pushed the audio-playback-rate branch from 1fa6a21 to d159492 Compare April 10, 2025 16:57
@aaronfranke aaronfranke changed the title KHR_audio_emitter: Rename pitchSpeed to playbackRate KHR_audio_emitter: Rename pitchSpeed to playbackRate and rename autoplay Apr 10, 2025
Copy link
Copy Markdown

@yankscally yankscally left a comment

Choose a reason for hiding this comment

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

discussed in omi gltf meeting:

"autoplay" is defined as one word, another example is is "username" :
https://english.stackexchange.com/a/43440

we can find lowercase spelling of this everywhere:
https://developer.mozilla.org/en-US/docs/Web/Media/Guides/Autoplay#autoplay_and_autoplay_blocking
https://www.w3schools.com/jsref/prop_video_autoplay.asp
(and more)

playbackRate is also matching Web Audio.

Copy link
Copy Markdown

@yankscally yankscally left a comment

Choose a reason for hiding this comment

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

welcome changes. read conversation in PR for the full review.

@aaronfranke aaronfranke merged commit 7f6c6a7 into omigroup:main Apr 10, 2025
2 checks passed
@aaronfranke aaronfranke deleted the audio-playback-rate branch April 10, 2025 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants