[Qemu-devel] [PULL 0/5] Audio 20190228 patches

[Qemu-devel] [PULL 0/5] Audio 20190228 patches

[Gerd Hoffmann]

The following changes since commit 86c7e2f4a93322a76afea5ee6806a83420d1dfea:

  Merge remote-tracking branch 'remotes/berrange/tags/authz-core-pull-request' into staging (2019-02-26 17:59:41 +0000)

are available in the git repository at:

  git://git.kraxel.org/qemu tags/audio-20190228-pull-request

for you to fetch changes up to 9399ef168377d9e7f2e33b1c2eb61751aa1b72fa:

  audio/sdlaudio: Simplify the sdl_callback function (2019-02-28 10:30:08 +0100)

----------------------------------------------------------------
audio: fixes and cleanups.

----------------------------------------------------------------

Frediano Ziglio (2):
  audio: Use g_strdup_printf instead of manual building a string
  audio: Do not check for audio_calloc failure

Gerd Hoffmann (1):
  audio: don't build alsa and sdl by default on linux

Thomas Huth (2):
  audio/sdlaudio: Remove the semaphore code
  audio/sdlaudio: Simplify the sdl_callback function

 configure        |   2 +-
 audio/audio.c    |  73 ++++-----------------
 audio/sdlaudio.c | 190 +++++++------------------------------------------------
 3 files changed, 36 insertions(+), 229 deletions(-)

-- 
2.9.3

[Qemu-devel] [PULL 1/5] audio: Use g_strdup_printf instead of manual building a string

[Gerd Hoffmann]

From: Frediano Ziglio <fziglio@redhat.com>

