SDL_mixer icon indicating copy to clipboard operation
SDL_mixer copied to clipboard

MIX_LoadAudio_IO fails to decode a basic WAV (32-bit float, 22050)

Open AntTheAlchemist opened this issue 4 months ago • 19 comments

Migration went okay. xm music decodes and plays okay. But WAVs aren't decoding. This worked before the migration.

Stepped through code and in decoder_wav.c it fails here (in WAV_init_audio_internal()):

        if (chunk_length == 0) {
            break;
        } else if ((chunk_start_position + chunk_length) > flen) {
            return SDL_SetError("Corrupt WAV file (chunk goes past EOF)");
        }

chunk_length is ridiculously long. This comes after successfully loading valid FMT and DATA chunks.

Additionally, that error gets wiped out when it returns back to SDL_mixer.c's PrepareDecoder(), which sets a new error: SDL_SetError("Audio data is in unknown/unsupported/corrupt format");.

Worked previously. I'm digging deeper to rule out that something I've done...

AntTheAlchemist avatar Aug 14 '25 14:08 AntTheAlchemist

If the stream passed to MIX_LoadAudio_IO() contains only 1 WAV file, it will trigger an EOF, and naturally break out of the chunk parsing loop.

What's happening is this isn't triggered because my stream contains multiple WAVs in 1 file. This was handled okay before migration. There is no chunk that is size 0, so a break the loop never triggers.

Is there an official END RIFF chunk? Isn't there a header chunk that tells us exactly how much data to parse?

AntTheAlchemist avatar Aug 14 '25 14:08 AntTheAlchemist

Thanks for the report, can you attach the file that's showing the issue?

slouken avatar Aug 14 '25 15:08 slouken

Thanks for the report, can you attach the file that's showing the issue?

You mean the concatenated WAV file? I can't; they're copyrighted assets. I can put together a public domain safe file? Just take any WAV file and add random data to the end of the file, that should reproduce the bug, because it'll stop EOF from triggering out of the loop.

AntTheAlchemist avatar Aug 14 '25 15:08 AntTheAlchemist

Or you could load a WAV into an expanded allocated block, then pass a RAM IO of that block?

AntTheAlchemist avatar Aug 14 '25 15:08 AntTheAlchemist

Shall I knock up a full repro?

AntTheAlchemist avatar Aug 14 '25 15:08 AntTheAlchemist

Sure, that would be helpful, thanks!

slouken avatar Aug 14 '25 15:08 slouken

[Update] turns out there's a bug in my test code (fixed it here now), so a single WAV in a stream will decode successfully, but if the stream contains anything after the WAV file, it fails, pointing to the decoder requiring an EOF to break out of the chunk parser.

test.wav

@slouken ~~It turns out that even a single WAV in an IO stream fails to decode also.~~ Don't shoot me, but I got GPT-5 to knock-up a fix for WAV_init_audio_internal(), which works! I can share it here if you want. Here is the repro. Pressing 1 decodes from a stream with the single WAV in, and pressing 2 creates a padded RAM stream containing the WAV. ~~Both fail, but I can't confirm if it's for the same reason.~~ The original failure is certainly due to EOF not being triggered in a database of WAVs, which should be simulated by pressing 2.

#define SDL_MAIN_USE_CALLBACKS 1
#include <SDL3/SDL_main.h>
#include <SDL3/SDL.h>
#include <SDL3_mixer/SDL_mixer.h>

SDL_Window *win;
SDL_Renderer *ren;
SDL_IOStream *io;
MIX_Mixer *mix;
MIX_Track *trk;
MIX_Audio *wav;

SDL_AppResult SDL_AppInit(void **appstate, int argc, char * argv[]) {
	if (!SDL_Init(SDL_INIT_VIDEO | SDL_INIT_AUDIO))
		return SDL_APP_FAILURE;
	if (!(win = SDL_CreateWindow("Test", 600, 300, SDL_WINDOW_HIGH_PIXEL_DENSITY | SDL_WINDOW_RESIZABLE)))
		return SDL_APP_FAILURE;
	if (!(ren = SDL_CreateRenderer(win, NULL)))
		return SDL_APP_FAILURE;
	if (!MIX_Init())
		return SDL_APP_FAILURE;
	SDL_AudioSpec spec = { SDL_AUDIO_F32, 2, 44100 };
	if(!(mix = MIX_CreateMixerDevice(SDL_AUDIO_DEVICE_DEFAULT_PLAYBACK, &spec)))
		return SDL_APP_FAILURE;
	if(!(trk = MIX_CreateTrack(mix)))
		return SDL_APP_FAILURE;
	if (!(io = SDL_IOFromFile("test.wav", "r")))
		return SDL_APP_FAILURE;
	wav = NULL;
	SDL_SetRenderVSync(ren, 1);
	return SDL_APP_CONTINUE;
}

