[SDL3] testautomation failure on arm64: unexpected differences in audio conversion

[smcv]

When building 3abb464 on Debian arm64 (also known as aarch64), I'm seeing test failures since we started running testautomation. I wonder whether this indicates a problem with a NEON-optimized code path?

(Note that we don't enable NEON on 32-bit ARM in Debian, because our "baseline" CPU that is the minimum for the 32-bit ARM port doesn't have it; so arm64 is the only build with NEON enabled.)

https://buildd.debian.org/status/fetch.php?pkg=libsdl3&arch=arm64&ver=3%7Egit20231002%7E3abb464%2Bds-1&stamp=1696268040&raw=0

INFO:  10/02/23 17:32:41: ----- Test Case 1.17: 'audio_convertAccuracy' started
INFO:  10/02/23 17:32:41: Test Description: 'Check accuracy converting between audio formats.'
INFO:  10/02/23 17:32:41: Test Iteration 1: execKey 18358328168491283429
INFO:  10/02/23 17:32:41: Assert 'Call to SDL_InitSubSystem(SDL_INIT_AUDIO)': Passed
INFO:  10/02/23 17:32:41: Assert 'Check result from SDL_InitSubSystem(SDL_INIT_AUDIO)': Passed
INFO:  10/02/23 17:32:41: Assert 'Expected source buffer to be created.': Passed
INFO:  10/02/23 17:32:41: Assert 'Expected SDL_ConvertAudioSamples(F32->S8) to succeed': Passed
INFO:  10/02/23 17:32:41: Assert 'Expected SDL_ConvertAudioSamples(S8->F32) to succeed': Passed
INFO:  10/02/23 17:32:41: Conversion via S8 took 0.001348 seconds.
ERROR: 10/02/23 17:32:41: Assert 'S8 has min delta of -0.015625, should be >= -0.007812': Failed
ERROR: 10/02/23 17:32:41: Assert 'S8 has max delta of +0.015625, should be <= +0.007812': Failed
INFO:  10/02/23 17:32:41: Assert 'Expected SDL_ConvertAudioSamples(F32->U8) to succeed': Passed
INFO:  10/02/23 17:32:41: Assert 'Expected SDL_ConvertAudioSamples(U8->F32) to succeed': Passed
INFO:  10/02/23 17:32:41: Conversion via U8 took 0.000835 seconds.
INFO:  10/02/23 17:32:41: Assert 'U8 has min delta of +0.000000, should be >= -0.007812': Passed
ERROR: 10/02/23 17:32:41: Assert 'U8 has max delta of +0.023437, should be <= +0.007812': Failed
INFO:  10/02/23 17:32:41: Assert 'Expected SDL_ConvertAudioSamples(F32->S16) to succeed': Passed
INFO:  10/02/23 17:32:41: Assert 'Expected SDL_ConvertAudioSamples(S16->F32) to succeed': Passed
INFO:  10/02/23 17:32:41: Conversion via S16 took 0.000929 seconds.
ERROR: 10/02/23 17:32:41: Assert 'S16 has min delta of -0.000061, should be >= -0.000031': Failed
ERROR: 10/02/23 17:32:41: Assert 'S16 has max delta of +0.000061, should be <= +0.000031': Failed
INFO:  10/02/23 17:32:41: Assert 'Expected SDL_ConvertAudioSamples(F32->S32) to succeed': Passed
INFO:  10/02/23 17:32:41: Assert 'Expected SDL_ConvertAudioSamples(S32->F32) to succeed': Passed
INFO:  10/02/23 17:32:41: Conversion via S32 took 0.001178 seconds.
ERROR: 10/02/23 17:32:41: Assert 'S32 has min delta of -0.000000, should be >= -0.000000': Failed
ERROR: 10/02/23 17:32:41: Assert 'S32 has max delta of +0.000000, should be <= +0.000000': Failed
INFO:  10/02/23 17:32:41: Assert 'Cleanup of test files completed': Passed
INFO:  10/02/23 17:32:41: Fuzzer invocations: 267888
ERROR: 10/02/23 17:32:41: Assert Summary: Total=20 Passed=13 Failed=7
INFO:  10/02/23 17:32:41: Total Test runtime: 0.0 sec
ERROR: 10/02/23 17:32:41: >>> Test 'audio_convertAccuracy': Failed

