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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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 ... */
+                    }
                  }
              }
          }
---