search

mackron / dr_libs

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

dr_mp3: DR_MP3_ONLY_SIMD used, but SSE/NEON not enabled #169

Closed MikuAuahDark closed 3 years ago

MikuAuahDark commented 3 years ago

I'm trying to compile a project that uses dr_mp3 targetting Windows 10 ARM (MSVC compiler) and it errors with message at the title. Looking at the code, it seems MSVC targetting ARM64 is not expected, but to be honest, supporting this is bit annoying as the predefined preprocessor is different than GCC or Clang.

dr_flac also has similar problem, but it compiles fine although from what I see, with reduced performance (fails to detect NEON support). For dr_mp3, I have to add defined(_M_ARM64) at line 677 so it compiles and successfully detects NEON support.

Is it possible to support this, Windows/ARM64 combination? Or is it just too troublesome?

lieff commented 3 years ago

Fixed upstream https://github.com/lieff/minimp3/commit/0d0c452dc76f744ceea4782de17bb735163cdee4 Thanks for catching this)

mackron commented 3 years ago

Thanks @lieff, just saw that in my feed. Will update dr_mp3 soon.

mackron commented 3 years ago

I've updated dr_mp3. Concerning dr_flac, I was surprised to see that I'm actually already checking _M_ARM64:

#if !defined(DRFLAC_NO_NEON) && (defined(__ARM_NEON) || defined(__aarch64__) || defined(_M_ARM64))
    #define DRFLAC_SUPPORT_NEON
#endif

What I did change, however, was the detection of the DRFLAC_ARM define. I was checking _M_ARM, but not _M_ARM64. I wonder if maybe that was the cause of it? In any case, I've pushed a small change to dr_flac to the dev branch - are you able to give that quick try and see how it goes? How did you determine that dr_flac fails to detect NEON support by the way?

Commit: https://github.com/mackron/dr_libs/commit/5c6b5b1638c625d82ac350ce4c1e7581cc10f292

MikuAuahDark commented 3 years ago

How did you determine that dr_flac fails to detect NEON support by the way?

Visual Studio IDE desaturate/dehighlight lines which won't become part of the compilation caused by preprocessor selection. In this case, drflac__has_neon() returns false because it has preprocessor check of DRFLAC_SUPPORT_NEON which is not defined. Looking back, DRFLAC_SUPPORT_NEON depends on DRFLAC_ARM being defined which, when targetting MSVC ARM64, isn't defined.

Testing that commit now shows DRFLAC_SUPPORT_NEON being defined and result in code that returns true in drflac__has_neon() no longer desaturated.

mackron commented 3 years ago

Great! Thanks for checking that for me. I've gone ahead and released that fix to the master branch. Thanks for reporting this.