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

@@ -583,52 +583,44 @@ 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;
-	{
+	
-		//Reverb pointer advances regardless of the FxEnable bit...
+	// ----------------------------------------------------------------------------
+	//    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 )
 	{
 		// Mix Input, Voice, and External data:
-			StereoOut32 TW(
+
-				Input.Left & WetGate.InpL,
+		TW.Left = Input.Left & WetGate.InpL;
-				Input.Right & WetGate.InpR
+		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
+	// (master volume is applied later to the result of both outputs added together).
 	return TD + ApplyVolume( RV, FxVol );
 }
-		else
-		{
-			WaveDump::WriteCore( Index, CoreSrc_PreReverb, 0, 0 );
-			WaveDump::WriteCore( Index, CoreSrc_PostReverb, 0, 0 );
-		}
-	}
-	return TD;
-}
 
 // used to throttle the output rate of cache stat reports
 static int p_cachestat_counter=0;
@@ -659,8 +651,7 @@ __forceinline void Mix()
 		Ext = StereoOut32::Empty;
 	else
 	{
-		Ext = ApplyVolume( Ext, Cores[0].MasterVol );
+		Ext = clamp_mix( ApplyVolume( Ext, Cores[0].MasterVol ) );
-		clamp_mix( Ext );
 	}
 
 	// 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)

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_a0 = RevbGetIndexer( RevBuffers.IIR_DEST_A0 + 4 );
-		const u32 dest2_a1 = RevbGetIndexer( RevBuffers.IIR_DEST_A1 + 1 );
+		const u32 dest2_a1 = RevbGetIndexer( RevBuffers.IIR_DEST_A1 + 4 );
-		const u32 dest2_b0 = RevbGetIndexer( RevBuffers.IIR_DEST_B0 + 1 );
+		const u32 dest2_b0 = RevbGetIndexer( RevBuffers.IIR_DEST_B0 + 4 );
-		const u32 dest2_b1 = RevbGetIndexer( RevBuffers.IIR_DEST_B1 + 1 );
+		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)))
+			((_spu2mem[acc_src_d1] * Revb.ACC_COEF_D))
-		 >> 16;
+		) >> 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_a0] + _spu2mem[mix_dest_b0]) / 2,	// left
-			_spu2mem[mix_dest_a1] + _spu2mem[mix_dest_b1]	// right
+			(_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;
 }

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.

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:
+		
+			// 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.UpdateEffectsBufferSize();
+
+			thiscore.RevBuffers.NeedsUpdated = true;
 			thiscore.ReverbX = 0;
 		break;