and, perhaps relatedly:

INFO:  10/02/23 17:32:39: ----- Test Case 1.10: 'audio_convertAudio' started
INFO:  10/02/23 17:32:39: Test Description: 'Convert audio using available formats.'
...
INFO:  10/02/23 17:32:39: Creating dummy sample buffer of 64 length (128 bytes)
INFO:  10/02/23 17:32:39: Assert 'Check src data buffer to convert is not NULL': Passed
INFO:  10/02/23 17:32:39: Assert 'Check dst data buffer to convert is not NULL': Passed
INFO:  10/02/23 17:32:39: Assert 'Verify available (pre-put); expected: 0; got: 0': Passed
INFO:  10/02/23 17:32:39: Assert 'Verify available (post-put); expected: 128; got: 128': Passed
INFO:  10/02/23 17:32:39: Assert 'Verify result value; expected: 128; got: 128': Passed
INFO:  10/02/23 17:32:39: Assert 'Verify available (post-get); expected: 0; got: 0': Passed
ERROR: 10/02/23 17:32:39: Output buffer is not silent
INFO:  10/02/23 17:32:39: Assert 'Cleanup of test files completed': Passed
INFO:  10/02/23 17:32:39: Fuzzer invocations: 7
ERROR: 10/02/23 17:32:39: >>> Test 'audio_convertAudio': Failed (Aborted)
INFO:  10/02/23 17:32:39: Total Test runtime: 0.0 sec
ERROR: 10/02/23 17:32:39: >>> Test 'audio_convertAudio': Failed

[madebr]

I get identical results when running testautomation on an Android aarch64 device.

[smcv]

I get identical results when running testautomation on an Android aarch64 device.

Sorry, do you mean the test passes with results identical to what it expects, or do you mean the test fails in a way that is identical to what I reported? :-)

[madebr]

I meant to say it fails with an identical error message :). It has the same error deltas. I thought this was useful to say since more people have an aarch64 phone then a aarch64 linux desktop.

[icculus]

Can you comment out this thing in SDL_ChooseAudioConverters in src/audio/SDL_audiotypecvt.c and see if the tests pass?

#ifdef SDL_NEON_INTRINSICS
    if (SDL_HasNEON()) {
        SET_CONVERTER_FUNCS(NEON);
        return;
    }
#endif

[0x1F9F1]

Yeah when I was updating the converters I mostly left the neon stuff untouched since I don't really have a way of testing them. I'm currently a bit busy but I should be to able port the SSE2 versions fairly easily.

[smcv]

Can you comment out this thing in SDL_ChooseAudioConverters in src/audio/SDL_audiotypecvt.c and see if the tests pass?

I don't have actual arm64 hardware ready for use, but I can try doing this on a remote-access Debian machine. (There is some lead time on this.)

[smcv]

8379 works for me, with:

cmake -S . -B _build -DSDL_TESTS=ON -DSDL_TESTS_TIMEOUT_MULTIPLIER=20
make -C _build
SDL_VIDEO_DRIVER=dummy SDL_AUDIO_DRIVER=dummy make -C _build test

The simpler change from if (SDL_HasNEON()) to if (SDL_HasNEON() && SDL_FALSE) doesn't work for me because it suppresses generation of a scalar fallback, leading to an assertion failure.

When someone fixes the NEON code path, #8379 will need reverting as part of that change.

[icculus]

I'll take a run at updating the NEON code today. If I get into trouble, I'll defer to @0x1F9F1.

[0x1F9F1]

I'll take a run at updating the NEON code today. If I get into trouble, I'll defer to @0x1F9F1.

FWIW I did have a quick attempt at some of int->float ones earlier, though they are completely untested apart from compiling on godbolt https://gist.github.com/0x1F9F1/9d44ebfaa31f7271d0ef11cc7f09d304 I also then realised NEON has vcvtq_n which seems like it directly handles all the fixed point shifting and saturation, so that might be more efficient anyway?