Re: [hatari-devel] Re: Interesting sound problems with Hatari?

[David Savinkoff <dsavnkff@xxxxxxxxx>]

Hi,

This patch combines the two previous with a bit more.
Don't apply this patch with my previous patches.
Please test.

David Savinkoff

----- David Savinkoff wrote:
> ----- Laurent Sallafranque wrote:
> > Hi,
> > 
> > First, sorry for not replying sooner, I was not at home this week end.
> > I've just discovered you patches.
> > 
> > I did write the whole crossbar sound code 3 years ago, and if I remember 
> > correctly, David added some more precise values and filters (while he 
> > was increasing the STE DMA sound part).
> > 
> > I added the 0 after reading a sound because of the DSP part in the 
> > crossbar (on the real Falcon) (I was really concerned by the DSP part at 
> > this time).
> > 
> > When the DSP sends a data to the SSI, this value is played by the 
> > crossbar directly into the DAC part.
> > If then you tristate the DSP (or stop it, reboot it or whatever stops 
> > the sounds), there's no more sound coming out of the DAC.
> > But if I didn't put zeros after reading the value (into hatari), the 
> > sound (coming from the DSP) was looping forever into the circular DAC 
> > buffer.
> > 
> > maybe this should be done differently, but I still think that a played 
> > sound into the DAC buffer should be erazed.
> 
> Another way is to repeat the last good sample (instead of inserting zeros).
> This is what a DAC will do when the data stops.
> 
> Note that inserting samples (filtered or otherwise) will not be used to hide
> correctable problems.
> 
> > That's the same if the crossbar does a DMA --> DAC connection (the sound 
> > read from the DMA (Falcon MEMORY) should be set to 0 after being read 
> > (to remove the circular buffer sound playing forever).
> > 
> > Only the source of the sound should remain unchanged (mainly the DMA 
> > memory), whereas the DSP sounds are volatile (computed once, sent once 
> > to the SSI, played once by the DAC, and forgotten after this).
> > 
> > I like the idea of the patch by David (a FIFO, which allows to read a 
> > sound only once).
> > 
> > 
> > Doug, the *if (n) *patch is a great idea.
> > I was just thinking that is n is > 1 (let's say n = 5), maybe we should 
> > reset to 0 the whole values between n=1 and n=5 ?
> > 
> > I'll have a closer look too at this problem.
> > 
> > Regards
> > 
> > Laurent
> > 
> 
> 

--- hatari/src/falcon/crossbar.c	2014-04-17 12:44:33.000000000 -0700
+++ hatari/src/falcon/crossbar.c	2014-04-21 16:40:25.000000000 -0700
@@ -135,6 +135,10 @@
 /* ADC functions */
 static void Crossbar_Process_ADCXmit_Transfer(void);
 
+/* Anti-alias crossbar generated samples */
+static Sint16 Crossbar_LowPassFilterLeft(Sint16 in);
+static Sint16 Crossbar_LowPassFilterRight(Sint16 in);
+
 /* external data used by the MFP */
 Uint16 nCbar_DmaSoundControl;
 
@@ -261,13 +265,12 @@
 	Uint32 dmaRecord_CurrentFrameStart; /* current DmaPlay Frame end ($ff890f $ff8911 $ff8913) */
 	Uint32 dmaRecord_CurrentFrameCount; /* current DmaRecord Frame start ($ff8903 $ff8905 $ff8907) */
 	Uint32 dmaRecord_CurrentFrameEnd;   /* current DmaRecord Frame end ($ff890f $ff8911 $ff8913) */
-	Uint32 adc2dac_readBufferPosition;  /* read position for direct adc->dac transfer */
-	Sint64 adc2dac_readBufferPosition_float; /* float value of read position for direct adc->dac transfer index */
 	
 	Uint32 save_special_transfer;		/* Used in a special undocumented transfer mode (dsp sent is not in handshake mode and dsp receive is in handshake mode) */
 };
 
 struct codec_s {
+	int    FIFO_NbBytes;
 	Sint16 buffer_left[DACBUFFER_SIZE];
 	Sint16 buffer_right[DACBUFFER_SIZE];
 	Sint64 readPosition_float;
@@ -293,6 +296,7 @@
 static struct dma_s dmaRecord;
 static struct codec_s dac;
 static struct codec_s adc;
+static struct codec_s mic;
 static struct dsp_s dspXmit;
 static struct dsp_s dspReceive;
 
@@ -326,6 +330,7 @@
 	dac.readPosition_float = 0;
 	dac.readPosition = 0;
 	dac.writePosition = 0;
+	dac.FIFO_NbBytes = 0;
 
 	/* ADC inits */
 	memset(adc.buffer_left, 0, sizeof(adc.buffer_left));
@@ -333,6 +338,14 @@
 	adc.readPosition_float = 0;
 	adc.readPosition = 0;
 	adc.writePosition = 0;
+	adc.FIFO_NbBytes = 0;
+
+	/* Microphone inits */
+	memset(mic.buffer_left, 0, sizeof(mic.buffer_left));
+	memset(mic.buffer_right, 0, sizeof(mic.buffer_right));
+	mic.readPosition = 0;
+	mic.writePosition = 0;
+	mic.FIFO_NbBytes = 0;
 
 	/* DSP inits */
 	dspXmit.wordCount = 0;
@@ -363,8 +376,6 @@
 	crossbar.gainSettingRight = 3276;
 	crossbar.attenuationSettingLeft = 65535;
 	crossbar.attenuationSettingRight = 65535;
-	crossbar.adc2dac_readBufferPosition = 0;
-	crossbar.adc2dac_readBufferPosition_float = 0;
 
 	/* Start 25 Mhz and 32 Mhz Clocks */
 	Crossbar_Recalculate_Clocks_Cycles();
@@ -1699,14 +1710,34 @@
 	idxPos = 0;
 
 	for (i = 0; i < size; i++) {
-		adc.writePosition = (adc.writePosition + 1) % DACBUFFER_SIZE;
-
 		adc.buffer_left[adc.writePosition] = micro_bufferL[bufferIndex];
 		adc.buffer_right[adc.writePosition] = micro_bufferR[bufferIndex]; 
 
 		idxPos += crossbar.frequence_ratio2;
 		bufferIndex += idxPos>>32;	
 		idxPos  &= 0xffffffff;			/* only keep the fractional part */
+
+		adc.writePosition = (adc.writePosition + 1) % DACBUFFER_SIZE;
+
+		if (++adc.FIFO_NbBytes > DACBUFFER_SIZE) {
+			adc.FIFO_NbBytes = DACBUFFER_SIZE;
+			adc.readPosition = adc.writePosition;
+		}
+	}
+
+	bufferIndex = 0;
+	for (i = 0; i < microBuffer_size; i++) {
+		mic.buffer_left[mic.writePosition] = micro_bufferL[bufferIndex];
+		mic.buffer_right[mic.writePosition] = micro_bufferR[bufferIndex]; 
+
+		bufferIndex++;
+
+		mic.writePosition = (mic.writePosition + 1) % DACBUFFER_SIZE;
+
+		if (++mic.FIFO_NbBytes > DACBUFFER_SIZE) {
+			mic.FIFO_NbBytes = DACBUFFER_SIZE;
+			mic.readPosition = mic.writePosition;
+		}
 	}
 }
 
