[PATCH 0/3] misc. audio fixes

[PATCH 0/3] misc. audio fixes

[Volker Rümelin]

Three unrelated audio fixes.

The first one is a build fix for the Jack Audio backend on Windows and 
MacOS.

Volker Rümelin (3):
   jackaudio: use ifdefs to hide unavailable functions
   dsoundaudio: fix crackling audio recordings
   hw/audio/intel-hda: fix stream reset

  audio/dsoundaudio.c  | 7 +++----
  audio/jackaudio.c    | 4 ++++
  hw/audio/intel-hda.c | 2 +-
  3 files changed, 8 insertions(+), 5 deletions(-)

-- 
2.31.1

[PATCH 1/3] jackaudio: use ifdefs to hide unavailable functions

[Volker Rümelin]

On Windows the jack_set_thread_creator() function and on MacOS the
pthread_setname_np() function with a thread pointer paramater is
not available. Use #ifdefs to remove the jack_set_thread_creator()
function call and the qjack_thread_creator() function in both
cases.

The qjack_thread_creator() function just sets the name of the
created thread for debugging purposes and isn't really necessary.

From the jack_set_thread_creator() documentation:
(...)

No normal application/client should consider calling this. (...)

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/785
Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 audio/jackaudio.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/audio/jackaudio.c b/audio/jackaudio.c
index e7de6d5433..317009e936 100644
--- a/audio/jackaudio.c
+++ b/audio/jackaudio.c
@@ -622,6 +622,7 @@ static void qjack_enable_in(HWVoiceIn *hw, bool enable)
     ji->c.enabled = enable;
 }
 
