[PATCH 00/23] next round of audio patches

[PATCH 00/23] next round of audio patches

[Volker Rümelin]

A mix of bug fixes and improvements.

Patches 01/23 - 11/23 have a few SDL fixes and add audio recording
functions to the SDL audio backend.

Patch 12/23 removes unnecessary code from audio/audio.c.

Patches 13/23 - 16/23 fix a few PulseAudio backend bugs.

Patch 17/23 shows a PulseAudio backend bug. So far I don't think I
broke any fine tuned user settings and I don't want to do it here.

Patches 18/23 - 19/23 are the first steps towards glitch free and
lower latency PulseAudio playback and recording. Currently qemu
uses incredibly large buffers in the PulseAudio backend. For
playback this just increases the playback latency but doesn't
improve dropout safety, because PulseAudio can't access this buffer
directly. With these patches it's possible to move the large qemu
buffer to the PulseAudio server side and just keep a small buffer
on the qemu side. On the PulseAudio server side PulseAudio tries
to place a part of these buffers directly on the hardware and
PulseAudio runs with a higher priority than qemu, so it has a
better chance to deliver audio data in time.

Here is an example to show how this works:
-device intel-hda -device hda-duplex,audiodev=audio0
-machine pcspk-audiodev=audio0 -audiodev pa,id=audio0,
out.buffer-length=14000,out.latency=46440,in.latency=46440

Due to a bug in the PulseAudio backend, these command line options
actually decrease the playback latency compared to current defaults.
For playback with defaults (16 bits, stereo, 44100 samples/s)
we have a 15ms server side buffer + 2 * 46.44ms qemu audio buffer
+ 23.22ms hda codec buffer = 131.1ms latency. With my example it's
46.44ms + 2 * 14ms + 23.22ms = 97.66ms latency and I guess you
won't hear any drop outs. Btw.: 14ms = 10ms timer-period + 4ms
additional playback data the hda codec can produce in timer-period
time.

Patches 20/23 - 23/23 fix small issues with DirectSound.

Volker Rümelin (23):
  sdlaudio: remove leftover SDL1.2 code
  audio: fix bit-rotted code
  sdlaudio: add -audiodev sdl,out.buffer-count option
  sdlaudio: don't start playback in init routine
  sdlaudio: always clear the sample buffer
  sdlaudio: fill remaining sample buffer with silence
  sdlaudio: replace legacy functions with modern ones
  audio: split pcm_ops function get_buffer_in
  sdlaudio: add recording functions
  audio: break generic buffer dependency on mixing-engine
  sdlaudio: enable (in|out).mixing-engine=off
  audio: remove remaining unused plive code
  paaudio: avoid to clip samples multiple times
  paaudio: wait for PA_STREAM_READY in qpa_write()
  paaudio: wait until the playback stream is ready
  paaudio: remove unneeded code
  paaudio: comment bugs in functions qpa_init_*
  paaudio: limit minreq to 75% of audio timer_rate
  paaudio: send recorded data in smaller chunks
  dsoundaudio: replace GetForegroundWindow()
  dsoundaudio: rename dsound_open()
  dsoundaudio: enable f32 audio sample format
  dsoundaudio: fix log message

 audio/alsaaudio.c       |   3 +-
 audio/audio.c           |  48 +++----
 audio/audio_int.h       |   2 +
 audio/audio_legacy.c    |   3 +-
 audio/audio_template.h  |   2 +-
 audio/audio_win_int.c   |  73 ++++++----
 audio/dsound_template.h |   2 +-
 audio/dsoundaudio.c     |   6 +-
 audio/jackaudio.c       |   1 +
 audio/noaudio.c         |   1 +
 audio/ossaudio.c        |   1 +
 audio/paaudio.c         |  71 ++++++++--
 audio/sdlaudio.c        | 305 ++++++++++++++++++++++++++++------------
 audio/spiceaudio.c      |   1 +
 qapi/audio.json         |  33 ++++-
 qemu-options.hx         |   8 +-
 16 files changed, 399 insertions(+), 161 deletions(-)

-- 
2.26.2

[PATCH 01/23] sdlaudio: remove leftover SDL1.2 code

[Volker Rümelin]

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/sdlaudio.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c
index 21b7a0484b..bf3cfb8456 100644
--- a/audio/sdlaudio.c
+++ b/audio/sdlaudio.c
@@ -240,28 +240,24 @@ static void sdl_callback (void *opaque, Uint8 *buf, int len)
     }
 }
 
-#define SDL_WRAPPER_FUNC(name, ret_type, args_decl, args, fail, unlock) \
-    static ret_type glue(sdl_, name)args_decl                           \
-    {                                                                   \
-        ret_type ret;                                                   \
-                                                                        \
-        SDL_LockAudio();                                                \
-                                                                        \
-        ret = glue(audio_generic_, name)args;                           \
-                                                                        \
-        SDL_UnlockAudio();                                              \
-        return ret;                                                     \
+#define SDL_WRAPPER_FUNC(name, ret_type, args_decl, args)      \
+    static ret_type glue(sdl_, name)args_decl                  \
+    {                                                          \
+        ret_type ret;                                          \
+                                                               \
+        SDL_LockAudio();                                       \
+        ret = glue(audio_generic_, name)args;                  \
+        SDL_UnlockAudio();                                     \
+                                                               \
+        return ret;                                            \
     }
 
 SDL_WRAPPER_FUNC(get_buffer_out, void *, (HWVoiceOut *hw, size_t *size),
-                 (hw, size), *size = 0, sdl_unlock)
+                 (hw, size))
 SDL_WRAPPER_FUNC(put_buffer_out, size_t,
-                 (HWVoiceOut *hw, void *buf, size_t size), (hw, buf, size),
-                 /*nothing*/, sdl_unlock_and_post)
+                 (HWVoiceOut *hw, void *buf, size_t size), (hw, buf, size))
 SDL_WRAPPER_FUNC(write, size_t,
-                 (HWVoiceOut *hw, void *buf, size_t size), (hw, buf, size),
-                 /*nothing*/, sdl_unlock_and_post)
-
+                 (HWVoiceOut *hw, void *buf, size_t size), (hw, buf, size))
 #undef SDL_WRAPPER_FUNC
 
 static void sdl_fini_out (HWVoiceOut *hw)
-- 
2.26.2

[PATCH 02/23] audio: fix bit-rotted code

[Volker Rümelin]

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/alsaaudio.c | 2 +-
 audio/sdlaudio.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
index a8e62542f9..6787e91bc1 100644
--- a/audio/alsaaudio.c
+++ b/audio/alsaaudio.c
@@ -599,7 +599,7 @@ static int alsa_open(bool in, struct alsa_params_req *req,
     }
 
 #ifdef DEBUG
-    alsa_dump_info(req, obt, obtfmt, pdo);
+    alsa_dump_info(req, obt, obtfmt, apdo);
 #endif
     return 0;
 
diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c
index bf3cfb8456..00cd12ba66 100644
--- a/audio/sdlaudio.c
+++ b/audio/sdlaudio.c
@@ -215,7 +215,7 @@ static void sdl_callback (void *opaque, Uint8 *buf, int len)
         return;
     }
 
-    /* dolog ("in callback samples=%zu live=%zu\n", samples, sdl->live); */
+    /* dolog("callback: len=%d avail=%zu\n", len, hw->pending_emul); */
 
     while (hw->pending_emul && len) {
         size_t write_len;
-- 
2.26.2

[PATCH 03/23] sdlaudio: add -audiodev sdl,out.buffer-count option

[Volker Rümelin]

Currently there is a crackling noise with SDL2 audio playback.
Commit bcf19777df: "audio/sdlaudio: Allow audio playback with
SDL2" already mentioned the crackling noise.

Add an out.buffer-count option to give users a chance to select
sane settings for glitch free audio playback. The idea was taken
from the coreaudio backend.

The in.buffer-count option will be used with one of the next
patches.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/audio.c          |  2 +-
 audio/audio_legacy.c   |  3 ++-
 audio/audio_template.h |  2 +-
 audio/sdlaudio.c       | 11 +++++++++--
 qapi/audio.json        | 33 ++++++++++++++++++++++++++++++++-
 qemu-options.hx        |  8 +++++++-
 6 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index b48471bb3f..d048d26283 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -2003,7 +2003,7 @@ void audio_create_pdos(Audiodev *dev)
         CASE(JACK, jack, Jack);
         CASE(OSS, oss, Oss);
         CASE(PA, pa, Pa);
-        CASE(SDL, sdl, );
+        CASE(SDL, sdl, Sdl);
         CASE(SPICE, spice, );
         CASE(WAV, wav, );
 
diff --git a/audio/audio_legacy.c b/audio/audio_legacy.c
index ffdbd0bcce..0fe827b057 100644
--- a/audio/audio_legacy.c
+++ b/audio/audio_legacy.c
@@ -286,7 +286,8 @@ static void handle_sdl(Audiodev *dev)
 {
     /* SDL is output only */
     get_samples_to_usecs("QEMU_SDL_SAMPLES", &dev->u.sdl.out->buffer_length,
-                         &dev->u.sdl.out->has_buffer_length, dev->u.sdl.out);
+        &dev->u.sdl.out->has_buffer_length,
+        qapi_AudiodevSdlPerDirectionOptions_base(dev->u.sdl.out));
 }
 
 /* wav */
diff --git a/audio/audio_template.h b/audio/audio_template.h
index 8dd48ce14e..434df5d5e7 100644
--- a/audio/audio_template.h
+++ b/audio/audio_template.h
@@ -337,7 +337,7 @@ AudiodevPerDirectionOptions *glue(audio_get_pdo_, TYPE)(Audiodev *dev)
     case AUDIODEV_DRIVER_PA:
         return qapi_AudiodevPaPerDirectionOptions_base(dev->u.pa.TYPE);
     case AUDIODEV_DRIVER_SDL:
-        return dev->u.sdl.TYPE;
+        return qapi_AudiodevSdlPerDirectionOptions_base(dev->u.sdl.TYPE);
     case AUDIODEV_DRIVER_SPICE:
         return dev->u.spice.TYPE;
     case AUDIODEV_DRIVER_WAV:
diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c
index 00cd12ba66..431bfcfddd 100644
--- a/audio/sdlaudio.c
+++ b/audio/sdlaudio.c
@@ -276,12 +276,18 @@ static int sdl_init_out(HWVoiceOut *hw, struct audsettings *as,
     int endianness;
     int err;
     AudioFormat effective_fmt;
+    AudiodevSdlPerDirectionOptions *spdo = s->dev->u.sdl.out;
     struct audsettings obt_as;
 
     req.freq = as->freq;
     req.format = aud_to_sdlfmt (as->fmt);
     req.channels = as->nchannels;
-    req.samples = audio_buffer_samples(s->dev->u.sdl.out, as, 11610);
+    /*
+     * This is wrong. SDL samples are QEMU frames. The buffer size will be
+     * the requested buffer size multiplied by the number of channels.
+     */
+    req.samples = audio_buffer_samples(
+        qapi_AudiodevSdlPerDirectionOptions_base(spdo), as, 11610);
     req.callback = sdl_callback;
     req.userdata = sdl;
 
@@ -301,7 +307,8 @@ static int sdl_init_out(HWVoiceOut *hw, struct audsettings *as,
     obt_as.endianness = endianness;
 
     audio_pcm_init_info (&hw->info, &obt_as);
-    hw->samples = obt.samples;
+    hw->samples = (spdo->has_buffer_count ? spdo->buffer_count : 4) *
+        obt.samples;
 
     s->initialized = 1;
     s->exit = 0;
diff --git a/qapi/audio.json b/qapi/audio.json
index 072ed79def..9cba0df8a4 100644
--- a/qapi/audio.json
+++ b/qapi/audio.json
@@ -301,6 +301,37 @@
     '*out':    'AudiodevPaPerDirectionOptions',
     '*server': 'str' } }
 