Instead of using lot of low level function and manually allocate
the temporary string in audio_process_options use more high
level GLib function. The function is not used in hot path but to
read some initial setting.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Message-id: 20190225154335.11397-1-fziglio@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 audio/audio.c | 25 +++++--------------------
 1 file changed, 5 insertions(+), 20 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index d163ffbc88f4..472721a7a9f0 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -454,9 +454,7 @@ static void audio_print_options (const char *prefix,
 static void audio_process_options (const char *prefix,
                                    struct audio_option *opt)
 {
-    char *optname;
-    const char qemu_prefix[] = "QEMU_";
-    size_t preflen, optlen;
+    gchar *prefix_upper;
 
     if (audio_bug(__func__, !prefix)) {
         dolog ("prefix = NULL\n");
@@ -468,10 +466,10 @@ static void audio_process_options (const char *prefix,
         return;
     }
 
-    preflen = strlen (prefix);
+    prefix_upper = g_utf8_strup(prefix, -1);
 
     for (; opt->name; opt++) {
-        size_t len, i;
+        char *optname;
         int def;
 
         if (!opt->valp) {
@@ -480,21 +478,7 @@ static void audio_process_options (const char *prefix,
             continue;
         }
 
-        len = strlen (opt->name);
-        /* len of opt->name + len of prefix + size of qemu_prefix
-         * (includes trailing zero) + zero + underscore (on behalf of
-         * sizeof) */
-        optlen = len + preflen + sizeof (qemu_prefix) + 1;
-        optname = g_malloc (optlen);
-
-        pstrcpy (optname, optlen, qemu_prefix);
-
-        /* copy while upper-casing, including trailing zero */
-        for (i = 0; i <= preflen; ++i) {
-            optname[i + sizeof (qemu_prefix) - 1] = qemu_toupper(prefix[i]);
-        }
-        pstrcat (optname, optlen, "_");
-        pstrcat (optname, optlen, opt->name);
+        optname = g_strdup_printf("QEMU_%s_%s", prefix_upper, opt->name);
 
         def = 1;
         switch (opt->tag) {
@@ -532,6 +516,7 @@ static void audio_process_options (const char *prefix,
         *opt->overriddenp = !def;
         g_free (optname);
     }
+    g_free(prefix_upper);
 }
 
 static void audio_print_settings (struct audsettings *as)
-- 
2.9.3

[Qemu-devel] [PULL 2/5] audio: Do not check for audio_calloc failure

[Gerd Hoffmann]

From: Frediano Ziglio <fziglio@redhat.com>

audio_calloc uses g_malloc0 which never returns in case of
memory failure.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Message-id: 20190225154335.11397-2-fziglio@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 audio/audio.c | 48 ++++++------------------------------------------
 1 file changed, 6 insertions(+), 42 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 472721a7a9f0..909c817103c8 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -811,12 +811,7 @@ static int audio_attach_capture (HWVoiceOut *hw)
         SWVoiceOut *sw;
         HWVoiceOut *hw_cap = &cap->hw;
 
-        sc = audio_calloc(__func__, 1, sizeof(*sc));
-        if (!sc) {
-            dolog ("Could not allocate soft capture voice (%zu bytes)\n",
-                   sizeof (*sc));
-            return -1;
-        }
+        sc = g_malloc0(sizeof(*sc));
 
         sc->cap = cap;
         sw = &sc->sw;
@@ -1960,15 +1955,10 @@ CaptureVoiceOut *AUD_add_capture (
     if (audio_validate_settings (as)) {
         dolog ("Invalid settings were passed when trying to add capture\n");
         audio_print_settings (as);
-        goto err0;
+        return NULL;
     }
 
-    cb = audio_calloc(__func__, 1, sizeof(*cb));
-    if (!cb) {
-        dolog ("Could not allocate capture callback information, size %zu\n",
-               sizeof (*cb));
-        goto err0;
-    }
+    cb = g_malloc0(sizeof(*cb));
     cb->ops = *ops;
     cb->opaque = cb_opaque;
 
@@ -1981,12 +1971,7 @@ CaptureVoiceOut *AUD_add_capture (
         HWVoiceOut *hw;
         CaptureVoiceOut *cap;
 
-        cap = audio_calloc(__func__, 1, sizeof(*cap));
-        if (!cap) {
-            dolog ("Could not allocate capture voice, size %zu\n",
-                   sizeof (*cap));
-            goto err1;
-        }
+        cap = g_malloc0(sizeof(*cap));
 
         hw = &cap->hw;
         QLIST_INIT (&hw->sw_head);
@@ -1994,23 +1979,11 @@ CaptureVoiceOut *AUD_add_capture (
 
         /* XXX find a more elegant way */
         hw->samples = 4096 * 4;
-        hw->mix_buf = audio_calloc(__func__, hw->samples,
-                                   sizeof(struct st_sample));
-        if (!hw->mix_buf) {
-            dolog ("Could not allocate capture mix buffer (%d samples)\n",
-                   hw->samples);
-            goto err2;
-        }
+        hw->mix_buf = g_new0(struct st_sample, hw->samples);
 
         audio_pcm_init_info (&hw->info, as);
 
-        cap->buf = audio_calloc(__func__, hw->samples, 1 << hw->info.shift);
-        if (!cap->buf) {
-            dolog ("Could not allocate capture buffer "
-                   "(%d samples, each %d bytes)\n",
-                   hw->samples, 1 << hw->info.shift);
-            goto err3;
-        }
+        cap->buf = g_malloc0_n(hw->samples, 1 << hw->info.shift);
 
         hw->clip = mixeng_clip
             [hw->info.nchannels == 2]
@@ -2025,15 +1998,6 @@ CaptureVoiceOut *AUD_add_capture (
             audio_attach_capture (hw);
         }
         return cap;
-
-    err3:
-        g_free (cap->hw.mix_buf);
-    err2:
-        g_free (cap);
-    err1:
-        g_free (cb);
-    err0:
-        return NULL;
     }
 }
 
-- 
2.9.3

[Qemu-devel] [PULL 3/5] audio: don't build alsa and sdl by default on linux

[Gerd Hoffmann]

In case no sound hardware is present both alsa and sdl drivers
initialize successfully and throw errors later on, i.e. effectively
the automatic probing doesn't work.  Drop them from the list of
default audio drivers for linux because of that.

Fixes: 6a48541873 audio: probe audio drivers by default
Buglink: https://bugs.launchpad.net/qemu/+bug/1816052
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Tested-by: David Hildenbrand <david@redhat.com>
Message-id: 20190219124257.3001-1-kraxel@redhat.com
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index 694088a4ec99..540bee19ba1d 100755
--- a/configure
+++ b/configure
@@ -879,7 +879,7 @@ Haiku)
   LIBS="-lposix_error_mapper -lnetwork $LIBS"
 ;;
 Linux)
-  audio_drv_list="try-pa try-alsa try-sdl oss"
+  audio_drv_list="try-pa oss"
   audio_possible_drivers="oss alsa sdl pa"
   linux="yes"
   linux_user="yes"
-- 
2.9.3

[Qemu-devel] [PULL 4/5] audio/sdlaudio: Remove the semaphore code

[Gerd Hoffmann]

From: Thomas Huth <thuth@redhat.com>

The semaphore code was only working with SDL1.2 - with SDL2, it causes
a deadlock. Since we've removed support for SDL1.2 recently, we can
now completely remove the semaphore code from sdlaudio.c.

Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 1549336101-17623-2-git-send-email-thuth@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 audio/sdlaudio.c | 145 ++-----------------------------------------------------
 1 file changed, 5 insertions(+), 140 deletions(-)

diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c
index 9db5ac92bcc0..53bfdbf72439 100644
--- a/audio/sdlaudio.c
+++ b/audio/sdlaudio.c
@@ -38,14 +38,9 @@
 #define AUDIO_CAP "sdl"
 #include "audio_int.h"
 
-#define USE_SEMAPHORE (SDL_MAJOR_VERSION < 2)
-
 typedef struct SDLVoiceOut {
     HWVoiceOut hw;
     int live;
-#if USE_SEMAPHORE
-    int rpos;
-#endif
     int decr;
 } SDLVoiceOut;
 
@@ -57,10 +52,6 @@ static struct {
 
 static struct SDLAudioState {
     int exit;
-#if USE_SEMAPHORE
-    SDL_mutex *mutex;
-    SDL_sem *sem;
-#endif
     int initialized;
     bool driver_created;
 } glob_sdl;
@@ -77,66 +68,6 @@ static void GCC_FMT_ATTR (1, 2) sdl_logerr (const char *fmt, ...)
     AUD_log (AUDIO_CAP, "Reason: %s\n", SDL_GetError ());
 }
 
-static int sdl_lock (SDLAudioState *s, const char *forfn)
-{
-#if USE_SEMAPHORE
-    if (SDL_LockMutex (s->mutex)) {
-        sdl_logerr ("SDL_LockMutex for %s failed\n", forfn);
-        return -1;
-    }
-#else
-    SDL_LockAudio();
-#endif
-
-    return 0;
-}
-
-static int sdl_unlock (SDLAudioState *s, const char *forfn)
-{
-#if USE_SEMAPHORE
-    if (SDL_UnlockMutex (s->mutex)) {
-        sdl_logerr ("SDL_UnlockMutex for %s failed\n", forfn);
-        return -1;
-    }
-#else
-    SDL_UnlockAudio();
-#endif
-
-    return 0;
-}
-
-static int sdl_post (SDLAudioState *s, const char *forfn)
-{
-#if USE_SEMAPHORE
-    if (SDL_SemPost (s->sem)) {
-        sdl_logerr ("SDL_SemPost for %s failed\n", forfn);
-        return -1;
-    }
-#endif
-
-    return 0;
-}
-
-#if USE_SEMAPHORE
-static int sdl_wait (SDLAudioState *s, const char *forfn)
-{
-    if (SDL_SemWait (s->sem)) {
-        sdl_logerr ("SDL_SemWait for %s failed\n", forfn);
-        return -1;
-    }
-    return 0;
-}
-#endif
-
-static int sdl_unlock_and_post (SDLAudioState *s, const char *forfn)
-{
-    if (sdl_unlock (s, forfn)) {
-        return -1;
-    }
-
-    return sdl_post (s, forfn);
-}
-
 static int aud_to_sdlfmt (audfmt_e fmt)
 {
     switch (fmt) {
@@ -243,9 +174,9 @@ static int sdl_open (SDL_AudioSpec *req, SDL_AudioSpec *obt)
 static void sdl_close (SDLAudioState *s)
 {
     if (s->initialized) {
-        sdl_lock (s, "sdl_close");
+        SDL_LockAudio();
         s->exit = 1;
-        sdl_unlock_and_post (s, "sdl_close");
+        SDL_UnlockAudio();
         SDL_PauseAudio (1);
         SDL_CloseAudio ();
         s->initialized = 0;
@@ -267,30 +198,10 @@ static void sdl_callback (void *opaque, Uint8 *buf, int len)
         int to_mix, decr;
 
         /* dolog ("in callback samples=%d\n", samples); */
-#if USE_SEMAPHORE
-        sdl_wait (s, "sdl_callback");
-        if (s->exit) {
-            return;
-        }
 
-        if (sdl_lock (s, "sdl_callback")) {
-            return;
-        }
-
-        if (audio_bug(__func__, sdl->live < 0 || sdl->live > hw->samples)) {
-            dolog ("sdl->live=%d hw->samples=%d\n",
-                   sdl->live, hw->samples);
-            return;
-        }
-
-        if (!sdl->live) {
-            goto again;
-        }
-#else
         if (s->exit || !sdl->live) {
             break;
         }
-#endif
 
         /* dolog ("in callback live=%d\n", live); */
         to_mix = audio_MIN (samples, sdl->live);
@@ -301,33 +212,20 @@ static void sdl_callback (void *opaque, Uint8 *buf, int len)
 
             /* dolog ("in callback to_mix %d, chunk %d\n", to_mix, chunk); */
             hw->clip (buf, src, chunk);
-#if USE_SEMAPHORE
-            sdl->rpos = (sdl->rpos + chunk) % hw->samples;
-#else
             hw->rpos = (hw->rpos + chunk) % hw->samples;
-#endif
             to_mix -= chunk;
             buf += chunk << hw->info.shift;
         }
         samples -= decr;
         sdl->live -= decr;
         sdl->decr += decr;
-
-#if USE_SEMAPHORE
-    again:
-        if (sdl_unlock (s, "sdl_callback")) {
-            return;
-        }
-#endif
     }
     /* dolog ("done len=%d\n", len); */
 
-#if (SDL_MAJOR_VERSION >= 2)
     /* SDL2 does not clear the remaining buffer for us, so do it on our own */
     if (samples) {
         memset(buf, 0, samples << hw->info.shift);
     }
-#endif
 }
 
 static int sdl_write_out (SWVoiceOut *sw, void *buf, int len)
@@ -339,11 +237,8 @@ static int sdl_run_out (HWVoiceOut *hw, int live)
 {
     int decr;
     SDLVoiceOut *sdl = (SDLVoiceOut *) hw;
-    SDLAudioState *s = &glob_sdl;
 
-    if (sdl_lock (s, "sdl_run_out")) {
-        return 0;
-    }
+    SDL_LockAudio();
 
     if (sdl->decr > live) {
         ldebug ("sdl->decr %d live %d sdl->live %d\n",
@@ -355,19 +250,10 @@ static int sdl_run_out (HWVoiceOut *hw, int live)
     decr = audio_MIN (sdl->decr, live);
     sdl->decr -= decr;
 
-#if USE_SEMAPHORE
-    sdl->live = live - decr;
-    hw->rpos = sdl->rpos;
-#else
     sdl->live = live;
-#endif
 
-    if (sdl->live > 0) {
-        sdl_unlock_and_post (s, "sdl_run_out");
-    }
-    else {
-        sdl_unlock (s, "sdl_run_out");
-    }
+    SDL_UnlockAudio();
+
     return decr;
 }
 
@@ -449,23 +335,6 @@ static void *sdl_audio_init (void)
         return NULL;
     }
 
-#if USE_SEMAPHORE
-    s->mutex = SDL_CreateMutex ();
-    if (!s->mutex) {
-        sdl_logerr ("Failed to create SDL mutex\n");
-        SDL_QuitSubSystem (SDL_INIT_AUDIO);
-        return NULL;
-    }
-
-    s->sem = SDL_CreateSemaphore (0);
-    if (!s->sem) {
-        sdl_logerr ("Failed to create SDL semaphore\n");
-        SDL_DestroyMutex (s->mutex);
-        SDL_QuitSubSystem (SDL_INIT_AUDIO);
-        return NULL;
-    }
-#endif
-
     s->driver_created = true;
     return s;
 }
@@ -474,10 +343,6 @@ static void sdl_audio_fini (void *opaque)
 {
     SDLAudioState *s = opaque;
     sdl_close (s);
-#if USE_SEMAPHORE
-    SDL_DestroySemaphore (s->sem);
-    SDL_DestroyMutex (s->mutex);
-#endif
     SDL_QuitSubSystem (SDL_INIT_AUDIO);
     s->driver_created = false;
 }
-- 
2.9.3

[Qemu-devel] [PULL 5/5] audio/sdlaudio: Simplify the sdl_callback function

[Gerd Hoffmann]

From: Thomas Huth <thuth@redhat.com>

At the end of the while-loop, either "samples" or "sdl->live" is zero, so
now that we've removed the semaphore code, the content of the while-loop
is always only executed once. Thus we can remove the while-loop now to
get rid of one indentation level here.

Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-id: 1549336101-17623-3-git-send-email-thuth@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 audio/sdlaudio.c | 45 +++++++++++++++++++--------------------------
 1 file changed, 19 insertions(+), 26 deletions(-)

diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c
index 53bfdbf72439..f7ee70b15347 100644
--- a/audio/sdlaudio.c
+++ b/audio/sdlaudio.c
@@ -189,37 +189,30 @@ static void sdl_callback (void *opaque, Uint8 *buf, int len)
     SDLAudioState *s = &glob_sdl;
     HWVoiceOut *hw = &sdl->hw;
     int samples = len >> hw->info.shift;
+    int to_mix, decr;
 
-    if (s->exit) {
+    if (s->exit || !sdl->live) {
         return;
     }
 
-    while (samples) {
-        int to_mix, decr;
-
-        /* dolog ("in callback samples=%d\n", samples); */
-
-        if (s->exit || !sdl->live) {
-            break;
-        }
-
-        /* dolog ("in callback live=%d\n", live); */
-        to_mix = audio_MIN (samples, sdl->live);
-        decr = to_mix;
-        while (to_mix) {
-            int chunk = audio_MIN (to_mix, hw->samples - hw->rpos);
-            struct st_sample *src = hw->mix_buf + hw->rpos;
-
-            /* dolog ("in callback to_mix %d, chunk %d\n", to_mix, chunk); */
-            hw->clip (buf, src, chunk);
-            hw->rpos = (hw->rpos + chunk) % hw->samples;
-            to_mix -= chunk;
-            buf += chunk << hw->info.shift;
-        }
-        samples -= decr;
-        sdl->live -= decr;
-        sdl->decr += decr;
+    /* dolog ("in callback samples=%d live=%d\n", samples, sdl->live); */
+
+    to_mix = audio_MIN(samples, sdl->live);
+    decr = to_mix;
+    while (to_mix) {
+        int chunk = audio_MIN(to_mix, hw->samples - hw->rpos);
+        struct st_sample *src = hw->mix_buf + hw->rpos;
+
+        /* dolog ("in callback to_mix %d, chunk %d\n", to_mix, chunk); */
+        hw->clip(buf, src, chunk);
+        hw->rpos = (hw->rpos + chunk) % hw->samples;
+        to_mix -= chunk;
+        buf += chunk << hw->info.shift;
     }
+    samples -= decr;
+    sdl->live -= decr;
+    sdl->decr += decr;
+
     /* dolog ("done len=%d\n", len); */
 
     /* SDL2 does not clear the remaining buffer for us, so do it on our own */
-- 
2.9.3

Re: [Qemu-devel] [PULL 0/5] Audio 20190228 patches

[Peter Maydell]

On Thu, 28 Feb 2019 at 10:11, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> The following changes since commit 86c7e2f4a93322a76afea5ee6806a83420d1dfea:
>
>   Merge remote-tracking branch 'remotes/berrange/tags/authz-core-pull-request' into staging (2019-02-26 17:59:41 +0000)
>
> are available in the git repository at:
>
>   git://git.kraxel.org/qemu tags/audio-20190228-pull-request
>
> for you to fetch changes up to 9399ef168377d9e7f2e33b1c2eb61751aa1b72fa:
>
>   audio/sdlaudio: Simplify the sdl_callback function (2019-02-28 10:30:08 +0100)
>
> ----------------------------------------------------------------
> audio: fixes and cleanups.


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.0
for any user-visible changes.

-- PMM