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

AudioStreamPlaybackWAV: Inherit from Resampled #100652

Merged
merged 1 commit into from
Mar 19, 2025

Conversation

DeeJayLSP
Copy link
Contributor

@DeeJayLSP DeeJayLSP commented Dec 20, 2024

TL;DR: this is a non-breaking change that simplifies AudioStreamWAV a lot while improving the resampling quality.

A continuation and updated version of #83483, so everything that has been discussed there applies here. The original author, @wareya, has been added as the commit author, and I left myself as a co-author, as what I've done was mostly patching.

Removes the resampling code entirely from AudioStreamPlaybackWAV and makes it inherit from AudioStreamPlaybackResampled, fixes #58216. This brings the class in par with the Ogg Vorbis and MP3 equivalents, which already inherit from Resampled.

I have tested this with as many audio files I could, with every loop mode, and found no problems.

Any existing project needs no modifications to work with this PR. It barely modifies unexposed methods, so no compatibility breaks.

A few notable changes:

  • IMA ADPCM audio now gets interpolated.
  • do_resample did two things: convert samples from their format to AudioFrame, then apply linear interpolation. Interpolation code was removed and the function renamed to decode_samples.
  • DATA_PAD has been removed from AudioStreamWAV as it lost its purpose. Functions that required handling the pads have been simplified, potentially reducing memory overhead.
  • QOA caches were also removed, as they were a workaround workaround to make resampling work fine in the previous implementation. Unnecessary in this one.
  • Linux release template builds using production=yes had a 4KB reduction in binary size. Note however that AudioStreamPlaybackResampled have an internal buffer of 1056B.

Caveats

Files imported with loop mode set to Forward, Ping-Pong or Backward in a build before #96768 was merged will cause crashes, as it will try to read one index out of bounds. It can be fixed by simply reimporting.

@DeeJayLSP
Copy link
Contributor Author

DeeJayLSP commented Mar 6, 2025

Updated the offset variable back to int64_t. The max value can't be overflowed with a WAV file as those have a 32-bit size limit, and to overflow it with AudioEffectRecord it would take over 1.6 million years of recording (assuming 44100Hz stereo 16-bit).

Increment doesn't need to be multiplied by 8192, so I made it an int8_t as you only need 1 or -1.

Co-authored-by: DeeJayLSP <djlsp@proton.me>
@akien-mga akien-mga changed the title AudioStreamPlaybackWAV: inherit from Resampled AudioStreamPlaybackWAV: Inherit from Resampled Mar 18, 2025
@DeeJayLSP
Copy link
Contributor Author

This technically fixes #58216, could this get a bug label?

@akien-mga akien-mga added the bug label Mar 19, 2025
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Looks good to me from a cursory glance, and from reading the description. Let's get this tested.

@akien-mga akien-mga modified the milestones: 4.x, 4.5 Mar 19, 2025
@akien-mga akien-mga merged commit ac05256 into godotengine:master Mar 19, 2025
20 checks passed
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants