mackron / dr_libs

Audio decoding libraries for C/C++, each in a single source file.
Other
1.28k stars 205 forks source link

dr_flac: crashing on Nintendo 3DS #239

Closed TurtleP closed 2 years ago

TurtleP commented 2 years ago

Heya,

I'm writing a homebrew application for Nintendo 3DS, specifically lovepotion. I would like to replace using the flac library I currently use with dr_flac for its simplicity. However, dr_flac simply crashes whenever I call drflac_open. This has been observed by others as well. The last known working version is 0.8b, although I would prefer to be able to use the more up-to-date version of the library.

I think this issue also happens with dr_mp3, but I can't entirely recall. dr_wav I could also test, if you would like me to.

Using homebrew-provided gdb, I was able to get some kind of backtrace, at least. Hopefully it serves some help to you:

Thread 1 received signal SIGSEGV, Segmentation fault.
0x00121b18 in drflac_open_with_metadata_private (
    onRead=onRead@entry=0x11a168 <read(void*, void*, size_t)>,
    onSeek=onSeek@entry=0x11a188 <seek(void*, int, drflac_seek_origin)>,
    onMeta=onMeta@entry=0x0,
    container=container@entry=drflac_container_unknown, pUserData=0x0,
    pUserDataMD=0x801b58c, pAllocationCallbacks=0x8007a00,
    pAllocationCallbacks@entry=0x0)
    at G:/GitHub/Homebrew/lovepotion/libraries/dr/dr_flac.h:7826
7826    {
(gdb) bt
#0  0x00121b18 in drflac_open_with_metadata_private (
    onRead=onRead@entry=0x11a168 <read(void*, void*, size_t)>,
    onSeek=onSeek@entry=0x11a188 <seek(void*, int, drflac_seek_origin)>,
    onMeta=onMeta@entry=0x0,
    container=container@entry=drflac_container_unknown, pUserData=0x0,
    pUserDataMD=0x801b58c, pAllocationCallbacks=0x8007a00,
    pAllocationCallbacks@entry=0x0)
    at G:/GitHub/Homebrew/lovepotion/libraries/dr/dr_flac.h:7826
#1  0x00123894 in drflac_open (pAllocationCallbacks=0x0, pUserData=0x8032c38,
    onSeek=0x11a188 <seek(void*, int, drflac_seek_origin)>,
    onRead=0x11a168 <read(void*, void*, size_t)>)
    at G:/GitHub/Homebrew/lovepotion/libraries/dr/dr_flac.h:8787
#2  love::FLACDecoder::FLACDecoder (this=0x8032a48, stream=0x8032c38,
    bufferSize=<optimized out>)
    at G:/GitHub/Homebrew/lovepotion/source/utilities/decoder/types/flacdecoder.cpp:42
#3  0x0011069c in DecoderImplFor<love::FLACDecoder>()::{lambda(love::Stream*, int)#1}::_FUN(love::Stream*, int) ()
    at G:/GitHub/Homebrew/lovepotion/source/modules/sound/sound.cpp:26
#4  0x00110a14 in love::Sound::NewDecoder (this=<optimized out>,
    stream=stream@entry=0x8032c38, bufferSize=bufferSize@entry=16384)
    at G:/GitHub/Homebrew/lovepotion/source/modules/sound/sound.cpp:57
#5  0x00110f50 in operator() (__closure=<optimized out>)

If you need any further details, please let me know!

EDIT: tried dr_mp3, it also crashes. Will try dr_wav as well later today or tomorrow.

mackron commented 2 years ago

Hmm, interesting. Definitely not crashing here, and not had any similar reports. Very suspicious that it's also happening in dr_mp3. I can't really think of anything that might be causing this. This might be a long shot, but all of my libraries tend to use a lot of the stack to avoid heap allocations - is it possible that you're running out of stack space?

TurtleP commented 2 years ago

That could very much be possible. The 3DS does not have a lot of stack space to begin with. Although the data I am already passing in is read by other classes, if the libraries use lots of stack allocation, it would be an issue.

mackron commented 2 years ago

Yeah, so for example, in drflac_open_with_metadata_private() there is a stack allocation of an object called oggbs of the type drflac_oggbs. That's a 64K-ish struct (it has a buffer for storing raw page data from the Ogg file).

(If you don't need support for Ogg encapsulation you can use DR_FLAC_NO_OGG which might relieve a bit of pressure there.)

TurtleP commented 2 years ago

I could, yeah, though hopefully there's some stuff you could do for the library to not do so much stack allocation as well. I've had no issues using mpg123 (which I was using before trying dr_mp3) or this other FLAC decoder library, likely due to less stack allocations and using heap instead.

I'll disable the Ogg stuff and see how that goes. Not sure what to do about dr_mp3, though.

mackron commented 2 years ago

Do you by chance have a backtrace of the dr_mp3 crash?

TurtleP commented 2 years ago

I don't, but I could try it. Give me a few minutes and I'll report back.

TurtleP commented 2 years ago

Okay, so dr_mp3 does not crash. It's just that because FLAC was the first decoder the application tried to decode, it would crash since it did allocate that 64K struct. Disabling Ogg in dr_flac allows it to get dr_mp3 to load accordingly. I haven't tested how much memory either library take up on the stack right now, but hopefully not a lot, as you did mention that at least dr_flac does a lot of stack allocation.

mackron commented 2 years ago

I was just looking at dr_mp3, and it will allocate 16K on the stack when decoding which is specifically for data conversion. Which reading function do you use in dr_mp3 - s16 or f32? If you're using f32, use DR_MP3_FLOAT_OUTPUT to avoid that data conversion and therefore that stack allocation. If using s16, leave that option out. With that option set appropriately, dr_mp3 doesn't do anything unusual regarding stack allocations.

For dr_flac, that oggbs thing is the biggest one, and it has indeed come up in the past which reminded me about the whole stack space thing. And just having another look now, I can't see anything else remarkable regarding stack allocations.

I'll leave this issue open to remind me to assess that oggbs thing. Maybe I should just allocate it on the heap instead and just put up with it, or maybe even have an option for it (though that feels very specific).

TurtleP commented 2 years ago

dr_mp3 is reading with s16, so should be fine by what you said about the data conversion.

mackron commented 2 years ago

FYI, I've gone ahead and made a change to allocate that buffer onto the heap rather than the stack. It's a decent compromise - it'll only happen for Ogg encapsulated files which are pretty rare so the inefficiency of doing the heap allocation should have minimal impact in practice.

I'll go ahead and close this one now. Thanks for reporting and investigating this one.