Search code examples
c++sdl-2sdl-mixer

SDL_mixer function Mix_LoadMUS_RW causes Access Violation


I have a problem in loading music from memory with SDL_mixer. The following "minimal" example including a bit of error checking will always crash with an access violation in Music::play.

#include <SDL\SDL_mixer.h>
#include <SDL\SDL.h>
#include <vector>
#include <iostream>
#include <string>
#include <fstream>

class Music {
public:
    void play(int loops = 1);
    SDL_RWops* m_rw;
    std::vector<unsigned char> m_file;
    Mix_Music * m_music = nullptr;
};

void Music::play(int loops) {
    if (Mix_PlayMusic(m_music, loops) == -1)
        std::cout << "Error playing music " + std::string(Mix_GetError()) + " ...\n";
}

void readFileToBuffer(std::vector<unsigned char>& buffer, std::string filePath) {
    std::ifstream file(filePath, std::ios::binary);

    file.seekg(0, std::ios::end);
    int fileSize = file.tellg();
    file.seekg(0, std::ios::beg);
    fileSize -= file.tellg();

    buffer.resize(fileSize);
    file.read((char *)&(buffer[0]), fileSize);

    file.close();
}

void writeFileToBuffer(std::vector<unsigned char>& buffer, std::string filePath) {
    std::ofstream file(filePath, std::ios::out | std::ios::binary);
    for (size_t i = 0; i < buffer.size(); i++)
        file << buffer[i];
    file.close();
}

Music loadMusic(std::string filePath) {
    Music music;

    readFileToBuffer(music.m_file, filePath);
    music.m_rw = SDL_RWFromMem(&music.m_file[0], music.m_file.size());

    // Uncommenting the next block runs without problems
    /*
    writeFileToBuffer(music.m_file, filePath);
    music.m_rw = SDL_RWFromFile(filePath.c_str(), "r");
    */

    if (music.m_rw == nullptr)
        std::cout << "Error creating RW " + std::string(Mix_GetError()) + " ...\n";

    music.m_music = Mix_LoadMUSType_RW(music.m_rw, Mix_MusicType::MUS_OGG, SDL_FALSE);

    if (music.m_music == nullptr)
        std::cout << "Error creating music " + std::string(Mix_GetError()) + " ...\n";

    return music;
}

int main(int argc, char** argv) {
    SDL_Init(SDL_INIT_AUDIO);
    Mix_Init(MIX_INIT_MP3 | MIX_INIT_OGG);
    Mix_OpenAudio(MIX_DEFAULT_FREQUENCY, MIX_DEFAULT_FORMAT, MIX_DEFAULT_CHANNELS, 1024);

    Music music = loadMusic("Sound/music/XYZ.ogg");

    music.play();

    std::cin.ignore();

    return 0;
}

My ArchiveManager works for sure, which can also be seen because ucommenting the block that writes the buffer to a file and creating an SDL_RW from this will run just fine. The music file I load is just assumed to be an ogg file, which it is in this case, hence creating an SDL_RW from the file works fine. Meaning nothing crashes and the music plays properly start to end.

The music class is from my understanding much too big. I am just keeping the buffer m_file around, as well as the SDL_RW to make sure that the problem does not come from that data being freed. Running Mix_LoadMUS_RW with SDL_FALSE should also make sure that the RW is not freed.

Notably a similar example loading a wav file from the same archive using Mix_LoadWAV_RW works just fine:

Mix_Chunk * chunk;
std::vector<unsigned char> fileBuf = ArchiveManager::loadFileFromArchive(filePath);
chunk = Mix_LoadWAV_RW(SDL_RWFromConstMem(&fileBuf[0], fileBuf.size()), SDL_TRUE);

And here I am not even keeping the buffer around until calling the Mix_PlayCannel. Also here I am calling the load function with SDL_TRUE because I am not creating an explicit SDL_RW. Trying the similar thing for loading the music will not make a difference.

I studied the SDL_mixer source code, but it didn't help me. Maybe my knowledge is not sufficient or maybe I missed something crucial.

To get to the point: Where does that access violation come from and how can I prevent it?

EDIT: Changed the example code so it is straightforward for anyone to reproduce it. So no ArchiveManager or anything like that, just reading an ogg directly into memory. The crucial parts are just the few lines in loadMusic.


Solution

  • Music music = loadMusic("Sound/music/XYZ.ogg");
    music.play();
    

    The first line will copy the object of type class Music on the right into the new one called music. This will result in the vector m_file being copied, including the data in it. The data for the vector of our new object music will obviously be stored at a different memory location than that of the vector of the object returned by loadMusic. Then the object returned by loadMusic will be deleted from the stack and the data of it's vector will be freed thus invalidating the previously created Mix_Music object and causing an access violation on the second line.

    This can be remedied by only ever creating one Music object, for example by creating it via new on the heap and having loadMusic return a pointer to that object.

    Music* music = loadMusic("Sound/music/XYZ.ogg");
    music->play();
    

    It might anyway be the better choice to allocate the memory for a whole file on the heap instead of on the stack, though I would guess that vectors do this internally.

    So short version, it was (what I consider) an rookie mistake and I was too fixated on blaming SDL_Mixer. Bad idea.