qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 08/20] asc: generate silence if FIFO empty but engine stil


From: Philippe Mathieu-Daudé
Subject: Re: [PATCH v2 08/20] asc: generate silence if FIFO empty but engine still running
Date: Thu, 14 Sep 2023 15:16:22 +0200
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.15.0

On 14/9/23 09:56, Philippe Mathieu-Daudé wrote:

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-ayland@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:

-- >8 --
diff --git a/audio/audio.h b/audio/audio.h
index 01bdc567fb..4844771c92 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -30,7 +30,7 @@
  #include "hw/qdev-properties.h"
  #include "hw/qdev-properties-system.h"

-typedef void (*audio_callback_fn) (void *opaque, int avail);
+typedef bool (*audio_callback_fn) (void *opaque, int avail);

  #if HOST_BIG_ENDIAN
  #define AUDIO_HOST_ENDIANNESS 1
diff --git a/audio/audio.c b/audio/audio.c
index 90c7c49d11..5b6e69fbd6 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1178,8 +1178,11 @@ static void audio_run_out (AudioState *s)
                  if (free > sw->resample_buf.pos) {
                      free = MIN(free, sw->resample_buf.size)
                             - sw->resample_buf.pos;
-                    sw->callback.fn(sw->callback.opaque,
-                                    free * sw->info.bytes_per_frame);
+                    if (!sw->callback.fn(sw->callback.opaque,
+                                         free * sw->info.bytes_per_frame)
+                            && unlikely(hw->silentbuf_required)) {
+                        /* write silence ... */
+                    }
                  }
              }
          }
---

(Clarifying, not a blocker for this series, just wondering)




reply via email to

[Prev in Thread] Current Thread [Next in Thread]