[PATCH 00/23] next round of audio patches

Volker Rümelin posted 23 patches 3 years, 2 months ago
Test checkpatch failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/9315afe5-5958-c0b4-ea1e-14769511a9d5@t-online.de
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(-)
[PATCH 00/23] next round of audio patches
Posted by Volker Rümelin 3 years, 2 months ago
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

Re: [PATCH 00/23] next round of audio patches
Posted by no-reply@patchew.org 3 years, 2 months ago
Patchew URL: https://patchew.org/QEMU/9315afe5-5958-c0b4-ea1e-14769511a9d5@t-online.de/



Hi,

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

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

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

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

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

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

total: 2 errors, 0 warnings, 222 lines checked

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

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

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

total: 2 errors, 0 warnings, 185 lines checked

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

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

Test command exited with code: 1


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

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

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


Re: [PATCH 00/23] next round of audio patches
Posted by Gerd Hoffmann 3 years, 2 months ago
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