SDL_AppResult SDL_AppIterate(void *appstate) {
	SDL_SetRenderDrawColor(ren, 64, 64, 64, SDL_ALPHA_OPAQUE);
	SDL_RenderClear(ren);
	SDL_RenderPresent(ren);
	return SDL_APP_CONTINUE;
}

SDL_AppResult SDL_AppEvent(void *appstate, SDL_Event *event) {
	switch (event->type) {
	case SDL_EVENT_KEY_DOWN:
		switch (event->key.scancode) {
		case SDL_SCANCODE_1: // Press 1 to decode a single WAV file normally
			SDL_SeekIO(io, 0, SDL_IO_SEEK_SET);
			wav = MIX_LoadAudio_IO(mix, io, false, false);
			if(!wav)
				return SDL_APP_FAILURE;
			break;
		case SDL_SCANCODE_2: // Press 2 to decode WAV from a stream larger than the WAV (i.e. a database file); fails
			if (SDL_SeekIO(io, 0, SDL_IO_SEEK_SET) != -1) { // Just to give us scope to create variables in a case
				const size_t wav_len = (size_t)SDL_GetIOSize(io);
				const size_t padding = 500;
				Uint8* buf = (Uint8*)SDL_malloc(wav_len + padding);
				SDL_memset(buf, 0xFF, wav_len + padding);
				SDL_ReadIO(io, buf, wav_len);
				wav = MIX_LoadAudio_IO(mix, SDL_IOFromMem(buf, wav_len + padding), false, true);
				if (!wav)
					return SDL_APP_FAILURE;
			}
			break;
		}
		break;
	case SDL_EVENT_QUIT:
		return SDL_APP_SUCCESS;
	}
	if (wav) {
		MIX_SetTrackAudio(trk, wav);
		MIX_PlayTrack(trk, 0);
		MIX_DestroyAudio(wav);
		wav = NULL;
	}
	return SDL_APP_CONTINUE;
}

void SDL_AppQuit(void *appstate, SDL_AppResult result) {
	if (result == SDL_APP_FAILURE)
		SDL_ShowSimpleMessageBox(SDL_MESSAGEBOX_ERROR, "Error", SDL_GetError(), win);
}

AntTheAlchemist avatar Aug 14 '25 17:08 AntTheAlchemist

Sure, feel free to share the fix. What was its analysis of the problem?

slouken avatar Aug 14 '25 17:08 slouken

After a lengthy conversation about the issue, this is the summery:

Root cause: The loop termination relies on EOF/zero‑length chunk instead of the proper RIFF container size. In valid WAVE files, the outer RIFF chunk’s size field (read at offset 4) is the authoritative bound for all subchunk parsing.

Why the fix works: By calculating riff_end = riff_start + 8 + riff_size at the start, and iterating until the read pointer reaches riff_end, you stop exactly at the end of that WAVE’s declared data — whether or not the underlying stream continues. This prevents reading into the next file, avoids bogus chunk_length values, and restores correct behaviour for embedded/concatenated WAVs without changing valid single‑file handling.

