Fix audiomixer.Mixer regressions on SAMD51

[relic-se]

Updates made within #10529 may have caused audio corruption on the SAMD51 platform as reported by #10867. This update adds ARM-specific operations to copy16lsb and copy16msb to attempt to fix this regression.

This update has not yet been tested on the target hardware. Any assistance from @todbot or @gamblor21 would be greatly appreciated.

Fixes #10867

[gamblor21]

Thinking out loud a bit here. I don't see anything wrong in your change, but I'm not sure the distortion would be fixed by just a speed up of what amounts to a basic bit copy (but it may).

I think there is an issue in the SAMD51 ASM code, with mult16signed. The old code would take a 32bit value HHHHLLLL and multiply both halves by loudness returning a 32-bit result.

The new code gets HHHHHHHH and then LLLLLLLL as two values (because of the copy16lsb and msb). So the result is HHHHloudness and HHHHloudness as a 32-bit value. and then the same for the LSB. So now we get HHHHHHHH and then LLLLLLLL.

I'm fighting a cold so maybe I missed something but I think the C code works but the ASM has something off in it.

That said I haven't had a chance to test your change on h/w and maybe my brain is making things up at the moment :)

[relic-se]

Thinking out loud a bit here. I don't see anything wrong in your change, but I'm not sure the distortion would be fixed by just a speed up of what amounts to a basic bit copy (but it may).

Okay, that makes sense. I was unsure if there might be some issues with bitwise C operations on that platform that I wasn't aware of. I was able to test the pkhbt and pkhtb operands in an emulator with the desired result, so it probably won't hurt to keep that in.

I think there is an issue in the SAMD51 ASM code, with mult16signed. The old code would take a 32bit value HHHHLLLL and multiply both halves by loudness returning a 32-bit result.

The new code gets HHHHHHHH and then LLLLLLLL as two values (because of the copy16lsb and msb). So the result is HHHHloudness and HHHHloudness as a 32-bit value. and then the same for the LSB. So now we get HHHHHHHH and then LLLLLLLL.

I think you're on to something. I had forgotten that I modified mult16signed in c92efd6.

I'm fighting a cold so maybe I missed something but I think the C code works but the ASM has something off in it.

That said I haven't had a chance to test your change on h/w and maybe my brain is making things up at the moment :)

Sorry to hear that you're under the weather. 😿 As I said before, I did run the operation through an emulator, but I'm definitely no expert in ARM assembly. I still don't have hardware to test this, so whenever you get the chance, that would be awesome. Until then, I'll look into mult16signed.

[gamblor21]

Given what the sound is described as, it sounds like clipping in fixed-point math. I may have a potential reason.

int16_t panning = synthio_block_slot_get_scaled(&voice->panning, -ALMOST_ONE, ALMOST_ONE);

If panning is 1.0 then this returns 32768, which overflows.

So what isn't ideal is the SAMD51 code keeps with fixed point math, while the other code does floating point. I have a feeling that converting back to a float is potentially hiding the problem, while the SAMD code is working correctly and showing the error that exists elsewhere.

The only thing I can see is the multiplication of panning and loudness that could cause this issue.

Out of time to step through it more but I think the root cause is somewhat located in that vs the mul16signed code itself.