mackron / dr_libs

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

Compilation failure in MSVC ARM64 builds #229

Closed ccawley2011 closed 2 years ago

ccawley2011 commented 2 years ago

Encountered when attempting to build SDL2_sound:

D:\a\SDL_sound\SDL_sound\src\dr_mp3.h(2106,106): error C2078: too many initializers [D:\a\SDL_sound\SDL_sound\build\SDL2_sound.vcxproj]
D:\a\SDL_sound\SDL_sound\src\dr_mp3.h(2106,51): warning C4244: 'initializing': conversion from 'float' to 'unsigned __int64', possible loss of data [D:\a\SDL_sound\SDL_sound\build\SDL2_sound.vcxproj]
D:\a\SDL_sound\SDL_sound\src\dr_mp3.h(2106,66): warning C4244: 'initializing': conversion from 'float' to 'unsigned __int64', possible loss of data [D:\a\SDL_sound\SDL_sound\build\SDL2_sound.vcxproj]
D:\a\SDL_sound\SDL_sound\src\dr_mp3.h(2106,81): error C2078: too many initializers [D:\a\SDL_sound\SDL_sound\build\SDL2_sound.vcxproj]
sezero commented 2 years ago

Maybe changing like this would help? (SDL's SDL_Convert51ToStereo_NEON in SDL_audiocvt.c does something similar)

            static const float fval = 1.0f/32768.0f;
            static const drmp3_f4 g_scale = vdupq_n_f32(fval);
mackron commented 2 years ago

This is from minimp3 which I don't maintain (dr_mp3 is a wrapper around minimp3). If this is fixed here it should probably also be fixed upstream in minimp3 as well. CC @lieff

@sezero Have you plugged that code into dr_mp3 and verified that it works? I don't use the ARM targets with MSVC so I'd be depending on the community to verify it. If it's working for you I'd be happy to make the change (just looking at it, it looks fine though).

sezero commented 2 years ago

@sezero Have you plugged that code into dr_mp3 and verified that it works?

Doing do following change works (build-tested only, have nothing to run-test with):

--- a/dr_mp3.h
+++ b/dr_mp3.h
@@ -2123 +2123 @@ static void drmp3d_synth(float *xl, drmp3d_sample_t *dstl, int nch, float *lins)
-            static const drmp3_f4 g_scale = { 1.0f/32768.0f, 1.0f/32768.0f, 1.0f/32768.0f, 1.0f/32768.0f };
+            const drmp3_f4 g_scale = vdupq_n_f32(1.0f/32768.0f);

Side note: MSVC has arm_neon.h and arm64_neon.h for _M_ARM and _M_ARM64: Maybe worth adjusting (see e.g. SDL's SDL_cpuinfo.h)

mackron commented 2 years ago

OK, I'll make that change soon and just let the community report any issues, if any. I fully expect it'll be fine.

sezero commented 2 years ago

We ended up with the following patch in SDL_sound (the one above alone bombs with SSE2 builds.)

diff --git a/dr_mp3.h b/dr_mp3.h
index 2cf63f2..1803ecf 100644
--- a/dr_mp3.h
+++ b/dr_mp3.h
@@ -2103,7 +2103,11 @@ static void drmp3d_synth(float *xl, drmp3d_sample_t *dstl, int nch, float *lins)
             vst1_lane_s16(dstl + (49 + i)*nch, pcmb, 2);
 #endif
 #else
+#if DRMP3_HAVE_SSE
             static const drmp3_f4 g_scale = { 1.0f/32768.0f, 1.0f/32768.0f, 1.0f/32768.0f, 1.0f/32768.0f };
+#else /* NEON: */
+            const drmp3_f4 g_scale = vdupq_n_f32(1.0f/32768.0f);
+#endif
             a = DRMP3_VMUL(a, g_scale);
             b = DRMP3_VMUL(b, g_scale);
 #if DRMP3_HAVE_SSE
mackron commented 2 years ago

Thanks. I've applied this to the dev branch and will get it released soon.

mackron commented 2 years ago

This has been released.