Re: [hatari-devel] DSP bug found (and maybe solved) |
[ Thread Index | Date Index | More lists.tuxfamily.org/hatari-devel Archives ]
I've tested many programs tonight and didn't noticed any regression. The fix seems OK to me.Great. I love Audio Fun Machine, and I'm happy to have it working correctly now.
And also, thank you Eero for the moongame button C tips to launch the game. Regards Laurent Le 15/04/2024 à 22:10, Laurent Sallafranque a écrit :
Hi, Thanks Andreas. I prefer this 2nd patch.I've commited it and changed the compatibiliy file too (but it didn't change the main hatari site compatibility list).I'll do more tests tonight with programs I know that use modulo buffers. Regards Laurent Le 15/04/2024 à 21:58, Andreas Grabher a écrit :Am 15.04.2024 um 11:50 schrieb Andreas Grabher <andreas_g86@xxxxxxxxxx>:I was wrong with my first patch and I think the solution from Laurent is also wrong, because it does not correctly handle the case where M equals |Nn| but is not equal to 2^k.Am 14.04.2024 um 23:48 schrieb Laurent Sallafranque <laurent.sallafranque@xxxxxxx>:Great discovery! Obviously the special case where abs_modifier equals modulo was not handled. I think the more logical solution would beHi all,I've spent the last 2 weeks tracing Audio Fun Machine to find the bug when the equalizer is ON (sound becomes noisy).I've isolated the bug : it's about Rn + Nn + Mn updating. An example : move #>0,r0 ; R0 = 0 movec #$01,m0 ; modulo 2 move #$02,n0 ; incrementor = 2 move #>1,a move #>1,b move a,x:(r0)+ ; r0 = 1 nop nopmove b,x:(r0)+ ; r0 = 0 (because of the modulo)nop nop move (r0)+n0 ; R0 = 0 (should be 2) nop nop move a,x:(r0)+ ; R0 = 1 (should be 3) I suggest the following fix in the function : static void dsp_update_rn_modulo(uint32_t numreg, int16_t modifier) Line 1694 : replace if (abs_modifier>modulo) { by if (abs_modifier>modulo-1) { In line 1673, modulo is added +1 for buffer boundaries computing. But I think modulo should be -1 when tested with modifier.At least, Audio Fun Machine sound becomes good with this patch, but DSP programs should be tested for non regression.I've tested 2 or 3 of them, but not all of them. @eero, could you test my patch and tell me if AFM works well for you ? Regards Laurentif (abs_modifier>=modulo)I created a patch (appended). I think this is the cleanest way: It first checks for the case where |Nn| is not a multiple of 2^k. It then checks if Nn is greater than M which means unpredictable result else it does normal modulo operation. In the special case where |Nn| is a multiple of 2^k it jumps to the next buffer (this includes the case where P is 0 and makes this case a bit faster).Special case is described in the data sheet of the 56001 DSP:"If an offset, Nn, is used in the address calculations, the 16-bit absolute value, INnl, must be less than or equal to M for proper modulo addressing. If Nn>M, the result is data dependent and unpredictable, except for the special case where Nn = P x 2^k, a multiple of the block size where P is a positive integer. For this special case, when using the (Rn) + Nn addressing mode, the pointer, Rn, will jump linearly to the same relative address in a new buffer, which is P blocks forward in memory (see Figure 5-12). Similarly, for (Rn) - Nn, the pointer will jump P blocks backward in memory.“I think the data sheet is not very clear because if |Nn| is equal to M there are two possible cases but only one is described.Please verify my thoughts. I hope there is no mistake.
Mail converted by MHonArc 2.6.19+ | http://listengine.tuxfamily.org/ |