static bool WAV_init_audio_internal(WAV_AudioData *adata, SDL_IOStream *io, SDL_AudioSpec *spec, SDL_PropertiesID props)
{
    const Sint64 flen = SDL_GetIOSize(io);

    // Remember where this RIFF starts (supports embedded/concatenated WAVs).
    const Sint64 riff_start = SDL_TellIO(io);
    if (riff_start < 0) {
        return false;
    }

    bool found_FMT = false;
    bool found_DATA = false;

    // Read the outer RIFF header
    Uint32 riff_id = 0, riff_size = 0, form_type = 0;
    if (!SDL_ReadU32LE(io, &riff_id) || !SDL_ReadU32LE(io, &riff_size)) {
        return false;
    }

    // Basic sanity: RIFF size must at least include the 4 bytes of form type ("WAVE")
    if (riff_size < 4) {
        return SDL_SetError("Corrupt WAV file (RIFF size too small)");
    }

    if (riff_id == RIFF) {
        if (!SDL_ReadU32LE(io, &form_type)) {
            return false;
        }
    } else {
        // Not supporting RIFX/RF64 here; keep original behavior.
        return SDL_SetError("Not a WAV file");
    }

    if (form_type != WAVE) {
        return SDL_SetError("Not a WAV file");
    }

    // End of this RIFF chunk: start + 8 (id+size) + riff_size
    const Sint64 riff_end = riff_start + 8 + (Sint64) riff_size;

    // If we know the stream length, ensure the RIFF doesn't claim to run past it.
    if ((flen >= 0) && (riff_end > flen)) {
        return SDL_SetError("Corrupt WAV file (RIFF size goes past EOF)");
    }

    // Read subchunks within the RIFF bounds
    while (true) {
        const Sint64 pos = SDL_TellIO(io);
        if (pos < 0) {
            return false;
        }

        // Stop exactly at the end of this RIFF/WAVE
        if (pos >= riff_end) {
            break;
        }

        // If fewer than 8 bytes remain before riff_end, there isn't a complete chunk header; stop.
        if ((riff_end - pos) < 8) {
            // Gracefully stop; anything here would be incomplete junk within RIFF.
            break;
        }

        Uint32 chunk_type = 0, chunk_length = 0;
        if (!SDL_ReadU32LE(io, &chunk_type) || !SDL_ReadU32LE(io, &chunk_length)) {
            return false;
        }

        const Sint64 chunk_data_start = SDL_TellIO(io);
        if (chunk_data_start < 0) {
            return false;
        }

        // Check that the declared chunk (without padding) fits in the RIFF
        if ((chunk_data_start + (Sint64)chunk_length) > riff_end) {
            return SDL_SetError("Corrupt WAV file (chunk goes past end of RIFF)");
        }

        bool chunk_okay = true;
        switch (chunk_type) {
        case FMT:
            found_FMT = true;
            chunk_okay = ParseFMT(adata, io, spec, chunk_length);
            break;
        case DATA:
            found_DATA = true;
            chunk_okay = ParseDATA(adata, io, chunk_length);
            break;
        case SMPL:
            chunk_okay = ParseSMPL(adata, io, chunk_length);
            break;
        case LIST:
            chunk_okay = ParseLIST(adata, io, props, chunk_length);
            break;
        case ID3x:
        case ID3X:
            chunk_okay = ParseWAVID3(io, props, chunk_length);
            break;
        default:
            // Unknown/unsupported chunk: skip
            break;
        }

        if (!chunk_okay) {
            return false;
        }

        // Account for 2-byte alignment (padding byte if odd length)
        Uint32 padded = chunk_length + (chunk_length & 1);

        // Ensure padding doesn't push us past the RIFF end
        const Sint64 next_pos = chunk_data_start + (Sint64)padded;
        if (next_pos > riff_end) {
            return SDL_SetError("Corrupt WAV file (chunk padding goes past end of RIFF)");
        }

        // Move to the start of the next chunk
        if (SDL_SeekIO(io, next_pos, SDL_IO_SEEK_SET) < 0) {
            return false;
        }
    }

    if (!found_FMT) {
        return SDL_SetError("Bad WAV file (no FMT chunk)");
    } else if (!found_DATA) {
        return SDL_SetError("Bad WAV file (no DATA chunk)");
    }

    if (!BuildSeekBlocks(adata)) {
        return false;
    }

    // At this point, the stream is positioned at riff_end; the next read would be
    // the start of a concatenated WAV or physical EOF.
    return true;
}

Would this work? It fixes it my end, but I haven't tested it on multiple WAV formats.

AntTheAlchemist avatar Aug 14 '25 17:08 AntTheAlchemist

I just corrected a bug in my test code, which was causing button 1 from failing. So, a standalone WAV will decode okay, but a WAV with anything after it in the stream will fail due to the decoder requiring an EOF after the WAV in the stream.

AntTheAlchemist avatar Aug 16 '25 16:08 AntTheAlchemist

Another update: after using the "fix" code above, although it successfully decodes, it seems to be very slow - maybe it's not stopping until it reaches the end of the file - I haven't debugged it. So I wouldn't suggest using it as is, but hopefully can be cherry-picked to get the decoder working. As a workaround for now, I'm loading a single WAV into memory, creating a temp IO SDL_IOFromConstMem and then MIX_LoadAudio_IO.

AntTheAlchemist avatar Sep 02 '25 14:09 AntTheAlchemist

I was having problem loading a wav file. The suggested fix worked. Edit: was not working on Windows, Ubuntu was fine. Even with the proposed correction, I had to re-export original wav file from Audacity into a new wav, then it worked.

brccabral avatar Sep 06 '25 10:09 brccabral

Both enemy_hit wav files are working here now, but did not work before 40a580fe85c1092c9719b6588739a5e00517d022 landed yesterday. Update SDL3_mixer and try again?

icculus avatar Sep 07 '25 17:09 icculus

@AntTheAlchemist I haven't forgotten you, I'll dig into your specific issue shortly!

icculus avatar Sep 07 '25 17:09 icculus

