From da46df4d7cbd6372b105291ec61c7f45733e518c Mon Sep 17 00:00:00 2001 From: "Jake.Stine" Date: Tue, 27 Oct 2009 05:50:50 +0000 Subject: [PATCH] SPU2-X: More reverb work (mostly). * Fixed some bugs from the prev rev that ended up disabling all reverb completely. * Expanded reverb buffers to x4, as per Neill Corlett's recommendation; this should give reverb a lot more "body" * Implemented what I suspect is correct behavior for EEA register handling. When set to zero, effects are disabled. This fixes nasty reverb and feedback loops in Digital Devil Saga. * Reverb down/up mix buffers are now processed even when effects disabled; fixes the reverb being cut off prematurely during the BIOS splash screen. * Fixed some silly clamping bugs (should fix distortion problems in some titles) git-svn-id: http://pcsx2.googlecode.com/svn/trunk@2086 96395faa-99c1-11dd-bbfe-3dabce05a288 --- plugins/spu2-x/src/Mixer.cpp | 79 +++++++++++++++------------------- plugins/spu2-x/src/Reverb.cpp | 54 +++++++++++++++-------- plugins/spu2-x/src/defs.h | 4 ++ plugins/spu2-x/src/spu2sys.cpp | 41 ++++++++++++++---- 4 files changed, 106 insertions(+), 72 deletions(-) diff --git a/plugins/spu2-x/src/Mixer.cpp b/plugins/spu2-x/src/Mixer.cpp index 07170430da..0a48570e10 100644 --- a/plugins/spu2-x/src/Mixer.cpp +++ b/plugins/spu2-x/src/Mixer.cpp @@ -583,51 +583,43 @@ StereoOut32 V_Core::Mix( const VoiceMixSet& inVoices, const StereoOut32& Input, TD.Left += Ext.Left & DryGate.ExtL; TD.Right += Ext.Right & DryGate.ExtR; - if( !EffectsDisabled ) + if( EffectsDisabled ) return TD; + + // ---------------------------------------------------------------------------- + // Reverberation Effects Processing + // ---------------------------------------------------------------------------- + // The FxEnable bit is, like many other things in the SPU2, only a partial systems + // toogle. It disables the *inputs* to the reverb, such that the reverb is fed silence, + // but it does not actually disable reverb effects processing. In practical terms + // this means that when a game turns off reverb, an existing reverb effect should trail + // off naturally, instead of being chopped off dead silent. + + Reverb_AdvanceBuffer(); + + StereoOut32 TW; + + if( FxEnable ) { - //Reverb pointer advances regardless of the FxEnable bit... - Reverb_AdvanceBuffer(); + // Mix Input, Voice, and External data: - if( FxEnable ) - { - // Mix Input, Voice, and External data: - StereoOut32 TW( - Input.Left & WetGate.InpL, - Input.Right & WetGate.InpR - ); + TW.Left = Input.Left & WetGate.InpL; + TW.Right = Input.Right & WetGate.InpR; - TW.Left += Voices.Wet.Left & WetGate.SndL; - TW.Right += Voices.Wet.Right & WetGate.SndR; - TW.Left += Ext.Left & WetGate.ExtL; - TW.Right += Ext.Right & WetGate.ExtR; - - WaveDump::WriteCore( Index, CoreSrc_PreReverb, TW ); - - // Like all over volumes on SPU2, reverb coefficients and stuff are signed, - // range -50% to 50%, thus *2 is typically needed. The question is: boost the - // volume before going into the reverb unit, or after coming out? - - StereoOut32 RV( DoReverb( TW ) ); - - // (to do pre-reverb boost, change TW above to TW*2 and remove the *2 below on RW. - // This should give a sligntly deeper reverb effect, but may cause distortion on - // some games.) - - // (fixme: this may not be true anymore with the new reverb system, needs testing) - - RV *= 2; - WaveDump::WriteCore( Index, CoreSrc_PostReverb, RV ); - - // Mix Dry+Wet - return TD + ApplyVolume( RV, FxVol ); - } - else - { - WaveDump::WriteCore( Index, CoreSrc_PreReverb, 0, 0 ); - WaveDump::WriteCore( Index, CoreSrc_PostReverb, 0, 0 ); - } + TW.Left += Voices.Wet.Left & WetGate.SndL; + TW.Right += Voices.Wet.Right & WetGate.SndR; + TW.Left += Ext.Left & WetGate.ExtL; + TW.Right += Ext.Right & WetGate.ExtR; } - return TD; + + WaveDump::WriteCore( Index, CoreSrc_PreReverb, TW ); + + StereoOut32 RV( DoReverb( TW ) ); + + WaveDump::WriteCore( Index, CoreSrc_PostReverb, RV ); + + // Mix Dry + Wet + // (master volume is applied later to the result of both outputs added together). + return TD + ApplyVolume( RV, FxVol ); } // used to throttle the output rate of cache stat reports @@ -659,8 +651,7 @@ __forceinline void Mix() Ext = StereoOut32::Empty; else { - Ext = ApplyVolume( Ext, Cores[0].MasterVol ); - clamp_mix( Ext ); + Ext = clamp_mix( ApplyVolume( Ext, Cores[0].MasterVol ) ); } // Commit Core 0 output to ram before mixing Core 1: @@ -690,7 +681,7 @@ __forceinline void Mix() // I suspect this approach (clamping at the higher volume) is more true to the // PS2's real implementation. - clamp_mix( Out, SndOutVolumeShift ); + Out = clamp_mix( Out, SndOutVolumeShift ); } // Update spdif (called each sample) diff --git a/plugins/spu2-x/src/Reverb.cpp b/plugins/spu2-x/src/Reverb.cpp index 500bd7a395..1390682939 100644 --- a/plugins/spu2-x/src/Reverb.cpp +++ b/plugins/spu2-x/src/Reverb.cpp @@ -26,7 +26,7 @@ __forceinline s32 V_Core::RevbGetIndexer( s32 offset ) { - u32 pos = ReverbX + offset; + u32 pos = ReverbX + offset; //*4); // Need to use modulus here, because games can and will drop the buffer size // without notice, and it leads to offsets several times past the end of the buffer. @@ -53,10 +53,6 @@ void V_Core::Reverb_AdvanceBuffer() StereoOut32 V_Core::DoReverb( const StereoOut32& Input ) { - static StereoOut32 downbuf[8]; - static StereoOut32 upbuf[8]; - static int dbpos=0, ubpos=0; - static const s32 downcoeffs[8] = { 1283, 5344, 10895, 15243, @@ -75,7 +71,6 @@ StereoOut32 V_Core::DoReverb( const StereoOut32& Input ) // develops a nasty feedback loop. upbuf[ubpos] = StereoOut32::Empty; - ubpos = (ubpos+1) & 7; } else { @@ -83,7 +78,10 @@ StereoOut32 V_Core::DoReverb( const StereoOut32& Input ) UpdateEffectsBufferSize(); if( EffectsBufferSize <= 0 ) + { + ubpos = (ubpos+1) & 7; return StereoOut32::Empty; + } // Advance the current reverb buffer pointer, and cache the read/write addresses we'll be // needing for this session of reverb. @@ -98,10 +96,10 @@ StereoOut32 V_Core::DoReverb( const StereoOut32& Input ) const u32 dest_b0 = RevbGetIndexer( RevBuffers.IIR_DEST_B0 ); const u32 dest_b1 = RevbGetIndexer( RevBuffers.IIR_DEST_B1 ); - const u32 dest2_a0 = RevbGetIndexer( RevBuffers.IIR_DEST_A0 + 1 ); - const u32 dest2_a1 = RevbGetIndexer( RevBuffers.IIR_DEST_A1 + 1 ); - const u32 dest2_b0 = RevbGetIndexer( RevBuffers.IIR_DEST_B0 + 1 ); - const u32 dest2_b1 = RevbGetIndexer( RevBuffers.IIR_DEST_B1 + 1 ); + const u32 dest2_a0 = RevbGetIndexer( RevBuffers.IIR_DEST_A0 + 4 ); + const u32 dest2_a1 = RevbGetIndexer( RevBuffers.IIR_DEST_A1 + 4 ); + const u32 dest2_b0 = RevbGetIndexer( RevBuffers.IIR_DEST_B0 + 4 ); + const u32 dest2_b1 = RevbGetIndexer( RevBuffers.IIR_DEST_B1 + 4 ); const u32 acc_src_a0 = RevbGetIndexer( RevBuffers.ACC_SRC_A0 ); const u32 acc_src_b0 = RevbGetIndexer( RevBuffers.ACC_SRC_B0 ); @@ -143,11 +141,19 @@ StereoOut32 V_Core::DoReverb( const StereoOut32& Input ) const s32 IIR_INPUT_B0 = ((_spu2mem[src_b0] * Revb.IIR_COEF) + (INPUT_SAMPLE.Left * Revb.IN_COEF_L))>>16; const s32 IIR_INPUT_B1 = ((_spu2mem[src_b1] * Revb.IIR_COEF) + (INPUT_SAMPLE.Right * Revb.IN_COEF_R))>>16; + /*const s32 IIR_A0 = (IIR_INPUT_A0 * Revb.IIR_ALPHA) + (_spu2mem[dest_a0] * (0xffff - Revb.IIR_ALPHA)); + const s32 IIR_A1 = (IIR_INPUT_A1 * Revb.IIR_ALPHA) + (_spu2mem[dest_a1] * (0xffff - Revb.IIR_ALPHA)); + const s32 IIR_B0 = (IIR_INPUT_B0 * Revb.IIR_ALPHA) + (_spu2mem[dest_b0] * (0xffff - Revb.IIR_ALPHA)); + const s32 IIR_B1 = (IIR_INPUT_B1 * Revb.IIR_ALPHA) + (_spu2mem[dest_b1] * (0xffff - Revb.IIR_ALPHA)); + _spu2mem[dest2_a0] = clamp_mix( IIR_A0 >> 16 ); + _spu2mem[dest2_a1] = clamp_mix( IIR_A1 >> 16 ); + _spu2mem[dest2_b0] = clamp_mix( IIR_B0 >> 16 ); + _spu2mem[dest2_b1] = clamp_mix( IIR_B1 >> 16 );*/ + const s32 IIR_A0 = IIR_INPUT_A0 + (((_spu2mem[dest_a0]-IIR_INPUT_A0) * Revb.IIR_ALPHA)>>16); const s32 IIR_A1 = IIR_INPUT_A1 + (((_spu2mem[dest_a1]-IIR_INPUT_A1) * Revb.IIR_ALPHA)>>16); const s32 IIR_B0 = IIR_INPUT_B0 + (((_spu2mem[dest_b0]-IIR_INPUT_B0) * Revb.IIR_ALPHA)>>16); const s32 IIR_B1 = IIR_INPUT_B1 + (((_spu2mem[dest_b1]-IIR_INPUT_B1) * Revb.IIR_ALPHA)>>16); - _spu2mem[dest2_a0] = clamp_mix( IIR_A0 ); _spu2mem[dest2_a1] = clamp_mix( IIR_A1 ); _spu2mem[dest2_b0] = clamp_mix( IIR_B0 ); @@ -164,23 +170,31 @@ StereoOut32 V_Core::DoReverb( const StereoOut32& Input ) ((_spu2mem[acc_src_a1] * Revb.ACC_COEF_A)) + ((_spu2mem[acc_src_b1] * Revb.ACC_COEF_B)) + ((_spu2mem[acc_src_c1] * Revb.ACC_COEF_C)) + - ((_spu2mem[acc_src_d1] * Revb.ACC_COEF_D))) - >> 16; + ((_spu2mem[acc_src_d1] * Revb.ACC_COEF_D)) + ) >> 16; - const s32 acc_fb_mix_a = ACC0 + ((_spu2mem[fb_src_a0] - ACC0) * Revb.FB_ALPHA); - const s32 acc_fb_mix_b = ACC1 + ((_spu2mem[fb_src_a1] - ACC1) * Revb.FB_ALPHA); const s32 FB_A0 = (_spu2mem[fb_src_a0] * Revb.FB_ALPHA) >> 16; const s32 FB_A1 = (_spu2mem[fb_src_a1] * Revb.FB_ALPHA) >> 16; _spu2mem[mix_dest_a0] = clamp_mix( ACC0 - FB_A0 ); _spu2mem[mix_dest_a1] = clamp_mix( ACC1 - FB_A1 ); - _spu2mem[mix_dest_b0] = clamp_mix( (acc_fb_mix_a - (_spu2mem[fb_src_b0] * Revb.FB_X)) >> 16 ); - _spu2mem[mix_dest_b1] = clamp_mix( (acc_fb_mix_b - (_spu2mem[fb_src_b1] * Revb.FB_X)) >> 16 ); + const s32 acc_fb_mix_a = ACC0 + (((_spu2mem[fb_src_a0] - ACC0) * Revb.FB_ALPHA)>>16); + const s32 acc_fb_mix_b = ACC1 + (((_spu2mem[fb_src_a1] - ACC1) * Revb.FB_ALPHA)>>16); + _spu2mem[mix_dest_b0] = clamp_mix( acc_fb_mix_a - ((_spu2mem[fb_src_b0] * Revb.FB_X) >> 16) ); + _spu2mem[mix_dest_b1] = clamp_mix( acc_fb_mix_b - ((_spu2mem[fb_src_b1] * Revb.FB_X) >> 16) ); + + //const s32 fb_xor_a0 = _spu2mem[fb_src_a0] * ( Revb.FB_ALPHA ^ 0x8000 ); + //const s32 fb_xor_a1 = _spu2mem[fb_src_a1] * ( Revb.FB_ALPHA ^ 0x8000 ); + //_spu2mem[mix_dest_b0] = clamp_mix( (MulShr32(Revb.FB_ALPHA<<16, ACC0) - fb_xor_a0 - (_spu2mem[fb_src_b0] * Revb.FB_X)) >> 16 ); + //_spu2mem[mix_dest_b1] = clamp_mix( (MulShr32(Revb.FB_ALPHA<<16, ACC1) - fb_xor_a1 - (_spu2mem[fb_src_b1] * Revb.FB_X)) >> 16 ); + + // Note: According Neill these should be divided by 3, but currently the + // output is way too quiet for that to fly. upbuf[ubpos] = clamp_mix( StereoOut32( - _spu2mem[mix_dest_a0] + _spu2mem[mix_dest_b0], // left - _spu2mem[mix_dest_a1] + _spu2mem[mix_dest_b1] // right + (_spu2mem[mix_dest_a0] + _spu2mem[mix_dest_b0]) / 2, // left + (_spu2mem[mix_dest_a1] + _spu2mem[mix_dest_b1]) / 2 // right ) ); } @@ -194,5 +208,7 @@ StereoOut32 V_Core::DoReverb( const StereoOut32& Input ) retval.Left >>= (16-1); /* -1 To adjust for the null padding. */ retval.Right >>= (16-1); + ubpos = (ubpos+1) & 7; + return retval; } diff --git a/plugins/spu2-x/src/defs.h b/plugins/spu2-x/src/defs.h index 9536033848..e6af30b9dd 100644 --- a/plugins/spu2-x/src/defs.h +++ b/plugins/spu2-x/src/defs.h @@ -423,6 +423,10 @@ struct V_Core u32 MADR; u32 TADR; + StereoOut32 downbuf[8]; + StereoOut32 upbuf[8]; + int dbpos, ubpos; + // HACK -- This is a temp buffer which is (or isn't?) used to circumvent some memory // corruption that originates elsewhere in the plugin. >_< The actual ADMA buffer // is an area mapped to SPU2 main memory. diff --git a/plugins/spu2-x/src/spu2sys.cpp b/plugins/spu2-x/src/spu2sys.cpp index 36c1757556..3487fb514e 100644 --- a/plugins/spu2-x/src/spu2sys.cpp +++ b/plugins/spu2-x/src/spu2sys.cpp @@ -161,18 +161,18 @@ void V_Core::Reset( int index ) s32 V_Core::EffectsBufferIndexer( s32 offset ) const { - u32 pos = EffectsStartA + offset; + u32 pos = EffectsStartA + (offset*4); // Need to use modulus here, because games can and will drop the buffer size // without notice, and it leads to offsets several times past the end of the buffer. if( pos > EffectsEndA ) { - pos = EffectsStartA + (offset % EffectsBufferSize); + pos = EffectsStartA + ((offset*4) % EffectsBufferSize); } else if( pos < EffectsStartA ) { - pos = EffectsEndA+1 - (offset % EffectsBufferSize ); + pos = EffectsEndA+1 - ((offset*4) % EffectsBufferSize ); } return pos; } @@ -192,12 +192,12 @@ void V_Core::UpdateFeedbackBuffersB() void V_Core::UpdateEffectsBufferSize() { const s32 newbufsize = EffectsEndA - EffectsStartA + 1; - if( !RevBuffers.NeedsUpdated && newbufsize == EffectsBufferSize ) return; + if( !RevBuffers.NeedsUpdated && (newbufsize == EffectsBufferSize) ) return; RevBuffers.NeedsUpdated = false; EffectsBufferSize = newbufsize; - if( EffectsBufferSize == 0 ) return; + if( EffectsBufferSize <= 0 ) return; // Rebuild buffer indexers. @@ -1020,19 +1020,42 @@ static void __fastcall RegWrite_Core( u16 value ) case REG_A_ESA: SetHiWord( thiscore.EffectsStartA, value ); - thiscore.UpdateEffectsBufferSize(); + thiscore.RevBuffers.NeedsUpdated = true; thiscore.ReverbX = 0; break; case (REG_A_ESA + 2): SetLoWord( thiscore.EffectsStartA, value ); - thiscore.UpdateEffectsBufferSize(); + thiscore.RevBuffers.NeedsUpdated = true; thiscore.ReverbX = 0; break; case REG_A_EEA: - thiscore.EffectsEndA = ((u32)value<<16) | 0xFFFF; - thiscore.UpdateEffectsBufferSize(); + + // Experimental Fix!! --> Disable reverb is EEA is set to 0. + // + // Rationale: + // Digital Devil Saga on the PS2 has no reverb, but it *appears* to have all reverb + // enabled (FX bit, volumes, etc. are all on). On SPU2-X it's reverb settings inevitably + // result in a nasty feedback loop, even when all other games work fine (the values for + // buffers and coefficients are quite odd and unlike most other games' reverb settings, so + // the feedback isn't surprising). Theory: DDS is one of very few games that writes a 0 to + // EffectsEndA, and so my guess is that this disables effects processing on this game. + // + // Any other value written to the EEA serves as a page selector for the effects buffer, but + // it's possible the SPU2 ignores EEA set to 0 before it even gets to the point of establishing + // whether or not the 0xffff portion is a valid buffer position. + // + + if( value == 0 ) + { + thiscore.EffectsEndA = 0; + fprintf( stderr, "* SPU2: EffectsEndA set to 0; disabling reverb effects!\n" ); + } + else + thiscore.EffectsEndA = ((u32)value<<16) | 0xFFFF; + + thiscore.RevBuffers.NeedsUpdated = true; thiscore.ReverbX = 0; break;