+##
+# @AudiodevSdlPerDirectionOptions:
+#
+# Options of the SDL audio backend that are used for both playback and
+# recording.
+#
+# @buffer-count: number of buffers (default 4)
+#
+# Since: 6.0
+##
+{ 'struct': 'AudiodevSdlPerDirectionOptions',
+  'base': 'AudiodevPerDirectionOptions',
+  'data': {
+    '*buffer-count': 'uint32' } }
+
+##
+# @AudiodevSdlOptions:
+#
+# Options of the SDL audio backend.
+#
+# @in: options of the recording stream
+#
+# @out: options of the playback stream
+#
+# Since: 6.0
+##
+{ 'struct': 'AudiodevSdlOptions',
+  'data': {
+    '*in':  'AudiodevSdlPerDirectionOptions',
+    '*out': 'AudiodevSdlPerDirectionOptions' } }
+
 ##
 # @AudiodevWavOptions:
 #
@@ -385,6 +416,6 @@
     'jack':      'AudiodevJackOptions',
     'oss':       'AudiodevOssOptions',
     'pa':        'AudiodevPaOptions',
-    'sdl':       'AudiodevGenericOptions',
+    'sdl':       'AudiodevSdlOptions',
     'spice':     'AudiodevGenericOptions',
     'wav':       'AudiodevWavOptions' } }
diff --git a/qemu-options.hx b/qemu-options.hx
index 1698a0c751..4e02e9bd76 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -588,6 +588,7 @@ DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev,
 #endif
 #ifdef CONFIG_AUDIO_SDL
     "-audiodev sdl,id=id[,prop[=value][,...]]\n"
+    "                in|out.buffer-count= number of buffers\n"
 #endif
 #ifdef CONFIG_SPICE
     "-audiodev spice,id=id[,prop[=value][,...]]\n"
@@ -745,7 +746,12 @@ SRST
 ``-audiodev sdl,id=id[,prop[=value][,...]]``
     Creates a backend using SDL. This backend is available on most
     systems, but you should use your platform's native backend if
-    possible. This backend has no backend specific properties.
+    possible.
+
+    SDL specific options are:
+
+    ``in|out.buffer-count=count``
+        Sets the count of the buffers.
 
 ``-audiodev spice,id=id[,prop[=value][,...]]``
     Creates a backend that sends audio through SPICE. This backend
-- 
2.26.2

[PATCH 04/23] sdlaudio: don't start playback in init routine

[Volker Rümelin]

Every emulated audio device has a way to enable audio playback. Don't
start playback until the guest enables the audio device. This patch
keeps the SDL2 device pause state in sync with hw->enabled.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/sdlaudio.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c
index 431bfcfddd..68126a99ab 100644
--- a/audio/sdlaudio.c
+++ b/audio/sdlaudio.c
@@ -312,7 +312,6 @@ static int sdl_init_out(HWVoiceOut *hw, struct audsettings *as,
 
     s->initialized = 1;
     s->exit = 0;
-    SDL_PauseAudio (0);
     return 0;
 }
 
-- 
2.26.2

[PATCH 05/23] sdlaudio: always clear the sample buffer

[Volker Rümelin]

Always fill the remaining audio callback buffer with silence.
SDL 2.0 doesn't initialize the audio callback buffer. This was
an incompatible change compared to SDL 1.2. For reference read
the SDL 1.2 to 2.0 migration guide.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/sdlaudio.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c
index 68126a99ab..79eed23849 100644
--- a/audio/sdlaudio.c
+++ b/audio/sdlaudio.c
@@ -211,27 +211,26 @@ static void sdl_callback (void *opaque, Uint8 *buf, int len)
     SDLAudioState *s = &glob_sdl;
     HWVoiceOut *hw = &sdl->hw;
 
-    if (s->exit) {
-        return;
-    }
+    if (!s->exit) {
 
-    /* dolog("callback: len=%d avail=%zu\n", len, hw->pending_emul); */
+        /* dolog("callback: len=%d avail=%zu\n", len, hw->pending_emul); */
 
-    while (hw->pending_emul && len) {
-        size_t write_len;
-        ssize_t start = ((ssize_t) hw->pos_emul) - hw->pending_emul;
-        if (start < 0) {
-            start += hw->size_emul;
-        }
-        assert(start >= 0 && start < hw->size_emul);
+        while (hw->pending_emul && len) {
+            size_t write_len;
+            ssize_t start = (ssize_t)hw->pos_emul - hw->pending_emul;
+            if (start < 0) {
+                start += hw->size_emul;
+            }
+            assert(start >= 0 && start < hw->size_emul);
 
-        write_len = MIN(MIN(hw->pending_emul, len),
-                        hw->size_emul - start);
+            write_len = MIN(MIN(hw->pending_emul, len),
+                            hw->size_emul - start);
 
-        memcpy(buf, hw->buf_emul + start, write_len);
-        hw->pending_emul -= write_len;
-        len -= write_len;
-        buf += write_len;
+            memcpy(buf, hw->buf_emul + start, write_len);
+            hw->pending_emul -= write_len;
+            len -= write_len;
+            buf += write_len;
+        }
     }
 
     /* clear remaining buffer that we couldn't fill with data */
-- 
2.26.2

[PATCH 06/23] sdlaudio: fill remaining sample buffer with silence

[Volker Rümelin]

Fill the remaining sample buffer with silence. To fill it with
zeroes is wrong for unsigned samples because this is silence
with a DC bias.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/sdlaudio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c
index 79eed23849..01ae4c600e 100644
--- a/audio/sdlaudio.c
+++ b/audio/sdlaudio.c
@@ -235,7 +235,8 @@ static void sdl_callback (void *opaque, Uint8 *buf, int len)
 
     /* clear remaining buffer that we couldn't fill with data */
     if (len) {
-        memset(buf, 0, len);
+        audio_pcm_info_clear_buf(&hw->info, buf,
+                                 len / hw->info.bytes_per_frame);
     }
 }
 
-- 
2.26.2

[PATCH 07/23] sdlaudio: replace legacy functions with modern ones

[Volker Rümelin]

With the modern audio functions it's possible to add new
features like audio recording.

As a side effect this patch fixes a bug where SDL2 can't be used
on Windows. This bug was reported on the qemu-devel mailing list at

https://lists.nongnu.org/archive/html/qemu-devel/2020-01/msg04043.html

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/sdlaudio.c | 107 ++++++++++++++++++++++-------------------------
 1 file changed, 50 insertions(+), 57 deletions(-)

diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c
index 01ae4c600e..47968c5020 100644
--- a/audio/sdlaudio.c
+++ b/audio/sdlaudio.c
@@ -41,15 +41,11 @@
 
 typedef struct SDLVoiceOut {
     HWVoiceOut hw;
-} SDLVoiceOut;
-
-static struct SDLAudioState {
     int exit;
     int initialized;
-    bool driver_created;
     Audiodev *dev;
-} glob_sdl;
-typedef struct SDLAudioState SDLAudioState;
+    SDL_AudioDeviceID devid;
+} SDLVoiceOut;
 
 static void GCC_FMT_ATTR (1, 2) sdl_logerr (const char *fmt, ...)
 {
@@ -155,9 +151,10 @@ static int sdl_to_audfmt(int sdlfmt, AudioFormat *fmt, int *endianness)
     return 0;
 }
 
-static int sdl_open (SDL_AudioSpec *req, SDL_AudioSpec *obt)
+static SDL_AudioDeviceID sdl_open(SDL_AudioSpec *req, SDL_AudioSpec *obt,
+                                  int rec)
 {
-    int status;
+    SDL_AudioDeviceID devid;
 #ifndef _WIN32
     int err;
     sigset_t new, old;
@@ -166,18 +163,19 @@ static int sdl_open (SDL_AudioSpec *req, SDL_AudioSpec *obt)
     err = sigfillset (&new);
     if (err) {
         dolog ("sdl_open: sigfillset failed: %s\n", strerror (errno));
-        return -1;
+        return 0;
     }
     err = pthread_sigmask (SIG_BLOCK, &new, &old);
     if (err) {
         dolog ("sdl_open: pthread_sigmask failed: %s\n", strerror (err));
-        return -1;
+        return 0;
     }
 #endif
 
-    status = SDL_OpenAudio (req, obt);
-    if (status) {
-        sdl_logerr ("SDL_OpenAudio failed\n");
+    devid = SDL_OpenAudioDevice(NULL, rec, req, obt, 0);
+    if (!devid) {
+        sdl_logerr("SDL_OpenAudioDevice for %s failed\n",
+                   rec ? "recording" : "playback");
     }
 
 #ifndef _WIN32
@@ -190,30 +188,32 @@ static int sdl_open (SDL_AudioSpec *req, SDL_AudioSpec *obt)
         exit (EXIT_FAILURE);
     }
 #endif
-    return status;
+    return devid;
 }
 
-static void sdl_close (SDLAudioState *s)
+static void sdl_close_out(SDLVoiceOut *sdl)
 {
-    if (s->initialized) {
-        SDL_LockAudio();
-        s->exit = 1;
-        SDL_UnlockAudio();
-        SDL_PauseAudio (1);
-        SDL_CloseAudio ();
-        s->initialized = 0;
+    if (sdl->initialized) {
+        SDL_LockAudioDevice(sdl->devid);
+        sdl->exit = 1;
+        SDL_UnlockAudioDevice(sdl->devid);
+        SDL_PauseAudioDevice(sdl->devid, 1);
+        sdl->initialized = 0;
+    }
+    if (sdl->devid) {
+        SDL_CloseAudioDevice(sdl->devid);
+        sdl->devid = 0;
     }
 }
 
-static void sdl_callback (void *opaque, Uint8 *buf, int len)
+static void sdl_callback_out(void *opaque, Uint8 *buf, int len)
 {
     SDLVoiceOut *sdl = opaque;
-    SDLAudioState *s = &glob_sdl;
     HWVoiceOut *hw = &sdl->hw;
 
-    if (!s->exit) {
+    if (!sdl->exit) {
 
-        /* dolog("callback: len=%d avail=%zu\n", len, hw->pending_emul); */
+        /* dolog("callback_out: len=%d avail=%zu\n", len, hw->pending_emul); */
 
         while (hw->pending_emul && len) {
             size_t write_len;
@@ -240,43 +240,44 @@ static void sdl_callback (void *opaque, Uint8 *buf, int len)
     }
 }
 
-#define SDL_WRAPPER_FUNC(name, ret_type, args_decl, args)      \
+#define SDL_WRAPPER_FUNC(name, ret_type, args_decl, args, dir) \
     static ret_type glue(sdl_, name)args_decl                  \
     {                                                          \
         ret_type ret;                                          \
+        glue(SDLVoice, dir) *sdl = (glue(SDLVoice, dir) *)hw;  \
                                                                \
-        SDL_LockAudio();                                       \
+        SDL_LockAudioDevice(sdl->devid);                       \
         ret = glue(audio_generic_, name)args;                  \
-        SDL_UnlockAudio();                                     \
+        SDL_UnlockAudioDevice(sdl->devid);                     \
                                                                \
         return ret;                                            \
     }
 
 SDL_WRAPPER_FUNC(get_buffer_out, void *, (HWVoiceOut *hw, size_t *size),
-                 (hw, size))
+                 (hw, size), Out)
 SDL_WRAPPER_FUNC(put_buffer_out, size_t,
-                 (HWVoiceOut *hw, void *buf, size_t size), (hw, buf, size))
+                 (HWVoiceOut *hw, void *buf, size_t size), (hw, buf, size), Out)
 SDL_WRAPPER_FUNC(write, size_t,
-                 (HWVoiceOut *hw, void *buf, size_t size), (hw, buf, size))
+                 (HWVoiceOut *hw, void *buf, size_t size), (hw, buf, size), Out)
 #undef SDL_WRAPPER_FUNC
 