@@ -1728,10 +1759,16 @@
 	}
 	else {
 		sample = adc.buffer_right[adc.readPosition];
-		adc.readPosition = (adc.readPosition + 1) % DACBUFFER_SIZE;
 		frame = 0;
+
+		if (--adc.FIFO_NbBytes < 0) {
+			adc.FIFO_NbBytes = 0;
+		}
+		else {
+			adc.readPosition = (adc.readPosition + 1) % DACBUFFER_SIZE;
+		}
 	}
-	
+
 	/* Send sample to DSP receive ? */
 	if (adc.isConnectedToDsp) {
 		Crossbar_SendDataToDspReceive(sample, frame);
@@ -1770,6 +1807,11 @@
 		/* Right channel */
 		dac.buffer_right[dac.writePosition] = value;
 		dac.writePosition = (dac.writePosition + 1) % (DACBUFFER_SIZE);
+
+		if (++dac.FIFO_NbBytes > DACBUFFER_SIZE) {
+			dac.FIFO_NbBytes = DACBUFFER_SIZE;
+			dac.readPosition = dac.writePosition;
+		}
 	}
 }
 
@@ -1784,41 +1826,33 @@
 	unsigned n;
 	Sint16 adc_leftData, adc_rightData, dac_LeftData, dac_RightData;
 	
