On 16/9/23 10:19, Volker Rümelin wrote:
Am 14.09.23 um 09:56 schrieb Philippe Mathieu-Daudé:

On 9/9/23 11:48, Mark Cave-Ayland wrote:
MacOS (un)helpfully leaves the FIFO engine running even when all the
samples have
been written to the hardware, and expects the FIFO status flags and
IRQ to be
updated continuously.

There is an additional problem in that not all audio backends
guarantee an
all-zero output when there is no FIFO data available, in particular
the Windows
dsound backend which re-uses its internal circular buffer causing the
last played
sound to loop indefinitely.

Whilst this is effectively a bug in the Windows dsound backend, work
around it
for now using a simple heuristic: if the FIFO remains empty for half
a cycle
(~23ms) then continuously fill the generated buffer with empty silence.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>
---
   hw/audio/asc.c         | 19 +++++++++++++++++++
   include/hw/audio/asc.h |  2 ++
   2 files changed, 21 insertions(+)

diff --git a/hw/audio/asc.c b/hw/audio/asc.c
index 336ace0cd6..b01b285512 100644
--- a/hw/audio/asc.c
+++ b/hw/audio/asc.c
@@ -334,6 +334,21 @@ static void asc_out_cb(void *opaque, int free_b)
       }
         if (!generated) {
+        /* Workaround for audio underflow bug on Windows dsound
backend */
+        int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+        int silent_samples = muldiv64(now - s->fifo_empty_ns,
+                                      NANOSECONDS_PER_SECOND,
ASC_FREQ);
+
+        if (silent_samples > ASC_FIFO_CYCLE_TIME / 2) {
+            /*
+             * No new FIFO data within half a cycle time (~23ms) so
fill the
+             * entire available buffer with silence. This prevents
an issue
+             * with the Windows dsound backend whereby the sound
appears to
+             * loop because the FIFO has run out of data, and the
driver
+             * reuses the stale content in its circular audio buffer.
+             */
+            AUD_write(s->voice, s->silentbuf, samples << s->shift);
+        }
           return;
       }

What about having audio_callback_fn returning a boolean, and using
a flag in backends for that silence case? Roughtly:


Hi Philippe,

I don't think there will be many audio devices that need this feature in
the future. It's probably better to keep the code in hw/audio/asc.c

I plan to change the buffer underflow behavior of the DirectSound
backend after these patches are commited. I already have a patch
available. This doesn't mean this patch is unnecessary after my patch.
It is a mistake when audio devices simply stop writing audio samples
without changing the status of the audio playback stream to deactivated.
At the moment the ASC device can't deactivate the audio stream.

OK, thanks for clarifying.


Reply via email to