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

[Laurent Sallafranque <laurent.sallafranque@xxxxxxx>]

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.
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




Le 21/04/2014 14:07, Douglas Little a écrit :

I made a quick hack change over lunch as a test, which has eliminated the glitch. I'm basing it on the original code from the repo, unpatched. However this does not represent a 'fix' for the problem, it just indicates where the problem is coming from.

Originally zeroes were pushed into the dac.readPosition (ring tail) as soon as data was fetched by it. However if the readPosition does not move on (e.g. if the fractional step is < 1.0) then the next (duplicated) sample would have been zapped with a zero - which is incorrect.

I made this mod which writes the zero only if the integer part of the readPosition advances.

/* Upgrade dac's buffer read pointer */ 

dac.readPosition_float += crossbar.frequence_ratio;

n = dac.readPosition_float >> 32; /* number of samples to skip */

if (n)

{

// becomes safe to zero old data if tail has moved

dac.buffer_left[dac.readPosition] = 0;

dac.buffer_right[dac.readPosition] = 0;

}

dac.readPosition = (dac.readPosition + n) % DACBUFFER_SIZE;

dac.readPosition_float &= 0xffffffff; /* only keep the fractional part */

The captured sample does not have any of the unwanted zeroes present - which I think is indicative.

However it also doesn't have random data in its place. That suggests the previous patch was incorrect, and creating more glitches. I am working with the original (unpatched) code, making only the change above.

I'm still not happy with the change above, because the ring tail should not modify the data at all. However it might help somebody else formulate a fix. I don't understand the reason zeroes are written so I'm not the best person to make that choice.

For now I'll stick with this temporary hack until something better surfaces.

 

I am left with another problem though. It looks different - perhaps related but can't be sure. I can't blame this next problem on Hatari because I'll have to start all over again, following it through from my own code to the output. That won't happen soon, so you might or might not hear from me again at some point :)

D

On 21 April 2014 11:04, Douglas Little <doug694@xxxxxxxxxxxxxx> wrote:

Hi David,

Thanks for looking at this.

Thank you for the feedback. I'll improve the patch and add anti-aliasing
over the next few days. I will remove the zeros from the patch and
replace with filtered samples. Note that Hatari has to convert samples
of one sample rate to another in some places.

Are you sure this particular problem can/should be solved with filtering/interpolating?

The faulty samples seem to be coming from the 'old' state of the buffer from previous writes, and from a position unrelated to the current sample position. It appears to be a gap in the extraction of data (skipped sample) or a race condition between buffer read/write position.

Given this, I think it can only be correctly fixed by figuring out what sort of race condition or addressing bug is causing it. Trying to hide the values will just add complexity and likely produce other side effects on certain sounds.

Here's a snap from the latest output, with the zeroes no longer written to the readposition.

https://dl.dropboxusercontent.com/u/12947585/wav4.png

As you can see, the bad samples don't correlate at all with the current sample position, so filtering them won't help. Masking them (by interpolating them from neighbors) might, if their position is always known - but it will mean dropping a 'good' sample to do so. It seems to me that finding/fixing the buffer addressing fault would result in a better solution, with no samples dropped?

D