Move WAV loading functionality to a `wav_loader` module with `dr_wav` (adds AIFF support)

[DeeJayLSP]

trafficstars

Changes

Moves the entire AudioStreamWAV read and parsing functionality into a module, which delegates the responsibility to the dr_wav library and will be required to use the respective AudioStreamWAV::load_from_* functions (but not required for playing AudioStreamWAV audio or creating streams through AudioEffectRecord).

Formats already supported by Godot and loop info detection should remain supported without regressions.

Extra WAV encoding formats like A-law, μ-law, MS ADPCM and IMA ADPCM can now be imported.

Note: All of them will be decoded on load, so any loss will be retained and passed to the compress modes.

Note 2: Even if Godot is able to compress and play audio as IMA ADPCM, the way it's compressed within Godot doesn't match the usual compression done in WAV files, so those couldn't be imported without decoding anyway, unless a compatibility breaking change is done to modify how Godot encodes and plays it.

Indirectly fixes #90957. Indirectly fixes #107265 by replacing the code, delegating the task of finding tags to dr_wav. Fixes #40320 (it was already fixed in master, just doing this so it will be closed for good)

(Indirectly because this isn't meant to be a fix for them, but an enhancement that replaces the code causing the issues and mitigates them)

Technical changes

• The entire file load functionality of AudioStreamWAV was moved to a module named wav_loader. It defines function pointers that should do the same job. If the module is disabled, an error macro activates. An approach like this is used by the vhacd module for convex decomposition.
• The "READ DATA" step of the load was replaced, from a custom in-house solution to using dr_wav. Function pointers in the module generate a drwav struct that reads from buffer or from file as they should, then pass it to an universal load.

AIFF support

dr_wav is also able to read AIFF files, so ResourceImporterWAV was enabled to recognize their extensions (.aif, .aiff and .aifc). The importer was renamed from Microsoft WAV to Microsoft WAV/Apple AIFF to reflect that.

load_from_buffer and load_from_file should recognize AIFF data/files as well.

When it comes to sound effects, freesound.org provides much more AIFF than Ogg Vorbis files, so I thought adding support was reasonable:

Actually load from file instead of a file buffer

#93831 changed the load function to copy the entire file data into memory from using FileAccess to get values.

You can tell dr_wav to use specific read/seek callbacks, so I wrote callbacks to make load_from_file read using FileAccess instead of a memory copy.

Fixes to metadata tags

As of #99504, loading supports metadata tags. However, the current implementation on master is broken:

dr_wav can read WAV metadata tags, and due to a rebase to make it work on top, it works nicely:

Note: dr_wav can't read all types of tags expected in master, but at least it picks the most common and desired ones and works.

Note 2: Due to depending on dr_wav, this solution can't be used separately.

Extra Note

dr_alloc_callbacks.h exists because there are other DR libs in plans, namely dr_mp3 to replace the outdated minimp3, and it can be reused for all of them.

Caveats

Updated as of June 10, 2025

I noticed a 45 KiB size penalty on production template_release builds (77215168 -> 77260224). This PR was opened when the load procedure was editor-only, and during its development was moved into AudioStreamWAV to serve as a load function. I don't know if 45 KiB is too much for this, but as a side effect reduces the probability of errors.

And once again, it is a module that can be disabled.

[DeeJayLSP]

Added a check to detect if the container is RF64 or W64.

Even if they could be imported in theory, this is the best non-compatibility breaking way to deny getting a stream that is too big for AudioStreamPlaybackWAV. And it's not like anyone would export to those formats with a .wav extension.

[DeeJayLSP]

Modified the check again, this time it actually checks the amount of samples (maximum of 2147483647).

Normally, the importer uses an int to store the total amount of samples, therefore using a limit shouldn't actually break compatibility (in fact, probably fixes a potential overflow on import).

[fire]

Does the dr mp4 problem affect dr_wav? Not sure of the exact problem.

[DeeJayLSP]

Does the dr mp4 problem affect dr_wav? Not sure of the exact problem.

The problem with dr_mp3 is that it doesn't read a VBR header that informs an amount of samples to skip at the start.

Doesn't apply to WAV.

[DeeJayLSP]

Added .aifc extension to the list as it's supported too.

[DeeJayLSP]

Updated ResourceImporterWAV documentation to point out it also supports AIFF.

[DeeJayLSP]

Rebased.

I took the opportunity to redo how the data is read. dr_wav offers 3 ways: From a file through stdio, from memory and from custom read/seek functions.

Before the last push, it was loading the entire file data into the memory, then reading from memory, which resulted into a massive increase in memory usage during import if the file was too large.

Now it uses custom functions (lambdas) that tell FileAccess to react accordingly, effectively being an integration between FileAccess and dr_wav.

I don't know if I violated any Godot style guideline, so I'd like to ask for another review.

The patch in dr_wav could be applied directly into a define, so it was removed.

[DeeJayLSP]

PR and commit reworded to give emphasis to its enhancement.

[DeeJayLSP]

Addressing changes from #93831, which moved most of the WAV loading code from ResourceImporterWAV to AudioStreamWAV.

AudioStreamWAV documentation updated to explain that the load functions also work with AIFF files and data.

Since the load now requires loading into memory anyway, the old init_memory read has been restored.

[DeeJayLSP]

For some reason, when the tests call save_to_wav with an empty data, a SIGSEGV happens. And only on template builds.

This is odd since I didn't even touch that function.

Edit: it seems like getting the data pointer directly instead of using get_data() solved something. I don't get how it makes sense, but it fixes, so I applied it.

[DeeJayLSP]

Any probability of this being approved for 4.4?

[DeeJayLSP]

Rebased on top of master. I have undone a few adjustments to other unrelated things and kept mostly a direct dr_wav implementation.

[fire]

The best I can do is review for 4.5 and queue it for merge.

Test plan

• [ ] Find a few WAV and AIFF files from Wikipedia Commons and archive.org
• [ ] Test if the first 30 seconds sounds ok
• [ ] Try writing wav (optional?)

[DeeJayLSP]

Since the WAV import procedure was moved from the editor into the templates during the development of this PR (which was relying on it being editor only), I decided to make some builds for measurement.

On Linux builds, template_release got a binary size increase of 36 KB (71,979,280 -> 72,016,144). Would this be too impactful?

I don't see a solution to this other than making AudioStreamWAV::load_from_*() functions separate from the importer (but then template builds wouldn't be able to load AIFFs).

