On Wed, 08 Jul 2020 12:18:48 +0200, Mark Hills wrote:
Distorted audio appears occasionally, affecting either playback or capture and requiring the affected substream to be closed by all applications and re-opened.
The best way I have found to reproduce the bug is to use dmix in combination with Chromium, which opens the audio device multiple times in threads. Anecdotally, the problems appear to have increased with faster CPUs. I ruled out 32-bit counter wrapping; it often happens much earlier.
Since applying this patch I have not had problems, where previously they would occur several times a day.
The patch targets the following issues:
Check for progress using the counter from the hardware, not after it has been truncated to the buffer.
This is a clean way to address a possible bug where if a whole ringbuffer advances between interrupts, it goes unnoticed.
Move last_period state from chip to pipe
This more logically belongs as part of pipe, and code is reasier to read if it is "counter position last time a period elapsed".
Now the code has no references to period count. A period is just when the regular counter crosses a threshold. This increases readability and reduces scope for bugs.
Treat period notification and buffer advance independently:
This helps to clarify what is the responsibility of the interrupt handler, and what is pcm_pointer().
Removing shared state between these operations means race conditions are fixed without introducing locks. Synchronisation is only around the read of pipe->dma_counter. There may be cache line contention around "struct audiopipe" but I did not have cause to profile this.
Pay attention to be robust where dma_counter wrapping is not a multiple of period_size or buffer_size.
This is a revised patch based on feedback from Takashi and Giuliano.
Signed-off-by: Mark Hills mark@xwax.org
Applied now. Thanks.
Takashi