Both enemy_hit wav files are working here now, but did not work before 40a580f landed yesterday. Update SDL3_mixer and try again?

Commit 40a580f solves on Ubuntu, but not Windows. Also tested 40a580f+WAV_init_audio_internal , does not work. Only exported_enemy_hit.wav loads.

#include <SDL3/SDL.h>
#include <SDL3_mixer/SDL_mixer.h>

static SDL_Window *window = NULL;
static SDL_Renderer *renderer = NULL;
static MIX_Mixer *mixer = NULL;
static MIX_Track *track1 = NULL;
static MIX_Track *track2 = NULL;
static MIX_Audio *audio1 = NULL;
static MIX_Audio *audio2 = NULL;
static SDL_PropertiesID options;

int main()
{
	SDL_Init(SDL_INIT_VIDEO);
	SDL_CreateWindowAndRenderer("mixer test", 800, 600, 0, &window, &renderer);
	MIX_Init();

	SDL_AudioSpec spec;
	spec.freq = 48000;
	spec.channels = 2;
	spec.format = SDL_AUDIO_F32;
	mixer = MIX_CreateMixerDevice(SDL_AUDIO_DEVICE_DEFAULT_PLAYBACK, &spec);
	if (!mixer)
	{
		SDL_Log("Failed to create mixer.\n%s", SDL_GetError());
		return 1;
	}
	audio1 = MIX_LoadAudio(mixer, "audio/original_enemy_hit.wav", false);
	if (!audio1)
	{
		SDL_Log("Failed to load audio1.\n%s", SDL_GetError());
		return 1;
	}
	track1 = MIX_CreateTrack(mixer);
	if (!track1)
	{
		SDL_Log("Failed to create track1.\n%s", SDL_GetError());
		return 1;
	}
	MIX_SetTrackAudio(track1, audio1);

	audio2 = MIX_LoadAudio(mixer, "audio/exported_enemy_hit.wav", false);
	if (!audio2)
	{
		SDL_Log("Failed to load audio2.\n%s", SDL_GetError());
		return 1;
	}
	track2 = MIX_CreateTrack(mixer);
	if (!track2)
	{
		SDL_Log("Failed to create track2.\n%s", SDL_GetError());
		return 1;
	}
	MIX_SetTrackAudio(track2, audio2);

	options = SDL_CreateProperties();
	SDL_SetNumberProperty(options, MIX_PROP_PLAY_LOOPS_NUMBER, 0);

	bool done = false;
	while (!done) {
		SDL_Event event;
		while (SDL_PollEvent(&event)) {
			if (event.type == SDL_EVENT_QUIT) {
				done = true;
			}
			if (event.type == SDL_EVENT_KEY_DOWN)
			{
				if (event.key.key == SDLK_1)
				{
					SDL_Log("Playing track1");
					if (!MIX_PlayTrack(track1, options))
					{
						SDL_Log("Failed to play track1.\n%s", SDL_GetError());
					}
				}
				if (event.key.key == SDLK_2)
				{
					SDL_Log("Playing track2");
					if (!MIX_PlayTrack(track2, options))
					{
						SDL_Log("Failed to play track2.\n%s", SDL_GetError());
					}
				}
			}
		}
		SDL_SetRenderDrawColor(renderer, 255, 0, 0, 255);
		SDL_RenderClear(renderer);
		SDL_RenderPresent(renderer);
	}

	SDL_DestroyProperties(options);
	MIX_DestroyAudio(audio2);
	MIX_DestroyTrack(track2);
	MIX_DestroyAudio(audio1);
	MIX_DestroyTrack(track1);
	MIX_Quit();
	SDL_DestroyRenderer(renderer);
	SDL_DestroyWindow(window);
	SDL_Quit();
	return 0;
}
Failed to load audio1.
Audio data is in unknown/unsupported/corrupt format
  Error: bad module name; not ST
   Info: UMX: Unknown header tag 0x46464952

brccabral avatar Sep 07 '25 20:09 brccabral

Okay, I'll take a look at this on Windows, too.

icculus avatar Sep 08 '25 19:09 icculus

We believe this is fixed on Windows by https://github.com/libsdl-org/SDL/commit/3876ce3495fec76c66c1bc6e449673bb1a984def (in SDL3, not SDL3_mixer).

icculus avatar Sep 18 '25 13:09 icculus

This "fixes" on Windows, but still fails on Android. Relying on a stream EOF to terminate the decoding is the problem. Not properly fixed because Windows will still decode the entire stream, which means parsing invalid data after the WAV if the stream is greater than the WAV. Essentially, the decoder needs to terminate at the end of the WAV, not the end of the stream.

AntTheAlchemist avatar Sep 19 '25 12:09 AntTheAlchemist