[DeeJayLSP]

The best I can do is review for 4.5 and queue it for merge.

Test plan

• [ ] Find a few WAV and AIFF files from Wikipedia Commons and archive.org
• [ ] Test if the first 30 seconds sounds ok
• [ ] Try writing wav (optional?)

This PR only changes function to read non-imported WAV files, not the playback (in fact, this should have gotten a topic:import label). Saving still uses Godot's own procedure.

Additionally, modifying AudioStreamWAV to use dr_wav's save procedure (not done in this PR) increases binary size by an additional 4KB.

Unit tests already cover this by generating a PCM stream, saving it, loading and comparing if the loaded stream is 1:1 to the generated one before saving. It's the reason DR_WAV_LIBSNDFILE_COMPAT is even defined, as the test was failing without it.

[DeeJayLSP]

Rebased once more.

A modification from #93831 bothered me. It changed the file read procedure to load the entire file into memory instead of using FileAccess to get values from disk.

Thanks to dr_wav, we can initialize a drwav with either memory or a custom FileAccess API. I brought back this solution, modified both functions and made a third one to receive an initialized drwav and do the rest of the load procedure.

[DeeJayLSP]

I was doing an experiment with dr_flac. By cuttting AudioStreamWAV::load_from_buffer in half, it is possible to make this editor-only by implementing dr_wav solely on ResourceImporterWAV.

On the other hand, I decided to wait for a patch in dr_wav as #40320 comes back worse with these changes.

[migueldeicaza]

AIFF is also the default format for Garage Band, so it comes in very handy to have this support.

[DeeJayLSP]

I realized removing a check from the file seek callback would speed up imports without making anything fail. I thought it was some kind of overhead from dr_wav.

[DeeJayLSP]

Something that just came to mind: how about making this a module similar to vhacd?

All the vhacd module does is enabling Mesh::convex_decompose() to work. How about reworking this into something that makes AudioStreamWAV::load_from_buffer() and AudioStreamWAV::load_from_file() work?

The size penalty would still exist in distributed editor and template builds, but at least there would be an easy option to make a custom build without it.