-static void sdl_fini_out (HWVoiceOut *hw)
+static void sdl_fini_out(HWVoiceOut *hw)
 {
-    (void) hw;
+    SDLVoiceOut *sdl = (SDLVoiceOut *)hw;
 
-    sdl_close (&glob_sdl);
+    sdl_close_out(sdl);
 }
 
 static int sdl_init_out(HWVoiceOut *hw, struct audsettings *as,
                         void *drv_opaque)
 {
-    SDLVoiceOut *sdl = (SDLVoiceOut *) hw;
-    SDLAudioState *s = &glob_sdl;
+    SDLVoiceOut *sdl = (SDLVoiceOut *)hw;
     SDL_AudioSpec req, obt;
     int endianness;
     int err;
     AudioFormat effective_fmt;
-    AudiodevSdlPerDirectionOptions *spdo = s->dev->u.sdl.out;
+    Audiodev *dev = drv_opaque;
+    AudiodevSdlPerDirectionOptions *spdo = dev->u.sdl.out;
     struct audsettings obt_as;
 
     req.freq = as->freq;
@@ -288,16 +289,18 @@ static int sdl_init_out(HWVoiceOut *hw, struct audsettings *as,
      */
     req.samples = audio_buffer_samples(
         qapi_AudiodevSdlPerDirectionOptions_base(spdo), as, 11610);
-    req.callback = sdl_callback;
+    req.callback = sdl_callback_out;
     req.userdata = sdl;
 
-    if (sdl_open (&req, &obt)) {
+    sdl->dev = dev;
+    sdl->devid = sdl_open(&req, &obt, 0);
+    if (!sdl->devid) {
         return -1;
     }
 
     err = sdl_to_audfmt(obt.format, &effective_fmt, &endianness);
     if (err) {
-        sdl_close (s);
+        sdl_close_out(sdl);
         return -1;
     }
 
@@ -310,41 +313,31 @@ static int sdl_init_out(HWVoiceOut *hw, struct audsettings *as,
     hw->samples = (spdo->has_buffer_count ? spdo->buffer_count : 4) *
         obt.samples;
 
-    s->initialized = 1;
-    s->exit = 0;
+    sdl->initialized = 1;
+    sdl->exit = 0;
     return 0;
 }
 
 static void sdl_enable_out(HWVoiceOut *hw, bool enable)
 {
-    SDL_PauseAudio(!enable);
+    SDLVoiceOut *sdl = (SDLVoiceOut *)hw;
+
+    SDL_PauseAudioDevice(sdl->devid, !enable);
 }
 
 static void *sdl_audio_init(Audiodev *dev)
 {
-    SDLAudioState *s = &glob_sdl;
-    if (s->driver_created) {
-        sdl_logerr("Can't create multiple sdl backends\n");
-        return NULL;
-    }
-
     if (SDL_InitSubSystem (SDL_INIT_AUDIO)) {
         sdl_logerr ("SDL failed to initialize audio subsystem\n");
         return NULL;
     }
 
-    s->driver_created = true;
-    s->dev = dev;
-    return s;
+    return dev;
 }
 
 static void sdl_audio_fini (void *opaque)
 {
-    SDLAudioState *s = opaque;
-    sdl_close (s);
     SDL_QuitSubSystem (SDL_INIT_AUDIO);
-    s->driver_created = false;
-    s->dev = NULL;
 }
 
 static struct audio_pcm_ops sdl_pcm_ops = {
-- 
2.26.2

[PATCH 08/23] audio: split pcm_ops function get_buffer_in

[Volker Rümelin]

Split off pcm_ops function run_buffer_in from get_buffer_in and
call run_buffer_in before get_buffer_in.

The next patch only needs the generic buffer management part
from audio_generic_get_buffer_in().

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/alsaaudio.c  |  1 +
 audio/audio.c      | 18 ++++++++++++++----
 audio/audio_int.h  |  2 ++
 audio/jackaudio.c  |  1 +
 audio/noaudio.c    |  1 +
 audio/ossaudio.c   |  1 +
 audio/spiceaudio.c |  1 +
 7 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
index 6787e91bc1..5a871aaf6b 100644
--- a/audio/alsaaudio.c
+++ b/audio/alsaaudio.c
@@ -929,6 +929,7 @@ static struct audio_pcm_ops alsa_pcm_ops = {
     .init_in  = alsa_init_in,
     .fini_in  = alsa_fini_in,
     .read     = alsa_read,
+    .run_buffer_in = audio_generic_run_buffer_in,
     .enable_in = alsa_enable_in,
 };
 
diff --git a/audio/audio.c b/audio/audio.c
index d048d26283..480b3cce1f 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1241,6 +1241,10 @@ static size_t audio_pcm_hw_run_in(HWVoiceIn *hw, size_t samples)
     size_t conv = 0;
     STSampleBuffer *conv_buf = hw->conv_buf;
 
+    if (hw->pcm_ops->run_buffer_in) {
+        hw->pcm_ops->run_buffer_in(hw);
+    }
+
     while (samples) {
         size_t proc;
         size_t size = samples * hw->info.bytes_per_frame;
@@ -1381,10 +1385,8 @@ void audio_run(AudioState *s, const char *msg)
 #endif
 }
 
-void *audio_generic_get_buffer_in(HWVoiceIn *hw, size_t *size)
+void audio_generic_run_buffer_in(HWVoiceIn *hw)
 {
-    ssize_t start;
-
     if (unlikely(!hw->buf_emul)) {
         size_t calc_size = hw->conv_buf->size * hw->info.bytes_per_frame;
         hw->buf_emul = g_malloc(calc_size);
@@ -1403,8 +1405,12 @@ void *audio_generic_get_buffer_in(HWVoiceIn *hw, size_t *size)
             break;
         }
     }
+}
+
+void *audio_generic_get_buffer_in(HWVoiceIn *hw, size_t *size)
+{
+    ssize_t start = (ssize_t)hw->pos_emul - hw->pending_emul;
 
-    start = ((ssize_t) hw->pos_emul) - hw->pending_emul;
     if (start < 0) {
         start += hw->size_emul;
     }
@@ -1505,6 +1511,10 @@ size_t audio_generic_read(HWVoiceIn *hw, void *buf, size_t size)
 {
     size_t total = 0;
 
+    if (hw->pcm_ops->run_buffer_in) {
+        hw->pcm_ops->run_buffer_in(hw);
+    }
+
     while (total < size) {
         size_t src_size = size - total;
         void *src = hw->pcm_ops->get_buffer_in(hw, &src_size);
diff --git a/audio/audio_int.h b/audio/audio_int.h
index 4775857bf2..06f0913835 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -172,12 +172,14 @@ struct audio_pcm_ops {
     int    (*init_in) (HWVoiceIn *hw, audsettings *as, void *drv_opaque);
     void   (*fini_in) (HWVoiceIn *hw);
     size_t (*read)    (HWVoiceIn *hw, void *buf, size_t size);
+    void   (*run_buffer_in)(HWVoiceIn *hw);
     void  *(*get_buffer_in)(HWVoiceIn *hw, size_t *size);
     void   (*put_buffer_in)(HWVoiceIn *hw, void *buf, size_t size);
     void   (*enable_in)(HWVoiceIn *hw, bool enable);
     void   (*volume_in)(HWVoiceIn *hw, Volume *vol);
 };
 
+void audio_generic_run_buffer_in(HWVoiceIn *hw);
 void *audio_generic_get_buffer_in(HWVoiceIn *hw, size_t *size);
 void audio_generic_put_buffer_in(HWVoiceIn *hw, void *buf, size_t size);
 void audio_generic_run_buffer_out(HWVoiceOut *hw);
diff --git a/audio/jackaudio.c b/audio/jackaudio.c
index 3b7c18443d..f8afb5cc31 100644
--- a/audio/jackaudio.c
+++ b/audio/jackaudio.c
@@ -657,6 +657,7 @@ static struct audio_pcm_ops jack_pcm_ops = {
     .init_in        = qjack_init_in,
     .fini_in        = qjack_fini_in,
     .read           = qjack_read,
+    .run_buffer_in  = audio_generic_run_buffer_in,
     .enable_in      = qjack_enable_in
 };
 
diff --git a/audio/noaudio.c b/audio/noaudio.c
index 05798ea210..aac87dbc93 100644
--- a/audio/noaudio.c
+++ b/audio/noaudio.c
@@ -124,6 +124,7 @@ static struct audio_pcm_ops no_pcm_ops = {
     .init_in  = no_init_in,
     .fini_in  = no_fini_in,
     .read     = no_read,
+    .run_buffer_in = audio_generic_run_buffer_in,
     .enable_in = no_enable_in
 };
 
diff --git a/audio/ossaudio.c b/audio/ossaudio.c
index a7dcaa31ad..c1db89f233 100644
--- a/audio/ossaudio.c
+++ b/audio/ossaudio.c
@@ -762,6 +762,7 @@ static struct audio_pcm_ops oss_pcm_ops = {
     .init_in  = oss_init_in,
     .fini_in  = oss_fini_in,
     .read     = oss_read,
+    .run_buffer_in = audio_generic_run_buffer_in,
     .enable_in = oss_enable_in
 };
 
diff --git a/audio/spiceaudio.c b/audio/spiceaudio.c
index 8967cca129..999bfbde47 100644
--- a/audio/spiceaudio.c
+++ b/audio/spiceaudio.c
@@ -293,6 +293,7 @@ static struct audio_pcm_ops audio_callbacks = {
     .init_in  = line_in_init,
     .fini_in  = line_in_fini,
     .read     = line_in_read,
+    .run_buffer_in = audio_generic_run_buffer_in,
     .enable_in = line_in_enable,
 #if ((SPICE_INTERFACE_RECORD_MAJOR >= 2) && (SPICE_INTERFACE_RECORD_MINOR >= 2))
     .volume_in = line_in_volume,
-- 
2.26.2

[PATCH 09/23] sdlaudio: add recording functions

[Volker Rümelin]

Add audio recording functions. SDL 2.0.5 or later is required to
use the recording functions. Playback continues to work with
earlier SDL 2.0 versions.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/sdlaudio.c | 142 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 139 insertions(+), 3 deletions(-)

diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c
index 47968c5020..445cae8de5 100644
--- a/audio/sdlaudio.c
+++ b/audio/sdlaudio.c
@@ -47,6 +47,14 @@ typedef struct SDLVoiceOut {
     SDL_AudioDeviceID devid;
 } SDLVoiceOut;
 
+typedef struct SDLVoiceIn {
+    HWVoiceIn hw;
+    int exit;
+    int initialized;
+    Audiodev *dev;
+    SDL_AudioDeviceID devid;
+} SDLVoiceIn;
+
 static void GCC_FMT_ATTR (1, 2) sdl_logerr (const char *fmt, ...)
 {
     va_list ap;
@@ -240,6 +248,45 @@ static void sdl_callback_out(void *opaque, Uint8 *buf, int len)
     }
 }
 
+static void sdl_close_in(SDLVoiceIn *sdl)
+{
+    if (sdl->initialized) {
+        SDL_LockAudioDevice(sdl->devid);
+        sdl->exit = 1;
+        SDL_UnlockAudioDevice(sdl->devid);
+        SDL_PauseAudioDevice(sdl->devid, 1);
+        sdl->initialized = 0;
+    }
+    if (sdl->devid) {
+        SDL_CloseAudioDevice(sdl->devid);
+        sdl->devid = 0;
+    }
+}
+
+static void sdl_callback_in(void *opaque, Uint8 *buf, int len)
+{
+    SDLVoiceIn *sdl = opaque;
+    HWVoiceIn *hw = &sdl->hw;
+
+    if (sdl->exit) {
+        return;
+    }
+
+    /* dolog("callback_in: len=%d pending=%zu\n", len, hw->pending_emul); */
+
+    while (hw->pending_emul < hw->size_emul && len) {
+        size_t read_len = MIN(len, MIN(hw->size_emul - hw->pos_emul,
+                                       hw->size_emul - hw->pending_emul));
+
+        memcpy(hw->buf_emul + hw->pos_emul, buf, read_len);
+
+        hw->pending_emul += read_len;
+        hw->pos_emul = (hw->pos_emul + read_len) % hw->size_emul;
+        len -= read_len;
+        buf += read_len;
+    }
+}
+
 #define SDL_WRAPPER_FUNC(name, ret_type, args_decl, args, dir) \
     static ret_type glue(sdl_, name)args_decl                  \
     {                                                          \
@@ -253,13 +300,30 @@ static void sdl_callback_out(void *opaque, Uint8 *buf, int len)
         return ret;                                            \
     }
 
+#define SDL_WRAPPER_VOID_FUNC(name, args_decl, args, dir)      \
+    static void glue(sdl_, name)args_decl                      \
+    {                                                          \
+        glue(SDLVoice, dir) *sdl = (glue(SDLVoice, dir) *)hw;  \
+                                                               \
+        SDL_LockAudioDevice(sdl->devid);                       \
+        glue(audio_generic_, name)args;                        \
+        SDL_UnlockAudioDevice(sdl->devid);                     \
+    }
+
 SDL_WRAPPER_FUNC(get_buffer_out, void *, (HWVoiceOut *hw, size_t *size),
                  (hw, size), Out)
 SDL_WRAPPER_FUNC(put_buffer_out, size_t,
                  (HWVoiceOut *hw, void *buf, size_t size), (hw, buf, size), Out)
 SDL_WRAPPER_FUNC(write, size_t,
                  (HWVoiceOut *hw, void *buf, size_t size), (hw, buf, size), Out)
+SDL_WRAPPER_FUNC(read, size_t, (HWVoiceIn *hw, void *buf, size_t size),
+                 (hw, buf, size), In)
+SDL_WRAPPER_FUNC(get_buffer_in, void *, (HWVoiceIn *hw, size_t *size),
+                 (hw, size), In)
+SDL_WRAPPER_VOID_FUNC(put_buffer_in, (HWVoiceIn *hw, void *buf, size_t size),
+                      (hw, buf, size), In)
 #undef SDL_WRAPPER_FUNC
+#undef SDL_WRAPPER_VOID_FUNC
 
 static void sdl_fini_out(HWVoiceOut *hw)
 {
@@ -325,6 +389,69 @@ static void sdl_enable_out(HWVoiceOut *hw, bool enable)
     SDL_PauseAudioDevice(sdl->devid, !enable);
 }
 
+static void sdl_fini_in(HWVoiceIn *hw)
+{
+    SDLVoiceIn *sdl = (SDLVoiceIn *)hw;
+
+    sdl_close_in(sdl);
+}
+
+static int sdl_init_in(HWVoiceIn *hw, audsettings *as, void *drv_opaque)
+{
+    SDLVoiceIn *sdl = (SDLVoiceIn *)hw;
+    SDL_AudioSpec req, obt;
+    int endianness;
+    int err;
+    AudioFormat effective_fmt;
+    Audiodev *dev = drv_opaque;
+    AudiodevSdlPerDirectionOptions *spdo = dev->u.sdl.in;
+    struct audsettings obt_as;
+
+    req.freq = as->freq;
+    req.format = aud_to_sdlfmt(as->fmt);
+    req.channels = as->nchannels;
+    /* SDL samples are QEMU frames */
+    req.samples = audio_buffer_frames(
+        qapi_AudiodevSdlPerDirectionOptions_base(spdo), as, 11610);
+    req.callback = sdl_callback_in;
+    req.userdata = sdl;
+
+    sdl->dev = dev;
+    sdl->devid = sdl_open(&req, &obt, 1);
+    if (!sdl->devid) {
+        return -1;
+    }
+
+    err = sdl_to_audfmt(obt.format, &effective_fmt, &endianness);
+    if (err) {
+        sdl_close_in(sdl);
+        return -1;
+    }
+
+    obt_as.freq = obt.freq;
+    obt_as.nchannels = obt.channels;
+    obt_as.fmt = effective_fmt;
+    obt_as.endianness = endianness;
+
+    audio_pcm_init_info(&hw->info, &obt_as);
+    hw->samples = (spdo->has_buffer_count ? spdo->buffer_count : 4) *
+        obt.samples;
+    hw->size_emul = hw->samples * hw->info.bytes_per_frame;
+    hw->buf_emul = g_malloc(hw->size_emul);
+    hw->pos_emul = hw->pending_emul = 0;
+
+    sdl->initialized = 1;
+    sdl->exit = 0;
+    return 0;
+}
+
+static void sdl_enable_in(HWVoiceIn *hw, bool enable)
+{
+    SDLVoiceIn *sdl = (SDLVoiceIn *)hw;
+
+    SDL_PauseAudioDevice(sdl->devid, !enable);
+}
+
 static void *sdl_audio_init(Audiodev *dev)
 {
     if (SDL_InitSubSystem (SDL_INIT_AUDIO)) {
@@ -350,6 +477,15 @@ static struct audio_pcm_ops sdl_pcm_ops = {
   /* wrapper for audio_generic_put_buffer_out */
     .put_buffer_out = sdl_put_buffer_out,
     .enable_out = sdl_enable_out,
+    .init_in = sdl_init_in,
+    .fini_in = sdl_fini_in,
+  /* wrapper for audio_generic_read */
+    .read = sdl_read,
+  /* wrapper for audio_generic_get_buffer_in */
+    .get_buffer_in = sdl_get_buffer_in,
+  /* wrapper for audio_generic_put_buffer_in */
+    .put_buffer_in = sdl_put_buffer_in,
+    .enable_in = sdl_enable_in,
 };
 
 static struct audio_driver sdl_audio_driver = {
@@ -360,9 +496,9 @@ static struct audio_driver sdl_audio_driver = {
     .pcm_ops        = &sdl_pcm_ops,
     .can_be_default = 1,
     .max_voices_out = 1,
-    .max_voices_in  = 0,
-    .voice_size_out = sizeof (SDLVoiceOut),
-    .voice_size_in  = 0
+    .max_voices_in  = 1,
+    .voice_size_out = sizeof(SDLVoiceOut),
+    .voice_size_in  = sizeof(SDLVoiceIn),
 };
 
 static void register_audio_sdl(void)
-- 
2.26.2

[PATCH 10/23] audio: break generic buffer dependency on mixing-engine

[Volker Rümelin]

Break the unnecessary dependency of the generic buffer management
code on mixing-engine. This is required for the next patch.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/audio.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 480b3cce1f..22d769db0c 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1388,9 +1388,8 @@ void audio_run(AudioState *s, const char *msg)
 void audio_generic_run_buffer_in(HWVoiceIn *hw)
 {
     if (unlikely(!hw->buf_emul)) {
-        size_t calc_size = hw->conv_buf->size * hw->info.bytes_per_frame;
-        hw->buf_emul = g_malloc(calc_size);
-        hw->size_emul = calc_size;
+        hw->size_emul = hw->samples * hw->info.bytes_per_frame;
+        hw->buf_emul = g_malloc(hw->size_emul);
         hw->pos_emul = hw->pending_emul = 0;
     }
 
@@ -1452,10 +1451,8 @@ void audio_generic_run_buffer_out(HWVoiceOut *hw)
 void *audio_generic_get_buffer_out(HWVoiceOut *hw, size_t *size)
 {
     if (unlikely(!hw->buf_emul)) {
-        size_t calc_size = hw->mix_buf->size * hw->info.bytes_per_frame;
-
-        hw->buf_emul = g_malloc(calc_size);
-        hw->size_emul = calc_size;
+        hw->size_emul = hw->samples * hw->info.bytes_per_frame;
+        hw->buf_emul = g_malloc(hw->size_emul);
         hw->pos_emul = hw->pending_emul = 0;
     }
 
-- 
2.26.2

[PATCH 11/23] sdlaudio: enable (in|out).mixing-engine=off

[Volker Rümelin]

Enable the SDL2 backend options -audiodev sdl,out.mixing-
engine=off,in.mixing-engine=off.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/sdlaudio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c
index 445cae8de5..c68c62a3e4 100644
--- a/audio/sdlaudio.c
+++ b/audio/sdlaudio.c
@@ -495,8 +495,8 @@ static struct audio_driver sdl_audio_driver = {
     .fini           = sdl_audio_fini,
     .pcm_ops        = &sdl_pcm_ops,
     .can_be_default = 1,
-    .max_voices_out = 1,
-    .max_voices_in  = 1,
+    .max_voices_out = INT_MAX,
+    .max_voices_in  = INT_MAX,
     .voice_size_out = sizeof(SDLVoiceOut),
     .voice_size_in  = sizeof(SDLVoiceIn),
 };
-- 
2.26.2

[PATCH 12/23] audio: remove remaining unused plive code

[Volker Rümelin]

Commit 73ad33ef7b "audio: remove plive" forgot to remove this code.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/audio.c | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 22d769db0c..34c9cb9182 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1132,7 +1132,7 @@ static void audio_run_out (AudioState *s)
 
     while ((hw = audio_pcm_hw_find_any_enabled_out(s, hw))) {
         size_t played, live, prev_rpos, free;
-        int nb_live, cleanup_required;
+        int nb_live;
 
         live = audio_pcm_hw_get_live_out (hw, &nb_live);
         if (!nb_live) {
@@ -1194,7 +1194,6 @@ static void audio_run_out (AudioState *s)
             audio_capture_mix_and_clear (hw, prev_rpos, played);
         }
 
-        cleanup_required = 0;
         for (sw = hw->sw_head.lh_first; sw; sw = sw->entries.le_next) {
             if (!sw->active && sw->empty) {
                 continue;
@@ -1210,7 +1209,6 @@ static void audio_run_out (AudioState *s)
 
             if (!sw->total_hw_samples_mixed) {
                 sw->empty = 1;
-                cleanup_required |= !sw->active && !sw->callback.fn;
             }
 
             if (sw->active) {
@@ -1220,19 +1218,6 @@ static void audio_run_out (AudioState *s)
                 }
             }
         }
-
-        if (cleanup_required) {
-            SWVoiceOut *sw1;
-
-            sw = hw->sw_head.lh_first;
-            while (sw) {
-                sw1 = sw->entries.le_next;
-                if (!sw->active && !sw->callback.fn) {
-                    audio_close_out (sw);
-                }
-                sw = sw1;
-            }
-        }
     }
 }
 
-- 
2.26.2

[PATCH 13/23] paaudio: avoid to clip samples multiple times

[Volker Rümelin]

The pulseaudio backend currently converts, clips and copies audio
playback samples in the mixing-engine sample buffer multiple
times.

In qpa_get_buffer_out() the function pa_stream_begin_write()
returns a rather large buffer and this allows audio_pcm_hw_run_out()
in audio/audio.c to copy all samples in the mixing-engine buffer
to the pulse audio buffer. Immediately after copying, qpa_write()
notices with a call to pa_stream_writable_size() that pulse audio
only needs a smaller part of the copied samples and ignores the
rest. This copy and ignore process happens several times for each
audio sample.

To fix this behaviour, call pa_stream_writable_size() in
qpa_get_buffer_out() to limit the number of samples
audio_pcm_hw_run_out() will convert. With this change the
pulseaudio pcm_ops functions put_buffer_out and write are no
longer identical and a separate qpa_put_buffer_out is needed.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/paaudio.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/audio/paaudio.c b/audio/paaudio.c
index b052084698..229bcfcae8 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -207,6 +207,7 @@ static void *qpa_get_buffer_out(HWVoiceOut *hw, size_t *size)
     PAVoiceOut *p = (PAVoiceOut *) hw;
     PAConnection *c = p->g->conn;
     void *ret;
+    size_t l;
     int r;
 
     pa_threaded_mainloop_lock(c->mainloop);
@@ -214,12 +215,19 @@ static void *qpa_get_buffer_out(HWVoiceOut *hw, size_t *size)
     CHECK_DEAD_GOTO(c, p->stream, unlock_and_fail,
                     "pa_threaded_mainloop_lock failed\n");
 
+    l = pa_stream_writable_size(p->stream);
+    CHECK_SUCCESS_GOTO(c, l != (size_t) -1, unlock_and_fail,
+                       "pa_stream_writable_size failed\n");
+
     *size = -1;
     r = pa_stream_begin_write(p->stream, &ret, size);
     CHECK_SUCCESS_GOTO(c, r >= 0, unlock_and_fail,
                        "pa_stream_begin_write failed\n");
 
     pa_threaded_mainloop_unlock(c->mainloop);
+    if (*size > l) {
+        *size = l;
+    }
     return ret;
 
 unlock_and_fail:
@@ -228,6 +236,28 @@ unlock_and_fail:
     return NULL;
 }
 
+static size_t qpa_put_buffer_out(HWVoiceOut *hw, void *data, size_t length)
+{
+    PAVoiceOut *p = (PAVoiceOut *)hw;
+    PAConnection *c = p->g->conn;
+    int r;
+
+    pa_threaded_mainloop_lock(c->mainloop);
+
+    CHECK_DEAD_GOTO(c, p->stream, unlock_and_fail,
+                    "pa_threaded_mainloop_lock failed\n");
+
+    r = pa_stream_write(p->stream, data, length, NULL, 0LL, PA_SEEK_RELATIVE);
+    CHECK_SUCCESS_GOTO(c, r >= 0, unlock_and_fail, "pa_stream_write failed\n");
+
+    pa_threaded_mainloop_unlock(c->mainloop);
+    return length;
+
+unlock_and_fail:
+    pa_threaded_mainloop_unlock(c->mainloop);
+    return 0;
+}
+
 static size_t qpa_write(HWVoiceOut *hw, void *data, size_t length)
 {
     PAVoiceOut *p = (PAVoiceOut *) hw;
@@ -861,7 +891,7 @@ static struct audio_pcm_ops qpa_pcm_ops = {
     .fini_out = qpa_fini_out,
     .write    = qpa_write,
     .get_buffer_out = qpa_get_buffer_out,
-    .put_buffer_out = qpa_write, /* pa handles it */
+    .put_buffer_out = qpa_put_buffer_out,
     .volume_out = qpa_volume_out,
 
     .init_in  = qpa_init_in,
-- 
2.26.2

[PATCH 14/23] paaudio: wait for PA_STREAM_READY in qpa_write()

[Volker Rümelin]

Don't call pa_stream_writable_size() in qpa_write() before the
playback stream is ready. This prevents a lot of the following
pulseaudio error messages.

pulseaudio: pa_stream_writable_size failed
pulseaudio: Reason: Bad state

To reproduce start qemu with
-parallel none -device gus,audiodev=audio0
-audiodev pa,id=audio0,out.mixing-engine=off

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/paaudio.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/audio/paaudio.c b/audio/paaudio.c
index 229bcfcae8..1a7252b16d 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -269,6 +269,11 @@ static size_t qpa_write(HWVoiceOut *hw, void *data, size_t length)
 
     CHECK_DEAD_GOTO(c, p->stream, unlock_and_fail,
                     "pa_threaded_mainloop_lock failed\n");
+    if (pa_stream_get_state(p->stream) != PA_STREAM_READY) {
+        /* wait for stream to become ready */
+        l = 0;
+        goto unlock;
+    }
 
     l = pa_stream_writable_size(p->stream);
 
@@ -282,6 +287,7 @@ static size_t qpa_write(HWVoiceOut *hw, void *data, size_t length)
     r = pa_stream_write(p->stream, data, l, NULL, 0LL, PA_SEEK_RELATIVE);
     CHECK_SUCCESS_GOTO(c, r >= 0, unlock_and_fail, "pa_stream_write failed\n");
 
+unlock:
     pa_threaded_mainloop_unlock(c->mainloop);
     return l;
 
-- 
2.26.2

[PATCH 15/23] paaudio: wait until the playback stream is ready

[Volker Rümelin]

Don't call pa_stream_writable_size() in qpa_get_buffer_out()
before the playback stream is ready. This prevents a lot of the
following pulseaudio error messages.

pulseaudio: pa_stream_writable_size failed
pulseaudio: Reason: Bad state

To reproduce start qemu with
-parallel none -device gus,audiodev=audio0 -audiodev pa,id=audio0

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/paaudio.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/audio/paaudio.c b/audio/paaudio.c
index 1a7252b16d..4a1ffda753 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -214,6 +214,12 @@ static void *qpa_get_buffer_out(HWVoiceOut *hw, size_t *size)
 
     CHECK_DEAD_GOTO(c, p->stream, unlock_and_fail,
                     "pa_threaded_mainloop_lock failed\n");
+    if (pa_stream_get_state(p->stream) != PA_STREAM_READY) {
+        /* wait for stream to become ready */
+        l = 0;
+        ret = NULL;
+        goto unlock;
+    }
 
     l = pa_stream_writable_size(p->stream);
     CHECK_SUCCESS_GOTO(c, l != (size_t) -1, unlock_and_fail,
@@ -224,6 +230,7 @@ static void *qpa_get_buffer_out(HWVoiceOut *hw, size_t *size)
     CHECK_SUCCESS_GOTO(c, r >= 0, unlock_and_fail,
                        "pa_stream_begin_write failed\n");
 
+unlock:
     pa_threaded_mainloop_unlock(c->mainloop);
     if (*size > l) {
         *size = l;
-- 
2.26.2

[PATCH 16/23] paaudio: remove unneeded code

[Volker Rümelin]

Commit baea032ec7 "audio/paaudio: fix ignored buffer_length setting"
added code to handle buffer_length defaults. This was unnecessary
because the audio_buffer_* functions in audio/audio.c already handle
this. Remove the unneeded code.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/paaudio.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/audio/paaudio.c b/audio/paaudio.c
index 4a1ffda753..86038f3e13 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -539,8 +539,7 @@ static int qpa_init_out(HWVoiceOut *hw, struct audsettings *as,
 
     audio_pcm_init_info (&hw->info, &obt_as);
     hw->samples = audio_buffer_samples(
-        qapi_AudiodevPaPerDirectionOptions_base(ppdo),
-        &obt_as, ppdo->buffer_length);
+        qapi_AudiodevPaPerDirectionOptions_base(ppdo), &obt_as, 46440);
 
     return 0;
 
@@ -587,8 +586,7 @@ static int qpa_init_in(HWVoiceIn *hw, struct audsettings *as, void *drv_opaque)
 
     audio_pcm_init_info (&hw->info, &obt_as);
     hw->samples = audio_buffer_samples(
-        qapi_AudiodevPaPerDirectionOptions_base(ppdo),
-        &obt_as, ppdo->buffer_length);
+        qapi_AudiodevPaPerDirectionOptions_base(ppdo), &obt_as, 46440);
 
     return 0;
 
@@ -738,10 +736,6 @@ static void qpa_volume_in(HWVoiceIn *hw, Volume *vol)
 static int qpa_validate_per_direction_opts(Audiodev *dev,
                                            AudiodevPaPerDirectionOptions *pdo)
 {
-    if (!pdo->has_buffer_length) {
-        pdo->has_buffer_length = true;
-        pdo->buffer_length = 46440;
-    }
     if (!pdo->has_latency) {
         pdo->has_latency = true;
         pdo->latency = 15000;
-- 
2.26.2

[PATCH 17/23] paaudio: comment bugs in functions qpa_init_*

[Volker Rümelin]

The audio buffer size in audio/paaudio.c is typically larger
than expected. Just comment the bugs in qpa_init_in() and
qpa_init_out() for now. Fixing these bugs may break glitch free
audio playback with fine tuned user audio settings.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/paaudio.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/audio/paaudio.c b/audio/paaudio.c
index 86038f3e13..ff3dd01c96 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -538,6 +538,10 @@ static int qpa_init_out(HWVoiceOut *hw, struct audsettings *as,
     }
 
     audio_pcm_init_info (&hw->info, &obt_as);
+    /*
+     * This is wrong. hw->samples counts in frames. hw->samples will be
+     * number of channels times larger than expected.
+     */
     hw->samples = audio_buffer_samples(
         qapi_AudiodevPaPerDirectionOptions_base(ppdo), &obt_as, 46440);
 
@@ -585,6 +589,10 @@ static int qpa_init_in(HWVoiceIn *hw, struct audsettings *as, void *drv_opaque)
     }
 
     audio_pcm_init_info (&hw->info, &obt_as);
+    /*
+     * This is wrong. hw->samples counts in frames. hw->samples will be
+     * number of channels times larger than expected.
+     */
     hw->samples = audio_buffer_samples(
         qapi_AudiodevPaPerDirectionOptions_base(ppdo), &obt_as, 46440);
 
-- 
2.26.2

[PATCH 18/23] paaudio: limit minreq to 75% of audio timer_rate

[Volker Rümelin]

Currently with the playback buffer attribute minreq = -1 and flag
PA_STREAM_EARLY_REQUESTS PulseAudio uses minreq = tlength / 4.
To improve audio playback with larger PulseAudio server side
buffers, limit minreq to a maximum of 75% of audio timer_rate.
That way there is a good chance qemu receives a stream buffer
size update before it tries to write data to the playback stream.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/paaudio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/audio/paaudio.c b/audio/paaudio.c
index ff3dd01c96..3186868294 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -517,7 +517,8 @@ static int qpa_init_out(HWVoiceOut *hw, struct audsettings *as,
     ss.rate = as->freq;
 
     ba.tlength = pa_usec_to_bytes(ppdo->latency, &ss);
-    ba.minreq = -1;
+    ba.minreq = pa_usec_to_bytes(MIN(ppdo->latency >> 2,
+                                     (g->dev->timer_period >> 2) * 3), &ss);
     ba.maxlength = -1;
     ba.prebuf = -1;
 
-- 
2.26.2

[PATCH 19/23] paaudio: send recorded data in smaller chunks

[Volker Rümelin]

Tell PulseAudio to send recorded audio data in smaller chunks
than timer_period, so there's a good chance that qemu can read
recorded audio data every time it looks for new data.

PulseAudio tries to send buffer updates at a fragsize / 2 rate.
With fragsize = timer_period / 2 * 3 the update rate is 75% of
timer_period. The lower limit for the recording buffer size
maxlength is fragsize * 2.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/paaudio.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/audio/paaudio.c b/audio/paaudio.c
index 3186868294..1e6f4448ce 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -568,8 +568,9 @@ static int qpa_init_in(HWVoiceIn *hw, struct audsettings *as, void *drv_opaque)
     ss.channels = as->nchannels;
     ss.rate = as->freq;
 
-    ba.fragsize = pa_usec_to_bytes(ppdo->latency, &ss);
-    ba.maxlength = pa_usec_to_bytes(ppdo->latency * 2, &ss);
+    ba.fragsize = pa_usec_to_bytes((g->dev->timer_period >> 1) * 3, &ss);
+    ba.maxlength = pa_usec_to_bytes(
+        MAX(ppdo->latency, g->dev->timer_period * 3), &ss);
     ba.minreq = -1;
     ba.prebuf = -1;
 
-- 
2.26.2

[PATCH 20/23] dsoundaudio: replace GetForegroundWindow()

[Volker Rümelin]

GetForegroundWindow() doesn't necessarily return the own window
handle. It just returns a handle to the currently active window
and can even return NULL. At the time dsound_open() gets called
the active window is most likely the shell window and not the
QEMU window.

Replace GetForegroundWindow() with GetDesktopWindow() which
always returns a valid window handle, and at the same time
replace the DirectSound buffer flag DSBCAPS_STICKYFOCUS with
DSBCAPS_GLOBALFOCUS where Windows only expects a valid window
handle for DirectSound function SetCooperativeLevel(). The
Microsoft online docs for IDirectSound::SetCooperativeLevel
recommend this in the remarks.

This fixes a bug where you can't hear sound from the guest.

To reproduce start qemu with -machine pcspk-audiodev=audio0
-device intel-hda -device hda-duplex,audiodev=audio0
-audiodev dsound,id=audio0,out.mixing-engine=off
from a shell and start audio playback with the hda device in the
guest. The guest will be silent. To hear guest audio you have to
activate the shell window once.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/dsound_template.h | 2 +-
 audio/dsoundaudio.c     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/audio/dsound_template.h b/audio/dsound_template.h
index 9c5ce625ab..0678f2de38 100644
--- a/audio/dsound_template.h
+++ b/audio/dsound_template.h
@@ -205,7 +205,7 @@ static int dsound_init_out(HWVoiceOut *hw, struct audsettings *as,
         NULL
         );
 #else
-    bd.dwFlags = DSBCAPS_STICKYFOCUS | DSBCAPS_GETCURRENTPOSITION2;
+    bd.dwFlags = DSBCAPS_GLOBALFOCUS | DSBCAPS_GETCURRENTPOSITION2;
     hr = IDirectSound_CreateSoundBuffer (
         s->dsound,
         &bd,
diff --git a/audio/dsoundaudio.c b/audio/dsoundaudio.c
index 4cdf19ab67..0fbdf770ac 100644
--- a/audio/dsoundaudio.c
+++ b/audio/dsoundaudio.c
@@ -347,7 +347,7 @@ static int dsound_open (dsound *s)
     HRESULT hr;
     HWND hwnd;
 
-    hwnd = GetForegroundWindow ();
+    hwnd = GetDesktopWindow();
     hr = IDirectSound_SetCooperativeLevel (
         s->dsound,
         hwnd,
-- 
2.26.2

[PATCH 21/23] dsoundaudio: rename dsound_open()

[Volker Rümelin]

Rename dsound_open() to dsound_set_cooperative_level(). The
only task of that function is to set the cooperative level for
DirectSound.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/dsoundaudio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/audio/dsoundaudio.c b/audio/dsoundaudio.c
index 0fbdf770ac..d3695f3af6 100644
--- a/audio/dsoundaudio.c
+++ b/audio/dsoundaudio.c
@@ -342,7 +342,7 @@ static void dsound_clear_sample (HWVoiceOut *hw, LPDIRECTSOUNDBUFFER dsb,
     dsound_unlock_out (dsb, p1, p2, blen1, blen2);
 }
 
-static int dsound_open (dsound *s)
+static int dsound_set_cooperative_level(dsound *s)
 {
     HRESULT hr;
     HWND hwnd;
@@ -673,7 +673,7 @@ static void *dsound_audio_init(Audiodev *dev)
         }
     }
 
-    err = dsound_open (s);
+    err = dsound_set_cooperative_level(s);
     if (err) {
         dsound_audio_fini (s);
         return NULL;
-- 
2.26.2

[PATCH 22/23] dsoundaudio: enable f32 audio sample format

[Volker Rümelin]

Enable the f32 audio sample format for the DirectSound backend.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/audio_win_int.c | 71 ++++++++++++++++++++++++++++---------------
 1 file changed, 47 insertions(+), 24 deletions(-)

diff --git a/audio/audio_win_int.c b/audio/audio_win_int.c
index b938fd667b..b7db34900c 100644
--- a/audio/audio_win_int.c
+++ b/audio/audio_win_int.c
@@ -5,6 +5,7 @@
 
 #define AUDIO_CAP "win-int"
 #include <windows.h>
+#include <mmreg.h>
 #include <mmsystem.h>
 
 #include "audio.h"
@@ -16,7 +17,6 @@ int waveformat_from_audio_settings (WAVEFORMATEX *wfx,
 {
     memset (wfx, 0, sizeof (*wfx));
 
-    wfx->wFormatTag = WAVE_FORMAT_PCM;
     wfx->nChannels = as->nchannels;
     wfx->nSamplesPerSec = as->freq;
     wfx->nAvgBytesPerSec = as->freq << (as->nchannels == 2);
@@ -26,11 +26,13 @@ int waveformat_from_audio_settings (WAVEFORMATEX *wfx,
     switch (as->fmt) {
     case AUDIO_FORMAT_S8:
     case AUDIO_FORMAT_U8:
+        wfx->wFormatTag = WAVE_FORMAT_PCM;
         wfx->wBitsPerSample = 8;
         break;
 
     case AUDIO_FORMAT_S16:
     case AUDIO_FORMAT_U16:
+        wfx->wFormatTag = WAVE_FORMAT_PCM;
         wfx->wBitsPerSample = 16;
         wfx->nAvgBytesPerSec <<= 1;
         wfx->nBlockAlign <<= 1;
@@ -38,6 +40,14 @@ int waveformat_from_audio_settings (WAVEFORMATEX *wfx,
 
     case AUDIO_FORMAT_S32:
     case AUDIO_FORMAT_U32:
+        wfx->wFormatTag = WAVE_FORMAT_PCM;
+        wfx->wBitsPerSample = 32;
+        wfx->nAvgBytesPerSec <<= 2;
+        wfx->nBlockAlign <<= 2;
+        break;
+
+    case AUDIO_FORMAT_F32:
+        wfx->wFormatTag = WAVE_FORMAT_IEEE_FLOAT;
         wfx->wBitsPerSample = 32;
         wfx->nAvgBytesPerSec <<= 2;
         wfx->nBlockAlign <<= 2;
@@ -54,12 +64,6 @@ int waveformat_from_audio_settings (WAVEFORMATEX *wfx,
 int waveformat_to_audio_settings (WAVEFORMATEX *wfx,
                                   struct audsettings *as)
 {
-    if (wfx->wFormatTag != WAVE_FORMAT_PCM) {
-        dolog ("Invalid wave format, tag is not PCM, but %d\n",
-               wfx->wFormatTag);
-        return -1;
-    }
-
     if (!wfx->nSamplesPerSec) {
         dolog ("Invalid wave format, frequency is zero\n");
         return -1;
@@ -83,23 +87,42 @@ int waveformat_to_audio_settings (WAVEFORMATEX *wfx,
         return -1;
     }
 
-    switch (wfx->wBitsPerSample) {
-    case 8:
-        as->fmt = AUDIO_FORMAT_U8;
-        break;
-
-    case 16:
-        as->fmt = AUDIO_FORMAT_S16;
-        break;
-
-    case 32:
-        as->fmt = AUDIO_FORMAT_S32;
-        break;
-
-    default:
-        dolog ("Invalid wave format, bits per sample is not "
-               "8, 16 or 32, but %d\n",
-               wfx->wBitsPerSample);
+    if (wfx->wFormatTag == WAVE_FORMAT_PCM) {
+        switch (wfx->wBitsPerSample) {
+        case 8:
+            as->fmt = AUDIO_FORMAT_U8;
+            break;
+
+        case 16:
+            as->fmt = AUDIO_FORMAT_S16;
+            break;
+
+        case 32:
+            as->fmt = AUDIO_FORMAT_S32;
+            break;
+
+        default:
+            dolog("Invalid PCM wave format, bits per sample is not "
+                  "8, 16 or 32, but %d\n",
+                  wfx->wBitsPerSample);
+            return -1;
+        }
+    } else if (wfx->wFormatTag == WAVE_FORMAT_IEEE_FLOAT) {
+        switch (wfx->wBitsPerSample) {
+        case 32:
+            as->fmt = AUDIO_FORMAT_F32;
+            break;
+
+        default:
+            dolog("Invalid IEEE_FLOAT wave format, bits per sample is not "
+                  "32, but %d\n",
+                  wfx->wBitsPerSample);
+            return -1;
+        }
+    } else {
+        dolog("Invalid wave format, tag is not PCM and not IEEE_FLOAT, "
+              "but %d\n",
+              wfx->wFormatTag);
         return -1;
     }
 
-- 
2.26.2

[PATCH 23/23] dsoundaudio: fix log message

[Volker Rümelin]

There is a mismatch between message and used argument. Change
the argument from frequency to format.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/audio_win_int.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/audio/audio_win_int.c b/audio/audio_win_int.c
index b7db34900c..5ea8157dfc 100644
--- a/audio/audio_win_int.c
+++ b/audio/audio_win_int.c
@@ -54,7 +54,7 @@ int waveformat_from_audio_settings (WAVEFORMATEX *wfx,
         break;
 
     default:
-        dolog ("Internal logic error: Bad audio format %d\n", as->freq);
+        dolog("Internal logic error: Bad audio format %d\n", as->fmt);
         return -1;
     }
 
-- 
2.26.2

Re: [PATCH 00/23] next round of audio patches

[no-reply]

Patchew URL: https://patchew.org/QEMU/9315afe5-5958-c0b4-ea1e-14769511a9d5@t-online.de/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 9315afe5-5958-c0b4-ea1e-14769511a9d5@t-online.de
Subject: [PATCH 00/23] next round of audio patches

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/9315afe5-5958-c0b4-ea1e-14769511a9d5@t-online.de -> patchew/9315afe5-5958-c0b4-ea1e-14769511a9d5@t-online.de
Switched to a new branch 'test'
f5676b9 dsoundaudio: fix log message
825e3ad dsoundaudio: enable f32 audio sample format
ead5e23 dsoundaudio: rename dsound_open()
3bf4a8e dsoundaudio: replace GetForegroundWindow()
98454ba paaudio: send recorded data in smaller chunks
2f94e48 paaudio: limit minreq to 75% of audio timer_rate
4fd4d92 paaudio: comment bugs in functions qpa_init_*
1045ac7 paaudio: remove unneeded code
d5c8eb1 paaudio: wait until the playback stream is ready
11d1092 paaudio: wait for PA_STREAM_READY in qpa_write()
6cc0dee paaudio: avoid to clip samples multiple times
d670342 audio: remove remaining unused plive code
0cc736d sdlaudio: enable (in|out).mixing-engine=off
61543ec audio: break generic buffer dependency on mixing-engine
330dfbe sdlaudio: add recording functions
998b92f audio: split pcm_ops function get_buffer_in
bc84416 sdlaudio: replace legacy functions with modern ones
8f33798 sdlaudio: fill remaining sample buffer with silence
78c2474 sdlaudio: always clear the sample buffer
f5ea854 sdlaudio: don't start playback in init routine
7a1a2df sdlaudio: add -audiodev sdl,out.buffer-count option
6aa7760 audio: fix bit-rotted code
98cf04b sdlaudio: remove leftover SDL1.2 code

=== OUTPUT BEGIN ===
1/23 Checking commit 98cf04b7c44a (sdlaudio: remove leftover SDL1.2 code)
2/23 Checking commit 6aa776039e75 (audio: fix bit-rotted code)
3/23 Checking commit 7a1a2df1c97f (sdlaudio: add -audiodev sdl,out.buffer-count option)
4/23 Checking commit f5ea85493feb (sdlaudio: don't start playback in init routine)
5/23 Checking commit 78c2474549af (sdlaudio: always clear the sample buffer)
6/23 Checking commit 8f33798c4ff8 (sdlaudio: fill remaining sample buffer with silence)
7/23 Checking commit bc844166b227 (sdlaudio: replace legacy functions with modern ones)
ERROR: spaces required around that '*' (ctx:WxV)
#133: FILE: audio/sdlaudio.c:247:
+        glue(SDLVoice, dir) *sdl = (glue(SDLVoice, dir) *)hw;  \
                             ^

ERROR: spaces required around that '*' (ctx:WxB)
#133: FILE: audio/sdlaudio.c:247:
+        glue(SDLVoice, dir) *sdl = (glue(SDLVoice, dir) *)hw;  \
                                                         ^

total: 2 errors, 0 warnings, 222 lines checked

Patch 7/23 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

8/23 Checking commit 998b92fd32ff (audio: split pcm_ops function get_buffer_in)
9/23 Checking commit 330dfbed90b4 (sdlaudio: add recording functions)
ERROR: spaces required around that '*' (ctx:WxV)
#86: FILE: audio/sdlaudio.c:306:
+        glue(SDLVoice, dir) *sdl = (glue(SDLVoice, dir) *)hw;  \
                             ^

ERROR: spaces required around that '*' (ctx:WxB)
#86: FILE: audio/sdlaudio.c:306:
+        glue(SDLVoice, dir) *sdl = (glue(SDLVoice, dir) *)hw;  \
                                                         ^

total: 2 errors, 0 warnings, 185 lines checked

Patch 9/23 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

10/23 Checking commit 61543ec830f5 (audio: break generic buffer dependency on mixing-engine)
11/23 Checking commit 0cc736d525a7 (sdlaudio: enable (in|out).mixing-engine=off)
12/23 Checking commit d6703422e706 (audio: remove remaining unused plive code)
13/23 Checking commit 6cc0dee46213 (paaudio: avoid to clip samples multiple times)
14/23 Checking commit 11d109269391 (paaudio: wait for PA_STREAM_READY in qpa_write())
15/23 Checking commit d5c8eb16d112 (paaudio: wait until the playback stream is ready)
16/23 Checking commit 1045ac7440af (paaudio: remove unneeded code)
17/23 Checking commit 4fd4d92e5788 (paaudio: comment bugs in functions qpa_init_*)
18/23 Checking commit 2f94e489468a (paaudio: limit minreq to 75% of audio timer_rate)
19/23 Checking commit 98454ba25082 (paaudio: send recorded data in smaller chunks)
20/23 Checking commit 3bf4a8e55ba4 (dsoundaudio: replace GetForegroundWindow())
21/23 Checking commit ead5e23b8560 (dsoundaudio: rename dsound_open())
22/23 Checking commit 825e3ade19c8 (dsoundaudio: enable f32 audio sample format)
23/23 Checking commit f5676b9a646c (dsoundaudio: fix log message)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/9315afe5-5958-c0b4-ea1e-14769511a9d5@t-online.de/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH 00/23] next round of audio patches

[Volker Rümelin]

> Patchew URL: https://patchew.org/QEMU/9315afe5-5958-c0b4-ea1e-14769511a9d5@t-online.de/
>
>
>
> Hi,
>
> This series seems to have some coding style problems. See output below for
> more information:
>
> Type: series
> Message-id: 9315afe5-5958-c0b4-ea1e-14769511a9d5@t-online.de
> Subject: [PATCH 00/23] next round of audio patches
>
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> git rev-parse base > /dev/null || exit 0
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> git config --local diff.algorithm histogram
> ./scripts/checkpatch.pl --mailback base..
> === TEST SCRIPT END ===
>
> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> From https://github.com/patchew-project/qemu
>  * [new tag]         patchew/9315afe5-5958-c0b4-ea1e-14769511a9d5@t-online.de -> patchew/9315afe5-5958-c0b4-ea1e-14769511a9d5@t-online.de
> Switched to a new branch 'test'
> f5676b9 dsoundaudio: fix log message
> 825e3ad dsoundaudio: enable f32 audio sample format
> ead5e23 dsoundaudio: rename dsound_open()
> 3bf4a8e dsoundaudio: replace GetForegroundWindow()
> 98454ba paaudio: send recorded data in smaller chunks
> 2f94e48 paaudio: limit minreq to 75% of audio timer_rate
> 4fd4d92 paaudio: comment bugs in functions qpa_init_*
> 1045ac7 paaudio: remove unneeded code
> d5c8eb1 paaudio: wait until the playback stream is ready
> 11d1092 paaudio: wait for PA_STREAM_READY in qpa_write()
> 6cc0dee paaudio: avoid to clip samples multiple times
> d670342 audio: remove remaining unused plive code
> 0cc736d sdlaudio: enable (in|out).mixing-engine=off
> 61543ec audio: break generic buffer dependency on mixing-engine
> 330dfbe sdlaudio: add recording functions
> 998b92f audio: split pcm_ops function get_buffer_in
> bc84416 sdlaudio: replace legacy functions with modern ones
> 8f33798 sdlaudio: fill remaining sample buffer with silence
> 78c2474 sdlaudio: always clear the sample buffer
> f5ea854 sdlaudio: don't start playback in init routine
> 7a1a2df sdlaudio: add -audiodev sdl,out.buffer-count option
> 6aa7760 audio: fix bit-rotted code
> 98cf04b sdlaudio: remove leftover SDL1.2 code
>
> === OUTPUT BEGIN ===
> 1/23 Checking commit 98cf04b7c44a (sdlaudio: remove leftover SDL1.2 code)
> 2/23 Checking commit 6aa776039e75 (audio: fix bit-rotted code)
> 3/23 Checking commit 7a1a2df1c97f (sdlaudio: add -audiodev sdl,out.buffer-count option)
> 4/23 Checking commit f5ea85493feb (sdlaudio: don't start playback in init routine)
> 5/23 Checking commit 78c2474549af (sdlaudio: always clear the sample buffer)
> 6/23 Checking commit 8f33798c4ff8 (sdlaudio: fill remaining sample buffer with silence)
> 7/23 Checking commit bc844166b227 (sdlaudio: replace legacy functions with modern ones)
> ERROR: spaces required around that '*' (ctx:WxV)
> #133: FILE: audio/sdlaudio.c:247:
> +        glue(SDLVoice, dir) *sdl = (glue(SDLVoice, dir) *)hw;  \
>                              ^
>
> ERROR: spaces required around that '*' (ctx:WxB)
> #133: FILE: audio/sdlaudio.c:247:
> +        glue(SDLVoice, dir) *sdl = (glue(SDLVoice, dir) *)hw;  \
>                                                          ^
>
> total: 2 errors, 0 warnings, 222 lines checked
>
> Patch 7/23 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>
> 8/23 Checking commit 998b92fd32ff (audio: split pcm_ops function get_buffer_in)
> 9/23 Checking commit 330dfbed90b4 (sdlaudio: add recording functions)
> ERROR: spaces required around that '*' (ctx:WxV)
> #86: FILE: audio/sdlaudio.c:306:
> +        glue(SDLVoice, dir) *sdl = (glue(SDLVoice, dir) *)hw;  \
>                              ^
>
> ERROR: spaces required around that '*' (ctx:WxB)
> #86: FILE: audio/sdlaudio.c:306:
> +        glue(SDLVoice, dir) *sdl = (glue(SDLVoice, dir) *)hw;  \
>                                                          ^
>
> total: 2 errors, 0 warnings, 185 lines checked
>
> Patch 9/23 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.

All errors are false positives. The * isn't a multiplication.

>
> 10/23 Checking commit 61543ec830f5 (audio: break generic buffer dependency on mixing-engine)
> 11/23 Checking commit 0cc736d525a7 (sdlaudio: enable (in|out).mixing-engine=off)
> 12/23 Checking commit d6703422e706 (audio: remove remaining unused plive code)
> 13/23 Checking commit 6cc0dee46213 (paaudio: avoid to clip samples multiple times)
> 14/23 Checking commit 11d109269391 (paaudio: wait for PA_STREAM_READY in qpa_write())
> 15/23 Checking commit d5c8eb16d112 (paaudio: wait until the playback stream is ready)
> 16/23 Checking commit 1045ac7440af (paaudio: remove unneeded code)
> 17/23 Checking commit 4fd4d92e5788 (paaudio: comment bugs in functions qpa_init_*)
> 18/23 Checking commit 2f94e489468a (paaudio: limit minreq to 75% of audio timer_rate)
> 19/23 Checking commit 98454ba25082 (paaudio: send recorded data in smaller chunks)
> 20/23 Checking commit 3bf4a8e55ba4 (dsoundaudio: replace GetForegroundWindow())
> 21/23 Checking commit ead5e23b8560 (dsoundaudio: rename dsound_open())
> 22/23 Checking commit 825e3ade19c8 (dsoundaudio: enable f32 audio sample format)
> 23/23 Checking commit f5676b9a646c (dsoundaudio: fix log message)
> === OUTPUT END ===
>
> Test command exited with code: 1
>
>
> The full log is available at
> http://patchew.org/logs/9315afe5-5958-c0b4-ea1e-14769511a9d5@t-online.de/testing.checkpatch/?type=message.
> ---
> Email generated automatically by Patchew [https://patchew.org/].
> Please send your feedback to patchew-devel@redhat.com

Re: [PATCH 03/23] sdlaudio: add -audiodev sdl, out.buffer-count option

[Gerd Hoffmann]

  Hi,

> -    hw->samples = obt.samples;
> +    hw->samples = (spdo->has_buffer_count ? spdo->buffer_count : 4) *
> +        obt.samples;

> +# @buffer-count: number of buffers (default 4)

Any specific reason for this default?

In my testing I've needed much higher values.
8 still got me crackling sound, 16 worked ok.

take care,
  Gerd

Re: [PATCH 02/23] audio: fix bit-rotted code

[Gerd Hoffmann]

  Hi,

>  #ifdef DEBUG
> -    alsa_dump_info(req, obt, obtfmt, pdo);
> +    alsa_dump_info(req, obt, obtfmt, apdo);
>  #endif

"if (DEBUG) { .... }" is a nice way to have this checked by the
compiler.  With "#define DEBUG 0" the compiler will optimize away
the dead code, so it isn't much different to #ifdef'ed code.

take care,
  Gerd

Re: [PATCH 00/23] next round of audio patches

[Gerd Hoffmann]

On Sun, Jan 10, 2021 at 11:01:29AM +0100, Volker Rümelin wrote:
> A mix of bug fixes and improvements.
> 
> Patches 01/23 - 11/23 have a few SDL fixes and add audio recording
> functions to the SDL audio backend.
> 
> Patch 12/23 removes unnecessary code from audio/audio.c.
> 
> Patches 13/23 - 16/23 fix a few PulseAudio backend bugs.
> 
> Patch 17/23 shows a PulseAudio backend bug. So far I don't think I
> broke any fine tuned user settings and I don't want to do it here.
> 
> Patches 18/23 - 19/23 are the first steps towards glitch free and
> lower latency PulseAudio playback and recording. Currently qemu
> uses incredibly large buffers in the PulseAudio backend. For
> playback this just increases the playback latency but doesn't
> improve dropout safety, because PulseAudio can't access this buffer
> directly. With these patches it's possible to move the large qemu
> buffer to the PulseAudio server side and just keep a small buffer
> on the qemu side. On the PulseAudio server side PulseAudio tries
> to place a part of these buffers directly on the hardware and
> PulseAudio runs with a higher priority than qemu, so it has a
> better chance to deliver audio data in time.
> 
> Here is an example to show how this works:
> -device intel-hda -device hda-duplex,audiodev=audio0
> -machine pcspk-audiodev=audio0 -audiodev pa,id=audio0,
> out.buffer-length=14000,out.latency=46440,in.latency=46440
> 
> Due to a bug in the PulseAudio backend, these command line options
> actually decrease the playback latency compared to current defaults.
> For playback with defaults (16 bits, stereo, 44100 samples/s)
> we have a 15ms server side buffer + 2 * 46.44ms qemu audio buffer
> + 23.22ms hda codec buffer = 131.1ms latency. With my example it's
> 46.44ms + 2 * 14ms + 23.22ms = 97.66ms latency and I guess you
> won't hear any drop outs. Btw.: 14ms = 10ms timer-period + 4ms
> additional playback data the hda codec can produce in timer-period
> time.
> 
> Patches 20/23 - 23/23 fix small issues with DirectSound.

Nice overall improvements.  Some minor nits (see replies to some
patches).  Can be fixed incrementally though, so I'll do some more
testing and if nothing blows up queue the series for merge.

thanks,
  Gerd

Re: [PATCH 01/23] sdlaudio: remove leftover SDL1.2 code

[Thomas Huth]

On 10/01/2021 11.02, Volker Rümelin wrote:
> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
> ---
>   audio/sdlaudio.c | 30 +++++++++++++-----------------
>   1 file changed, 13 insertions(+), 17 deletions(-)
> 
> diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c
> index 21b7a0484b..bf3cfb8456 100644
> --- a/audio/sdlaudio.c
> +++ b/audio/sdlaudio.c
> @@ -240,28 +240,24 @@ static void sdl_callback (void *opaque, Uint8 *buf, int len)
>       }
>   }
>   
> -#define SDL_WRAPPER_FUNC(name, ret_type, args_decl, args, fail, unlock) \
> -    static ret_type glue(sdl_, name)args_decl                           \
> -    {                                                                   \
> -        ret_type ret;                                                   \
> -                                                                        \
> -        SDL_LockAudio();                                                \
> -                                                                        \
> -        ret = glue(audio_generic_, name)args;                           \
> -                                                                        \
> -        SDL_UnlockAudio();                                              \
> -        return ret;                                                     \
> +#define SDL_WRAPPER_FUNC(name, ret_type, args_decl, args)      \
> +    static ret_type glue(sdl_, name)args_decl                  \
> +    {                                                          \
> +        ret_type ret;                                          \
> +                                                               \
> +        SDL_LockAudio();                                       \
> +        ret = glue(audio_generic_, name)args;                  \
> +        SDL_UnlockAudio();                                     \
> +                                                               \
> +        return ret;                                            \
>       }
>   
>   SDL_WRAPPER_FUNC(get_buffer_out, void *, (HWVoiceOut *hw, size_t *size),
> -                 (hw, size), *size = 0, sdl_unlock)
> +                 (hw, size))
>   SDL_WRAPPER_FUNC(put_buffer_out, size_t,
> -                 (HWVoiceOut *hw, void *buf, size_t size), (hw, buf, size),
> -                 /*nothing*/, sdl_unlock_and_post)
> +                 (HWVoiceOut *hw, void *buf, size_t size), (hw, buf, size))
>   SDL_WRAPPER_FUNC(write, size_t,
> -                 (HWVoiceOut *hw, void *buf, size_t size), (hw, buf, size),
> -                 /*nothing*/, sdl_unlock_and_post)
> -
> +                 (HWVoiceOut *hw, void *buf, size_t size), (hw, buf, size))
>   #undef SDL_WRAPPER_FUNC
>   
>   static void sdl_fini_out (HWVoiceOut *hw)
> 

Right, sdl_unlock and sdl_unlock_and_post have been removed in commit 
8a7816c4ac13e6ba61de2 already.

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 06/23] sdlaudio: fill remaining sample buffer with silence

[Thomas Huth]

On 10/01/2021 11.02, Volker Rümelin wrote:
> Fill the remaining sample buffer with silence. To fill it with
> zeroes is wrong for unsigned samples because this is silence
> with a DC bias.
> 
> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
> ---
>   audio/sdlaudio.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c
> index 79eed23849..01ae4c600e 100644
> --- a/audio/sdlaudio.c
> +++ b/audio/sdlaudio.c
> @@ -235,7 +235,8 @@ static void sdl_callback (void *opaque, Uint8 *buf, int len)
>   
>       /* clear remaining buffer that we couldn't fill with data */
>       if (len) {
> -        memset(buf, 0, len);
> +        audio_pcm_info_clear_buf(&hw->info, buf,
> +                                 len / hw->info.bytes_per_frame);
>       }
>   }
>  

Ignorant question: Is anybody still using unsigned samples in the 21st century?

Anyway,
Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 04/23] sdlaudio: don't start playback in init routine

[Thomas Huth]

On 10/01/2021 11.02, Volker Rümelin wrote:
> Every emulated audio device has a way to enable audio playback. Don't
> start playback until the guest enables the audio device. This patch
> keeps the SDL2 device pause state in sync with hw->enabled.
> 
> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
> ---
>   audio/sdlaudio.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c
> index 431bfcfddd..68126a99ab 100644
> --- a/audio/sdlaudio.c
> +++ b/audio/sdlaudio.c
> @@ -312,7 +312,6 @@ static int sdl_init_out(HWVoiceOut *hw, struct audsettings *as,
>   
>       s->initialized = 1;
>       s->exit = 0;
> -    SDL_PauseAudio (0);
>       return 0;
>   }
>   
> 

Right, there is also the sdl_enable_out() function that enables audio when 
necessary.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Tested-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 05/23] sdlaudio: always clear the sample buffer

[Thomas Huth]

On 10/01/2021 11.02, Volker Rümelin wrote:
> Always fill the remaining audio callback buffer with silence.
> SDL 2.0 doesn't initialize the audio callback buffer. This was
> an incompatible change compared to SDL 1.2. For reference read
> the SDL 1.2 to 2.0 migration guide.
> 
> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
> ---
>   audio/sdlaudio.c | 33 ++++++++++++++++-----------------
>   1 file changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c
> index 68126a99ab..79eed23849 100644
> --- a/audio/sdlaudio.c
> +++ b/audio/sdlaudio.c
> @@ -211,27 +211,26 @@ static void sdl_callback (void *opaque, Uint8 *buf, int len)
>       SDLAudioState *s = &glob_sdl;
>       HWVoiceOut *hw = &sdl->hw;
>   
> -    if (s->exit) {
> -        return;
> -    }
> +    if (!s->exit) {
>   
> -    /* dolog("callback: len=%d avail=%zu\n", len, hw->pending_emul); */
> +        /* dolog("callback: len=%d avail=%zu\n", len, hw->pending_emul); */
>   
> -    while (hw->pending_emul && len) {
> -        size_t write_len;
> -        ssize_t start = ((ssize_t) hw->pos_emul) - hw->pending_emul;
> -        if (start < 0) {
> -            start += hw->size_emul;
> -        }
> -        assert(start >= 0 && start < hw->size_emul);
> +        while (hw->pending_emul && len) {
> +            size_t write_len;
> +            ssize_t start = (ssize_t)hw->pos_emul - hw->pending_emul;
> +            if (start < 0) {
> +                start += hw->size_emul;
> +            }
> +            assert(start >= 0 && start < hw->size_emul);
>   
> -        write_len = MIN(MIN(hw->pending_emul, len),
> -                        hw->size_emul - start);
> +            write_len = MIN(MIN(hw->pending_emul, len),
> +                            hw->size_emul - start);
>   
> -        memcpy(buf, hw->buf_emul + start, write_len);
> -        hw->pending_emul -= write_len;
> -        len -= write_len;
> -        buf += write_len;
> +            memcpy(buf, hw->buf_emul + start, write_len);
> +            hw->pending_emul -= write_len;
> +            len -= write_len;
> +            buf += write_len;
> +        }
>       }
>   
>       /* clear remaining buffer that we couldn't fill with data */
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 07/23] sdlaudio: replace legacy functions with modern ones

[Thomas Huth]

On 10/01/2021 11.02, Volker Rümelin wrote:
> With the modern audio functions it's possible to add new
> features like audio recording.
> 
> As a side effect this patch fixes a bug where SDL2 can't be used
> on Windows. This bug was reported on the qemu-devel mailing list at
> 
> https://lists.nongnu.org/archive/html/qemu-devel/2020-01/msg04043.html
> 
> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
> ---
>   audio/sdlaudio.c | 107 ++++++++++++++++++++++-------------------------
>   1 file changed, 50 insertions(+), 57 deletions(-)

Looks fine to me.

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 03/23] sdlaudio: add -audiodev sdl, out.buffer-count option

[Markus Armbruster]

Volker Rümelin <vr_qemu@t-online.de> writes:

> Currently there is a crackling noise with SDL2 audio playback.
> Commit bcf19777df: "audio/sdlaudio: Allow audio playback with
> SDL2" already mentioned the crackling noise.
>
> Add an out.buffer-count option to give users a chance to select
> sane settings for glitch free audio playback. The idea was taken
> from the coreaudio backend.
>
> The in.buffer-count option will be used with one of the next
> patches.
>
> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
> ---
[...]
> diff --git a/qapi/audio.json b/qapi/audio.json
> index 072ed79def..9cba0df8a4 100644
> --- a/qapi/audio.json
> +++ b/qapi/audio.json
> @@ -301,6 +301,37 @@
>      '*out':    'AudiodevPaPerDirectionOptions',
>      '*server': 'str' } }
>  
> +##
> +# @AudiodevSdlPerDirectionOptions:
> +#
> +# Options of the SDL audio backend that are used for both playback and
> +# recording.
> +#
> +# @buffer-count: number of buffers (default 4)
> +#
> +# Since: 6.0
> +##
> +{ 'struct': 'AudiodevSdlPerDirectionOptions',
> +  'base': 'AudiodevPerDirectionOptions',
> +  'data': {
> +    '*buffer-count': 'uint32' } }
> +
> +##
> +# @AudiodevSdlOptions:
> +#
> +# Options of the SDL audio backend.
> +#
> +# @in: options of the recording stream
> +#
> +# @out: options of the playback stream
> +#
> +# Since: 6.0
> +##
> +{ 'struct': 'AudiodevSdlOptions',
> +  'data': {
> +    '*in':  'AudiodevSdlPerDirectionOptions',
> +    '*out': 'AudiodevSdlPerDirectionOptions' } }
> +
>  ##
>  # @AudiodevWavOptions:
>  #
> @@ -385,6 +416,6 @@
>      'jack':      'AudiodevJackOptions',
>      'oss':       'AudiodevOssOptions',
>      'pa':        'AudiodevPaOptions',
> -    'sdl':       'AudiodevGenericOptions',
> +    'sdl':       'AudiodevSdlOptions',
>      'spice':     'AudiodevGenericOptions',
>      'wav':       'AudiodevWavOptions' } }
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 1698a0c751..4e02e9bd76 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -588,6 +588,7 @@ DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev,
>  #endif
>  #ifdef CONFIG_AUDIO_SDL
>      "-audiodev sdl,id=id[,prop[=value][,...]]\n"
> +    "                in|out.buffer-count= number of buffers\n"
>  #endif
>  #ifdef CONFIG_SPICE
>      "-audiodev spice,id=id[,prop[=value][,...]]\n"
> @@ -745,7 +746,12 @@ SRST
>  ``-audiodev sdl,id=id[,prop[=value][,...]]``
>      Creates a backend using SDL. This backend is available on most
>      systems, but you should use your platform's native backend if
> -    possible. This backend has no backend specific properties.
> +    possible.
> +
> +    SDL specific options are:
> +
> +    ``in|out.buffer-count=count``
> +        Sets the count of the buffers.
>  
>  ``-audiodev spice,id=id[,prop[=value][,...]]``
>      Creates a backend that sends audio through SPICE. This backend

These parts:
Acked-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH 06/23] sdlaudio: fill remaining sample buffer with silence

[Volker Rümelin]

> On 10/01/2021 11.02, Volker Rümelin wrote:
>> Fill the remaining sample buffer with silence. To fill it with
>> zeroes is wrong for unsigned samples because this is silence
>> with a DC bias.
>>
>> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
>> ---
>>   audio/sdlaudio.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c
>> index 79eed23849..01ae4c600e 100644
>> --- a/audio/sdlaudio.c
>> +++ b/audio/sdlaudio.c
>> @@ -235,7 +235,8 @@ static void sdl_callback (void *opaque, Uint8 
>> *buf, int len)
>>         /* clear remaining buffer that we couldn't fill with data */
>>       if (len) {
>> -        memset(buf, 0, len);
>> +        audio_pcm_info_clear_buf(&hw->info, buf,
>> +                                 len / hw->info.bytes_per_frame);
>>       }
>>   }
>>
>
> Ignorant question: Is anybody still using unsigned samples in the 21st 
> century?
>

Hi Thomas,

you are probably right that no one is knowingly using unsigned samples 
anymore. But qemu emulates audio devices from the last century. For 
example with the command line options -machine pcspk-audiodev=audio0 
-audiodev sdl,id=audio0,out.buffer-length=3750,out.mixing-engine=off you 
will see 8 bit unsigned samples in the SDL callback buffer.

With best regards,
Volker

> Anyway,
> Reviewed-by: Thomas Huth <thuth@redhat.com>
>

Re: [PATCH 02/23] audio: fix bit-rotted code

[Volker Rümelin]

>>   #ifdef DEBUG
>> -    alsa_dump_info(req, obt, obtfmt, pdo);
>> +    alsa_dump_info(req, obt, obtfmt, apdo);
>>   #endif
> "if (DEBUG) { .... }" is a nice way to have this checked by the
> compiler.  With "#define DEBUG 0" the compiler will optimize away
> the dead code, so it isn't much different to #ifdef'ed code.

Hi,

I will amend this in my next patch series.

With best regards,
Volker

> take care,
>    Gerd
>

Re: [PATCH 03/23] sdlaudio: add -audiodev sdl, out.buffer-count option

[Volker Rümelin]

>    Hi,
>
>> -    hw->samples = obt.samples;
>> +    hw->samples = (spdo->has_buffer_count ? spdo->buffer_count : 4) *
>> +        obt.samples;
>> +# @buffer-count: number of buffers (default 4)
> Any specific reason for this default?
>
> In my testing I've needed much higher values.
> 8 still got me crackling sound, 16 worked ok.

Hi Gerd,

this was an attempt to come up with SDL audio settings which work for 
all SDL audio drivers. Unfortunately, the different SDL audio drivers 
have different timings and there are no default settings that work for 
all of them. Here are two examples where buffer-count=4 works.

On my Linux system I use

export SDL_AUDIODRIVER=pulse
and start qemu with -device intel-hda -device hda-duplex,audiodev=audio0 
-machine pcspk-audiodev=audio0 -audiodev 
sdl,id=audio0,out.buffer-length=3750

Due to the mix-up of samples and frames in audio/sdlaudio.c the callback 
buffer has a size of 2 * 3.75ms = 7.5ms and SDL calls the callback 
function every 7.5ms. With out.buffer-count=4 that's a 4 * 7.5ms = 30ms 
buffer on the qemu side. This is larger than the minimum size of 
timer-period.

On Windows the timing is different. The time between SDL callback calls 
is a multiple of 10ms. I have to use

export SDL_AUDIODRIVER=directsound
and start qemu with -device intel-hda -device hda-duplex,audiodev=audio0 
-machine pcspk-audiodev=audio0 -audiodev 
sdl,id=audio0,timer-period=1000,out.buffer-length=5500

With the above settings the playback stream sometimes will see 2*10ms + 
1ms stalls. The qemu hda codec can barely handle this. On average it 
will drop playback data after 23.22ms.

With best regards,
Volker

> take care,
>    Gerd
>