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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
> 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
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
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
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
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>
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>
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>
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>
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>
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>
> 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> >
>> #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 >
> 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 >