[DeeJayLSP]

Rework done.

The entire AudioStreamWAV::load_from_* functions have been reworked into a ~~dr_wav~~wav_loader module that can be left out of engine builds.

Much like the vhacd module is required to use Mesh::convex_decompose(), the ~~dr_wav~~wav_loader module will be required to use AudioStreamWAV::load_from_buffer() and AudioStreamWAV::load_from_file().

Since there is a small but considerable demand for AIFF support, but in order to do so in an easy way would result into a big size penalty, I thought a module would be the best approach.

[DeeJayLSP]

Rebased and renamed to a more specific title.

[DeeJayLSP]

Renamed the module to wav_loader to better reflect its purpose and avoid ambiguity, as it could cause people to think it is necessary for WAV playback.

[DeeJayLSP]

Rebased on top of #99504 changes (which I was really expecting to be approved after this one).

dr_wav has support for WAV metadata tags, so it's now handled by the load code. However, not all tag types added by that PR are supported (namely location, organization, keywords, medium and description). I don't think anyone uses those, but if they come from already imported files, they should remain.

~~I removed the AudioStreamWAV() constructor meant to remap tags, alongside the specific function. They are not needed since dr_wav already does it internally.~~

~~Also seriously? Most people using AudioStreamWAV won't even touch tags, and that constructor would be just a little more unnecessary overhead. It should have been a part of the load code all along.~~ Solved with a PR.

It has been tested with a WAV file from the PR above, and it seems to work accordingly:

EDIT: it seems the current metadata tag implementation in master is broken, so technically this one is a fix.

[DeeJayLSP]

I kinda managed to find a fix for the #40320 issue coming back, so this is pretty much done.

It isn't perfect, but unless the file is a rare case that uses anything other than loop points and frequent metadata tags, it should work nicely.

Edit: I found out dr_wav provides the first position of the data chunk, so clamping should work the exact same way it was before and shouldn't cause regressions.

[DeeJayLSP]

Rebased and tested on top of #107250.

I have tested this with as much WAV and AIFF files I could, including broken ones. No regressions found.

Asking one more time for this to be considered in 4.5.

[DeeJayLSP]

May I request a topic:import label? The function affected will be used by ResourceImporterWAV like 90% of the time.

And, well, it modifies an importer.

[DeeJayLSP]

I noticed 4.5 has entered beta phase. If this gets postponed to whatever version maintainers decide to put an eye on this, be aware that dr_wav isn't able to read some types of tags (feature added into AudioStreamWAV still in 4.5).

However, those are tag types almost no one uses. Should be less severe than things like, you know, removing support for very old machines.

[lyuma]

Since this was put on my place with the urgency (crash fix) and being labeled an import issue, I decided to look into this issue in more detail and give a review.

The appeal of this change is supposed to be that the code is rewritten or no longer necessary, but the problematic code was mostly moved to static Ref<AudioStreamWAV> load in modules/wav_loader/register_types.cpp - This means that many of the errors present in AudioStreamWAV that cause issue #90957 are still present even with this change.

For example, data.resize() should be checked for invalid return value, and not doing so can cause out of bounds or crashes. And there still appears to be some unsigned -> signed conversion for some variable such as format_channels

We should isolate bugfixes from the feature (adding AIFF). In particular, the bugs in the current WAV implementation are caused by (1) missing bounds checks, (2) not checking the return value of .resize() and (3) unsigned to signed conversion.

So to move forward, I would start with fixing these issues (1, 2, 3) in the current wav loader, and see if we can close the related issue #90957

As for the metadata bug #107265, this also seems like it's a simple bug and doesn't necessary require a full rewrite. I would like to understand more why it's not working correctly today before we go headfirst into a rewrite.

And finally, as for "Fixes #40320 (it was already fixed in master)" this looks like a 3.x issue, so it's no longer relevant for godot 4.x - we can remove the reference to this issue.

[DeeJayLSP]

Rebased on top of #107596, making it a requirement.

The missing tags are now supported in upstream dr_wav, so there should be no compatibility breaks anymore.

I also did some changes to how dr_wav reads from FileAccess (which brings back how WAV files were imported before AudioStreamWAV::load_from_file() was added), so no more lambdas are used.

The bug label may be removed.