-	if (crossbar.isDacMuted) {
-		/* Output sound = 0 */
-		for (i = 0; i < nSamplesToGenerate; i++) {
-			nBufIdx = (nMixBufIdx + i) % MIXBUFFER_SIZE;
-			MixBuffer[nBufIdx][0] = 0;
-			MixBuffer[nBufIdx][1] = 0;
-		}
-
-		/* Counters are refreshed for when DAC becomes unmuted */
-		dac.readPosition = dac.writePosition;
-		crossbar.adc2dac_readBufferPosition = adc.writePosition;
-		return;
-	}
-
 	for (i = 0; i < nSamplesToGenerate; i++)
 	{
 		nBufIdx = (nMixBufIdx + i) % MIXBUFFER_SIZE;
 
+		if (dac.FIFO_NbBytes > 0)
+			dac.FIFO_NbBytes--;
+		
+		if (mic.FIFO_NbBytes > 0)
+			mic.FIFO_NbBytes--;
+		
 		/* ADC mixing (PSG sound or microphone sound for left and right channels) */
 		switch (crossbar.codecAdcInput) {
 			case 0:
 			default: /* Just here to remove compiler's warnings */
 				/* Microphone sound for left and right channels */
-				adc_leftData = adc.buffer_left[crossbar.adc2dac_readBufferPosition];
-				adc_rightData = adc.buffer_right[crossbar.adc2dac_readBufferPosition];
+				adc_leftData = mic.buffer_left[mic.readPosition];
+				adc_rightData = mic.buffer_right[mic.readPosition];
 				break;
 			case 1:
 				/* Microphone sound for left channel, PSG sound for right channel */
-				adc_leftData = adc.buffer_left[crossbar.adc2dac_readBufferPosition];
+				adc_leftData = mic.buffer_left[mic.readPosition];
 				adc_rightData = MixBuffer[nBufIdx][1];
 				break;
 			case 2:
 				/* PSG sound for left channel, microphone sound for right channel */
 				adc_leftData = MixBuffer[nBufIdx][0];
-				adc_rightData = adc.buffer_right[crossbar.adc2dac_readBufferPosition];
+				adc_rightData = mic.buffer_right[mic.readPosition];
 				break;
 			case 3:
 				/* PSG sound for left and right channels */
@@ -1842,19 +1876,15 @@
 				break;
 			case 2:
 				/* Crossbar->DAC sound only */
-				dac_LeftData = dac.buffer_left[dac.readPosition];
-				dac_RightData = dac.buffer_right[dac.readPosition];
-				dac.buffer_left[dac.readPosition] = 0;
-				dac.buffer_right[dac.readPosition] = 0;
+				dac_LeftData =  Crossbar_LowPassFilterLeft( dac.buffer_left[dac.readPosition] );
+				dac_RightData = Crossbar_LowPassFilterRight( dac.buffer_right[dac.readPosition] );
 				break;
 			case 3:
 				/* Mixing Direct ADC sound with Crossbar->DMA sound */
 				dac_LeftData = ((adc_leftData * crossbar.gainSettingLeft) >> 14) +
-						dac.buffer_left[dac.readPosition];
+						Crossbar_LowPassFilterLeft( dac.buffer_left[dac.readPosition] );
 				dac_RightData = ((adc_rightData  * crossbar.gainSettingRight) >> 14) +
-						dac.buffer_right[dac.readPosition];
-				dac.buffer_left[dac.readPosition] = 0;
-				dac.buffer_right[dac.readPosition] = 0;
+						Crossbar_LowPassFilterRight( dac.buffer_right[dac.readPosition] );
 				break;
 		}
 			
@@ -1863,14 +1893,56 @@
 
 		/* Upgrade dac's buffer read pointer */ 
 		dac.readPosition_float += crossbar.frequence_ratio;
-		n = dac.readPosition_float >> 32;				/* number of samples to skip */
+		n = dac.readPosition_float >> 32;			/* number of samples to skip */
+		for ( ;  n > 1 ;  n-- )					/* pull samples from FIFO    */
+		{
+			if (dac.FIFO_NbBytes > 0)
+			{
+				dac.FIFO_NbBytes--;
+				dac.readPosition = ++dac.readPosition % DACBUFFER_SIZE;
+			}
+			Crossbar_LowPassFilterLeft( dac.buffer_left[dac.readPosition] );
+			Crossbar_LowPassFilterRight( dac.buffer_right[dac.readPosition] );
+		}
 		dac.readPosition = (dac.readPosition + n) % DACBUFFER_SIZE;
 		dac.readPosition_float &= 0xffffffff;			/* only keep the fractional part */
-		
-		/* Upgrade adc->dac's buffer read pointer */ 
-		crossbar.adc2dac_readBufferPosition_float += crossbar.frequence_ratio;
-		n = crossbar.adc2dac_readBufferPosition_float >> 32;				/* number of samples to skip */
-		crossbar.adc2dac_readBufferPosition = (crossbar.adc2dac_readBufferPosition + n) % DACBUFFER_SIZE;
-		crossbar.adc2dac_readBufferPosition_float &= 0xffffffff;			/* only keep the fractional part */
+
+		/* Upgrade microphone buffer read pointer */ 
+		mic.readPosition = ++mic.readPosition % DACBUFFER_SIZE;
+
+		if (crossbar.isDacMuted) {
+			MixBuffer[nBufIdx][0] = 0;
+			MixBuffer[nBufIdx][1] = 0;
+		}
 	}
 }
+
+/**
+ * LowPass Filter Left
+ */
+static Sint16 Crossbar_LowPassFilterLeft(Sint16 in)
+{
+	static	Sint16	lowPassFilter[2] = { 0, 0 };
+	static	Sint16	out = 0;
+
+	out = lowPassFilter[0] + (lowPassFilter[1]<<1) + in;
+	lowPassFilter[0] = lowPassFilter[1];
+	lowPassFilter[1] = in;
+
+	return out >> 2; /* Filter Gain = 4 */
+}
+
+/**
+ * LowPass Filter Right
+ */
+static Sint16 Crossbar_LowPassFilterRight(Sint16 in)
+{
+	static	Sint16	lowPassFilter[2] = { 0, 0 };
+	static	Sint16	out = 0;
+
+	out = lowPassFilter[0] + (lowPassFilter[1]<<1) + in;
+	lowPassFilter[0] = lowPassFilter[1];
+	lowPassFilter[1] = in;
+
+	return out >> 2; /* Filter Gain = 4 */
+}