Re: [hatari-devel] Falcon left-right sound swapper bug?

[Eero Tamminen <oak@xxxxxxxxxxxxxx>]

Hi,

On torstai 17 huhtikuu 2014, David Savinkoff wrote:
> crossbar.c is the only file that has:
> dac.readPosition :: dac.writePosition
> and:
> adc2dac_readBufferPosition :: adc2dac_writeBufferPosition
> 
> These circular buffer indexes must check that they do not over-run each
> other.
> 
> dmaSnd.c maintains the circular buffer indexes with:
> static void DmaSnd_FIFO_Refill(void);
> static Sint8 DmaSnd_FIFO_PullByte(void);
> 
> crossbar.c has incomplete functions:
> void Crossbar_GetMicrophoneDatas(Sint16 *micro_bufferL, Sint16
> *micro_bufferR, Uint32 microBuffer_size); static void
> Crossbar_SendDataToDAC(Sint16 value, Uint16 sample_pos); void
> Crossbar_GenerateSamples(int nMixBufIdx, int nSamplesToGenerate);

There's following old bug in Hatari Crossbar DMA code:
http://listengine.tuxfamily.org/lists.tuxfamily.org/hatari-
devel/2012/02/msg00082.html

It doesn't check the indeces given by emulated code
which is really bad because emulated code can then
corrupt Hatari's own memory.  That might then have also
other effects.


	- Eero

> Sincerely,
> David Savinkoff
> 
> ----- Laurent Sallafranque wrote:
> > Hi David,
> > 
> > I introduced these 0 later in the crossbar code, because sometimes,
> > when the 68030 was crashing, there was an horrible repeating sound in
> > hatari. In the real hardware, when a value is read into the little 32
> > bytes memory, I think it doesn't exist anymore after, so that's why I
> > introduced these 0.
> > 
> > Removing them doesn't remove the problem. Instead, when there's the
> > problem, we can hear an echo (repeating of the latest 0.5 s or so (0.5
> > is not an exact value here))
> > 
> > 2 things I find stange :
> > 
> > - when I try to change #define DACBUFFER_SIZE    2048    by 4096 or
> > more, I have no sound or a really really dirty sound
> > 
> > - I've placed a little test which detects a value !=0 and then count
> > when there are more than 6 consecutive 0 in a sound (and then display
> > it
> > 
> > found a case) :
> >      int detect_in = 0;
> >      int count_in = 0;
> >      
> >          // Detect in
> >          if (dac_LeftData != 0)
> >          
> >              detect_in = 1;
> >          
> >          if (detect_in == 1) {
> >          
> >              if (dac_LeftData == 0) {
> >              
> >                  count_in ++;
> >                  if (count_in > 6) {
> >                  
> >                      fprintf(stderr, "IN > 6\n");
> >                      detect_in = 0;
> >                      count_in = 0;
> >                  
> >                  }
> >              
> >              }
> >          
> >          }
> > 
> > I've placed this code into Crossbar_GenerateSamples :
> >    I've places it at the end to detect if there's a 0 hole in the
> > 
> > dac_LeftData  and in the MixBuffer[nBufIdx][0]
> > 
> >    I get some few holes when the sound is OK and a lot of holes when
> >    the
> > 
> > sound is dirty.
> > 
> > So, the echo when I apply your patch (or the 0 value when not) + this
> > 
> > test seems to indicate :
> >    - we're sometimes reading the buffer backward (or too far at once,
> > 
> > wich, with the modulo DACBUFFER_SIZE returns before the latest values
> > played)
> > or
> > 
> >    - we're sometimes reading the buffer many times at the same place
> > 
> > (which would be strange according to the code).
> > 
> > I've given a look at nSamplesToGenerate (in the function parameters).
> > It's always 882 (with good or wrong sound)
> > 
> > I continue to look at this.
> > 
> > Laurent
> > 
> > Le 16/04/2014 21:12, David Savinkoff a écrit :
> > > ----- Laurent Sallafranque wrote:
> > >> Hi David,
> > >> 
> > >> I don't think the bug is here :
> > >> 
> > >> The aim of this code is to separate the sound into a left and a
> > >> right part.
> > > 
> > > Hi Laurent,
> > > 
> > > Thank you for the explanation. I have been comparing dmaSnd.c with
> > > crossbar.c and finding ways to improve crossbar.c
> > > 
> > > I noticed that zeros are being written back to a buffer after the
> > > data is read so that a subsequent read would be zero... to avoid
> > > introducing a zero order hold filtering effect. However; if this
> > > buffer is 'actual' Falcon memory, there is a problem.
> > > 
> > > Please see the enclosed patch.
> > > 
> > > Sincerely,
> > > David Savinkoff