-
-
Notifications
You must be signed in to change notification settings - Fork 21.1k
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
ResourceImporterWAV: Use dr_wav
to read file data
#96545
base: master
Are you sure you want to change the base?
Conversation
@@ -35,6 +35,15 @@ | |||
#include "core/io/resource_saver.h" | |||
#include "scene/resources/audio_stream_wav.h" | |||
|
|||
#define DRWAV_IMPLEMENTATION | |||
#define DR_WAV_NO_STDIO | |||
#define DR_WAV_LIBSNDFILE_COMPAT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Despite not using libsndfile
, the way Godot converts streams from/to f32 follows similar calculations, therefore not defining this leads to some test errors (save/reimport datas being different).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Horray
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 |
Modified the check again, this time it actually checks the amount of samples (maximum of 2147483647). Normally, the importer uses an |
Does the dr mp4 problem affect dr_wav? Not sure of the exact problem. |
The problem with Doesn't apply to WAV. |
0925491
to
ce27e00
Compare
Added |
else if (loop.type == drwav_smpl_loop_type_backward) | ||
loop_mode = AudioStreamWAV::LOOP_BACKWARD; | ||
loop_begin = loop.firstSampleByteOffset; | ||
loop_end = loop.lastSampleByteOffset; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "Byte" in the names here is misleading, it's actually the offsets of loop begin/end in frames, not bytes.
I have tested this considering the old read code actually copied a sample value directly, not a byte offset.
Works the same way as the previous implementation just fine.
ffbc68e
to
723e117
Compare
Updated ResourceImporterWAV documentation to point out it also supports AIFF. |
Changes
In
ResourceImporterWAV::import()
, replaces the entire data read step with one that usesdr_wav
.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.
AIFF support
dr_wav
is also able to read AIFF files, soResourceImporterWAV
was enabled to recognize it too.The importer was renamed from
Microsoft WAV
toMicrosoft WAV/Apple AIFF
to reflect that.wave
,aif
,aiff
andaifc
file extensions have been added as recognized extensions alongsidewav
.Others
Since both
AudioStreamWAV
andResourceImporterWAV
use a 32-bit integer variable to store the amount of PCM frames, I added a check that denies import if the number of frames exceedINT32_MAX
. This doesn't break compatibility, as attempting to import a file bigger than that would actually cause issues to the importer.