From 1d47067394ef79c2a7ed9d4dd0b18cdf24f88f2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Sun, 13 Dec 2020 14:05:25 +0100 Subject: [PATCH 1/7] coreaudio: rename misnamed variable fake_as MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While the variable once was used to fake audio settings, since commit ed2a4a7941 "audio: proper support for float samples in mixeng" this is no longer true. Rename the variable to obt_as. This is the same naming scheme as in audio/sdlaudio.c Tested-by: Howard Spoelstra Signed-off-by: Volker Rümelin Message-id: 20201213130528.5863-1-vr_qemu@t-online.de Signed-off-by: Gerd Hoffmann --- audio/coreaudio.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/audio/coreaudio.c b/audio/coreaudio.c index 4b4365660f..0ee85052c4 100644 --- a/audio/coreaudio.c +++ b/audio/coreaudio.c @@ -482,7 +482,7 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as, Audiodev *dev = drv_opaque; AudiodevCoreaudioPerDirectionOptions *cpdo = dev->u.coreaudio.out; int frames; - struct audsettings fake_as; + struct audsettings obt_as; /* create mutex */ err = pthread_mutex_init(&core->mutex, NULL); @@ -491,8 +491,8 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as, return -1; } - fake_as = *as; - as = &fake_as; + obt_as = *as; + as = &obt_as; as->fmt = AUDIO_FORMAT_F32; audio_pcm_init_info (&hw->info, as); From 53e78d1cfb43df733a278172dd11bc40d2fe69c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Sun, 13 Dec 2020 14:05:26 +0100 Subject: [PATCH 2/7] coreaudio: don't start playback in init routine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every emulated audio device has a way to enable audio playback. Don't start playback until the guest enables the audio device to keep the Core Audio device run state in sync with hw->enabled. Tested-by: Howard Spoelstra Signed-off-by: Volker Rümelin Message-id: 20201213130528.5863-2-vr_qemu@t-online.de Signed-off-by: Gerd Hoffmann --- audio/coreaudio.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/audio/coreaudio.c b/audio/coreaudio.c index 0ee85052c4..a5df950514 100644 --- a/audio/coreaudio.c +++ b/audio/coreaudio.c @@ -584,17 +584,6 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as, return -1; } - /* start Playback */ - if (!isPlaying(core->outputDeviceID)) { - status = AudioDeviceStart(core->outputDeviceID, core->ioprocid); - if (status != kAudioHardwareNoError) { - coreaudio_logerr2 (status, typ, "Could not start playback\n"); - AudioDeviceDestroyIOProcID(core->outputDeviceID, core->ioprocid); - core->outputDeviceID = kAudioDeviceUnknown; - return -1; - } - } - return 0; } From ceb1165e9d60dcf11bd9c2bb04078a96cdc3c65b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Sun, 13 Dec 2020 14:05:27 +0100 Subject: [PATCH 3/7] coreaudio: always stop audio playback on shut down MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Always stop audio playback and remove the playback callback when QEMU exits. On shut down the function coreaudio_fini_out() destroys the coreaudio mutex but fails to stop audio playback and to remove the audio playback callback, because function audio_is_cleaning_up() always returns true when called from coreaudio_fini_out(). Now there is a time window from pthread_mutex_destroy() to program exit where Core Audio may call the audio playback callback which tries to lock the destroyed coreaudio mutex. This leads to the following error. coreaudio: Could not lock voice for audioDeviceIOProc Reason: Invalid argument This bug was reported on the qemu-discuss mailing list. https://lists.nongnu.org/archive/html/qemu-discuss/2020-10/msg00018.html Tested-by: Howard Spoelstra Signed-off-by: Volker Rümelin Message-id: 20201213130528.5863-3-vr_qemu@t-online.de Signed-off-by: Gerd Hoffmann --- audio/coreaudio.c | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/audio/coreaudio.c b/audio/coreaudio.c index a5df950514..79a9d40bf8 100644 --- a/audio/coreaudio.c +++ b/audio/coreaudio.c @@ -593,22 +593,20 @@ static void coreaudio_fini_out (HWVoiceOut *hw) int err; coreaudioVoiceOut *core = (coreaudioVoiceOut *) hw; - if (!audio_is_cleaning_up()) { - /* stop playback */ - if (isPlaying(core->outputDeviceID)) { - status = AudioDeviceStop(core->outputDeviceID, core->ioprocid); - if (status != kAudioHardwareNoError) { - coreaudio_logerr (status, "Could not stop playback\n"); - } - } - - /* remove callback */ - status = AudioDeviceDestroyIOProcID(core->outputDeviceID, - core->ioprocid); + /* stop playback */ + if (isPlaying(core->outputDeviceID)) { + status = AudioDeviceStop(core->outputDeviceID, core->ioprocid); if (status != kAudioHardwareNoError) { - coreaudio_logerr (status, "Could not remove IOProc\n"); + coreaudio_logerr(status, "Could not stop playback\n"); } } + + /* remove callback */ + status = AudioDeviceDestroyIOProcID(core->outputDeviceID, + core->ioprocid); + if (status != kAudioHardwareNoError) { + coreaudio_logerr(status, "Could not remove IOProc\n"); + } core->outputDeviceID = kAudioDeviceUnknown; /* destroy mutex */ @@ -633,13 +631,11 @@ static void coreaudio_enable_out(HWVoiceOut *hw, bool enable) } } else { /* stop playback */ - if (!audio_is_cleaning_up()) { - if (isPlaying(core->outputDeviceID)) { - status = AudioDeviceStop(core->outputDeviceID, - core->ioprocid); - if (status != kAudioHardwareNoError) { - coreaudio_logerr (status, "Could not pause playback\n"); - } + if (isPlaying(core->outputDeviceID)) { + status = AudioDeviceStop(core->outputDeviceID, + core->ioprocid); + if (status != kAudioHardwareNoError) { + coreaudio_logerr(status, "Could not pause playback\n"); } } } From ba6371b0c374053163a9840a2df05d09848db57a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Sun, 13 Dec 2020 14:05:28 +0100 Subject: [PATCH 4/7] audio: remove unused function audio_is_cleaning_up() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit removed the last call site of audio_is_cleaning_up(). Remove the now unused function. Tested-by: Howard Spoelstra Signed-off-by: Volker Rümelin Message-id: 20201213130528.5863-4-vr_qemu@t-online.de Signed-off-by: Gerd Hoffmann --- audio/audio.c | 8 -------- audio/audio.h | 1 - 2 files changed, 9 deletions(-) diff --git a/audio/audio.c b/audio/audio.c index 46578e4a58..a213409270 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -1588,13 +1588,6 @@ static void audio_vm_change_state_handler (void *opaque, int running, audio_reset_timer (s); } -static bool is_cleaning_up; - -bool audio_is_cleaning_up(void) -{ - return is_cleaning_up; -} - static void free_audio_state(AudioState *s) { HWVoiceOut *hwo, *hwon; @@ -1647,7 +1640,6 @@ static void free_audio_state(AudioState *s) void audio_cleanup(void) { - is_cleaning_up = true; while (!QTAILQ_EMPTY(&audio_states)) { AudioState *s = QTAILQ_FIRST(&audio_states); QTAILQ_REMOVE(&audio_states, s, list); diff --git a/audio/audio.h b/audio/audio.h index b883ebfb1f..41b3ef04ea 100644 --- a/audio/audio.h +++ b/audio/audio.h @@ -160,7 +160,6 @@ static inline void *advance (void *p, int incr) int wav_start_capture(AudioState *state, CaptureState *s, const char *path, int freq, int bits, int nchannels); -bool audio_is_cleaning_up(void); void audio_cleanup(void); void audio_sample_to_uint64(const void *samples, int pos, From 44ba6039375615135bb82e9094c43a1cbeb75660 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Fri, 11 Dec 2020 17:04:58 -0500 Subject: [PATCH 5/7] cs4231: Get rid of empty property array MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An empty props array is unnecessary, we can just not call device_class_set_props(). Reviewed-by: Marc-André Lureau Signed-off-by: Eduardo Habkost Message-id: 20201211220529.2290218-2-ehabkost@redhat.com Signed-off-by: Gerd Hoffmann --- hw/audio/cs4231.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/hw/audio/cs4231.c b/hw/audio/cs4231.c index 8e9554ce9b..209c05a0a0 100644 --- a/hw/audio/cs4231.c +++ b/hw/audio/cs4231.c @@ -160,17 +160,12 @@ static void cs4231_init(Object *obj) sysbus_init_irq(dev, &s->irq); } -static Property cs4231_properties[] = { - {.name = NULL}, -}; - static void cs4231_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); dc->reset = cs_reset; dc->vmsd = &vmstate_cs4231; - device_class_set_props(dc, cs4231_properties); } static const TypeInfo cs4231_info = { From ab32b78cd1b3b31950c4332f0fa8b192295d77fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 10 Dec 2020 23:35:06 +0100 Subject: [PATCH 6/7] audio: Simplify audio_bug() removing old code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This code (introduced in commit 1d14ffa97ea, Oct 2005) is likely unused since years. Time to remove it. If the condition is true, simply call abort(). Suggested-by: Gerd Hoffmann Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Marc-André Lureau Message-id: 20201210223506.263709-1-philmd@redhat.com Signed-off-by: Gerd Hoffmann --- audio/audio.c | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/audio/audio.c b/audio/audio.c index a213409270..0fdb808d6a 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -104,9 +104,6 @@ const struct mixeng_volume nominal_volume = { static bool legacy_config = true; -#ifdef AUDIO_IS_FLAWLESS_AND_NO_CHECKS_ARE_REQURIED -#error No its not -#else int audio_bug (const char *funcname, int cond) { if (cond) { @@ -119,25 +116,11 @@ int audio_bug (const char *funcname, int cond) AUD_log (NULL, "I am sorry\n"); } AUD_log (NULL, "Context:\n"); - -#if defined AUDIO_BREAKPOINT_ON_BUG -# if defined HOST_I386 -# if defined __GNUC__ - __asm__ ("int3"); -# elif defined _MSC_VER - _asm _emit 0xcc; -# else - abort (); -# endif -# else - abort (); -# endif -#endif + abort(); } return cond; } -#endif static inline int audio_bits_to_index (int bits) { From 06c8c375389a54d8e4457d967f4f0896caecefb2 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 15 Dec 2020 09:11:51 +0100 Subject: [PATCH 7/7] audio: add sanity check Check whenever we actually found the spiceaudio driver before flipping the can_be_default field. Fixes: f0c4555edfdd ("audio: remove qemu_spice_audio_init()") Buglink: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=977301 Reported-by: dann frazier Signed-off-by: Gerd Hoffmann Message-Id: <20201215081151.20095-1-kraxel@redhat.com> --- audio/audio.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/audio/audio.c b/audio/audio.c index 0fdb808d6a..b48471bb3f 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -1684,7 +1684,9 @@ static AudioState *audio_init(Audiodev *dev, const char *name) * backend and this can go away. */ driver = audio_driver_lookup("spice"); - driver->can_be_default = 1; + if (driver) { + driver->can_be_default = 1; + } } if (dev) {