+#if !defined(WIN32) && defined(CONFIG_PTHREAD_SETNAME_NP_W_TID)
 static int qjack_thread_creator(jack_native_thread_t *thread,
     const pthread_attr_t *attr, void *(*function)(void *), void *arg)
 {
@@ -635,6 +636,7 @@ static int qjack_thread_creator(jack_native_thread_t *thread,
 
     return ret;
 }
+#endif
 
 static void *qjack_init(Audiodev *dev)
 {
@@ -687,7 +689,9 @@ static void register_audio_jack(void)
 {
     qemu_mutex_init(&qjack_shutdown_lock);
     audio_driver_register(&jack_driver);
+#if !defined(WIN32) && defined(CONFIG_PTHREAD_SETNAME_NP_W_TID)
     jack_set_thread_creator(qjack_thread_creator);
+#endif
     jack_set_error_function(qjack_error);
     jack_set_info_function(qjack_info);
 }
-- 
2.31.1

Re: [PATCH 1/3] jackaudio: use ifdefs to hide unavailable functions

[Christian Schoenebeck]

On Sonntag, 26. Dezember 2021 16:40:15 CET Volker Rümelin wrote:
> On Windows the jack_set_thread_creator() function and on MacOS the
> pthread_setname_np() function with a thread pointer paramater is
> not available. Use #ifdefs to remove the jack_set_thread_creator()
> function call and the qjack_thread_creator() function in both
> cases.
> 
> The qjack_thread_creator() function just sets the name of the
> created thread for debugging purposes and isn't really necessary.

As far as Windows is concerned, right, there is no jack_set_thread_creator().

I find it actually very useful to have named threads for debugging purposes, 
especially for processes that have a load of threads, but I see there would be 
no easy solution to preserve this for macOS, as on macOS pthread_setname_np() 
must be called from the to be named thread itself, and the thread's entry 
point is on JACK server side, so for now ...

Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>

> From the jack_set_thread_creator() documentation:
> (...)
> 
> No normal application/client should consider calling this. (...)
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/785
> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
> ---
>  audio/jackaudio.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/audio/jackaudio.c b/audio/jackaudio.c
> index e7de6d5433..317009e936 100644
> --- a/audio/jackaudio.c
> +++ b/audio/jackaudio.c
> @@ -622,6 +622,7 @@ static void qjack_enable_in(HWVoiceIn *hw, bool enable)
>      ji->c.enabled = enable;
>  }
> 
> +#if !defined(WIN32) && defined(CONFIG_PTHREAD_SETNAME_NP_W_TID)
>  static int qjack_thread_creator(jack_native_thread_t *thread,
>      const pthread_attr_t *attr, void *(*function)(void *), void *arg)
>  {
> @@ -635,6 +636,7 @@ static int qjack_thread_creator(jack_native_thread_t
> *thread,
> 
>      return ret;
>  }
> +#endif
> 
>  static void *qjack_init(Audiodev *dev)
>  {
> @@ -687,7 +689,9 @@ static void register_audio_jack(void)
>  {
>      qemu_mutex_init(&qjack_shutdown_lock);
>      audio_driver_register(&jack_driver);
> +#if !defined(WIN32) && defined(CONFIG_PTHREAD_SETNAME_NP_W_TID)
>      jack_set_thread_creator(qjack_thread_creator);
> +#endif
>      jack_set_error_function(qjack_error);
>      jack_set_info_function(qjack_info);
>  }

[PATCH 2/3] dsoundaudio: fix crackling audio recordings

[Volker Rümelin]

Audio recordings with the DirectSound backend don't sound right.
A look a the Microsoft online documentation tells us why.

From the DirectSound Programming Guide, Capture Buffer Information:
'You can safely copy data from the buffer only up to the read
cursor.'

Change the code to read up to the read cursor instead of the
capture cursor.

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

diff --git a/audio/dsoundaudio.c b/audio/dsoundaudio.c
index cfc79c129e..3dd2c4d4a6 100644
--- a/audio/dsoundaudio.c
+++ b/audio/dsoundaudio.c
@@ -536,13 +536,12 @@ static void *dsound_get_buffer_in(HWVoiceIn *hw, size_t *size)
     DSoundVoiceIn *ds = (DSoundVoiceIn *) hw;
     LPDIRECTSOUNDCAPTUREBUFFER dscb = ds->dsound_capture_buffer;
     HRESULT hr;
-    DWORD cpos, rpos, act_size;
+    DWORD rpos, act_size;
     size_t req_size;
     int err;
     void *ret;
 
-    hr = IDirectSoundCaptureBuffer_GetCurrentPosition(
-        dscb, &cpos, ds->first_time ? &rpos : NULL);
+    hr = IDirectSoundCaptureBuffer_GetCurrentPosition(dscb, NULL, &rpos);
     if (FAILED(hr)) {
         dsound_logerr(hr, "Could not get capture buffer position\n");
         *size = 0;
@@ -554,7 +553,7 @@ static void *dsound_get_buffer_in(HWVoiceIn *hw, size_t *size)
         ds->first_time = false;
     }
 
-    req_size = audio_ring_dist(cpos, hw->pos_emul, hw->size_emul);
+    req_size = audio_ring_dist(rpos, hw->pos_emul, hw->size_emul);
     req_size = MIN(*size, MIN(req_size, hw->size_emul - hw->pos_emul));
 
     if (req_size == 0) {
-- 
2.31.1

[PATCH 3/3] hw/audio/intel-hda: fix stream reset

[Volker Rümelin]

Quote from:
High Definition Audio Specification 1.0a, section 3.3.35

Offset 80: {IOB}SDnCTL Stream Reset (SRST): Writing a 1 causes
the corresponding stream to be reset. The Stream Descriptor
registers (except the SRST bit itself) ... are reset.

Change the code to reset the Stream Descriptor Control and Status
registers except the SRST bit.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/757
Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 hw/audio/intel-hda.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index 8ce9df64e3..eed81f9023 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -578,7 +578,7 @@ static void intel_hda_set_st_ctl(IntelHDAState *d, const IntelHDAReg *reg, uint3
     if (st->ctl & 0x01) {
         /* reset */
         dprint(d, 1, "st #%d: reset\n", reg->stream);
-        st->ctl = SD_STS_FIFO_READY << 24;
+        st->ctl = SD_STS_FIFO_READY << 24 | SD_CTL_STREAM_RESET;
     }
     if ((st->ctl & 0x02) != (old & 0x02)) {
         uint32_t stnr = (st->ctl >> 20) & 0x0f;
-- 
2.31.1