[PATCH 0/7] audio: Spring cleaning

[PATCH 0/7] audio: Spring cleaning

[Philippe Mathieu-Daudé]

Cleaning old branches, salvaging what seems worthwhile...
This series is from the time I wanted cleaner buffer handling
to avoid abuses, started with chardev/ but got lost with reviews.
audio/ is smaller, so easier.

- Convert various prototypes to use const buffers
- Expose 'audio/audio.h' via public include directory.

Philippe Mathieu-Daudé (7):
  audio: Let audio_sample_to_uint64() use const samples argument
  audio: Let capture_callback handler use const buffer argument
  audio: Move advance() helper to 'audio_int.h'
  audio: Split advance() helper as in() and out()
  audio: Let HWVoice write() handlers take a const buffer
  audio: Let AUD_write() use a const buffer argument
  audio: Expose 'audio/audio.h' under the include/ directory

 audio/audio_int.h                | 25 +++++++++++++++++++------
 {audio => include/audio}/audio.h | 14 ++++----------
 audio/alsaaudio.c                |  8 ++++----
 audio/audio.c                    | 19 ++++++++++---------
 audio/audio_legacy.c             |  2 +-
 audio/audio_win_int.c            |  2 +-
 audio/coreaudio.c                |  7 ++++---
 audio/dsoundaudio.c              |  4 ++--
 audio/mixeng.c                   |  6 +++---
 audio/noaudio.c                  |  4 ++--
 audio/ossaudio.c                 | 10 +++++-----
 audio/paaudio.c                  |  6 +++---
 audio/sdlaudio.c                 |  8 +++++---
 audio/spiceaudio.c               |  4 ++--
 audio/wavaudio.c                 |  4 ++--
 audio/wavcapture.c               |  4 ++--
 ui/vnc.c                         |  2 +-
 MAINTAINERS                      |  1 +
 18 files changed, 71 insertions(+), 59 deletions(-)
 rename {audio => include/audio}/audio.h (95%)

-- 
2.21.3

[PATCH 1/7] audio: Let audio_sample_to_uint64() use const samples argument

[Philippe Mathieu-Daudé]

The samples are the input to convert to u64. As we should
not modify them, mark the argument const.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 audio/audio.h  | 2 +-
 audio/mixeng.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/audio/audio.h b/audio/audio.h
index 0db3c7dd5e..f27a12298f 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -163,7 +163,7 @@ int wav_start_capture(AudioState *state, CaptureState *s, const char *path,
 bool audio_is_cleaning_up(void);
 void audio_cleanup(void);
 
-void audio_sample_to_uint64(void *samples, int pos,
+void audio_sample_to_uint64(const void *samples, int pos,
                             uint64_t *left, uint64_t *right);
 void audio_sample_from_uint64(void *samples, int pos,
                             uint64_t left, uint64_t right);
diff --git a/audio/mixeng.c b/audio/mixeng.c
index 739a500449..75f000e49f 100644
--- a/audio/mixeng.c
+++ b/audio/mixeng.c
@@ -338,10 +338,10 @@ f_sample *mixeng_clip_float[2] = {
     clip_natural_float_from_stereo,
 };
 
-void audio_sample_to_uint64(void *samples, int pos,
+void audio_sample_to_uint64(const void *samples, int pos,
                             uint64_t *left, uint64_t *right)
 {
-    struct st_sample *sample = samples;
+    const struct st_sample *sample = samples;
     sample += pos;
 #ifdef FLOAT_MIXENG
     error_report(
-- 
2.21.3

[PATCH 2/7] audio: Let capture_callback handler use const buffer argument

[Philippe Mathieu-Daudé]

The buffer is the captured input to pass to backends.
As we should not modify it, mark the argument const.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 audio/audio.h      | 2 +-
 audio/wavcapture.c | 2 +-
 ui/vnc.c           | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/audio/audio.h b/audio/audio.h
index f27a12298f..b883ebfb1f 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -60,7 +60,7 @@ typedef enum {
 
 struct audio_capture_ops {
     void (*notify) (void *opaque, audcnotification_e cmd);
-    void (*capture) (void *opaque, void *buf, int size);
+    void (*capture) (void *opaque, const void *buf, int size);
     void (*destroy) (void *opaque);
 };
 
diff --git a/audio/wavcapture.c b/audio/wavcapture.c
index 8d7ce2eda1..17e87ed6f4 100644
--- a/audio/wavcapture.c
+++ b/audio/wavcapture.c
@@ -71,7 +71,7 @@ static void wav_destroy (void *opaque)
     g_free (wav->path);
 }
 
-static void wav_capture (void *opaque, void *buf, int size)
+static void wav_capture(void *opaque, const void *buf, int size)
 {
     WAVState *wav = opaque;
 
diff --git a/ui/vnc.c b/ui/vnc.c
index 1d7138a3a0..12a12714e1 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1177,7 +1177,7 @@ static void audio_capture_destroy(void *opaque)
 {
 }
 
-static void audio_capture(void *opaque, void *buf, int size)
+static void audio_capture(void *opaque, const void *buf, int size)
 {
     VncState *vs = opaque;
 
-- 
2.21.3

[PATCH 3/7] audio: Move advance() helper to 'audio_int.h'

[Philippe Mathieu-Daudé]

The advance() helper is only used by the audio backends.
Restrict its use by moving it to the "audio_int.h" header.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 audio/audio.h     | 6 ------
 audio/audio_int.h | 6 ++++++
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/audio/audio.h b/audio/audio.h
index b883ebfb1f..9d759d644b 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -151,12 +151,6 @@ int  AUD_is_active_in (SWVoiceIn *sw);
 void     AUD_init_time_stamp_in (SWVoiceIn *sw, QEMUAudioTimeStamp *ts);
 uint64_t AUD_get_elapsed_usec_in (SWVoiceIn *sw, QEMUAudioTimeStamp *ts);
 
-static inline void *advance (void *p, int incr)
-{
-    uint8_t *d = p;
-    return (d + incr);
-}
-
 int wav_start_capture(AudioState *state, CaptureState *s, const char *path,
                       int freq, int bits, int nchannels);
 
diff --git a/audio/audio_int.h b/audio/audio_int.h
index 4775857bf2..fb6947c435 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -255,6 +255,12 @@ static inline size_t audio_ring_dist(size_t dst, size_t src, size_t len)
     return (dst >= src) ? (dst - src) : (len - src + dst);
 }
 
+static inline void *advance(void *p, int incr)
+{
+    uint8_t *d = p;
+    return d + incr;
+}
+
 #define dolog(fmt, ...) AUD_log(AUDIO_CAP, fmt, ## __VA_ARGS__)
 
 #ifdef DEBUG
-- 
2.21.3

[PATCH 4/7] audio: Split advance() helper as in() and out()

[Philippe Mathieu-Daudé]

When the buffer is to be filled, rename as advance_in().
When the buffer is already filled, use advance_out().

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 audio/audio_int.h | 8 +++++++-
 audio/alsaaudio.c | 4 ++--
 audio/audio.c     | 2 +-
 audio/ossaudio.c  | 4 ++--
 4 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/audio/audio_int.h b/audio/audio_int.h
index fb6947c435..62829de4e8 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -255,12 +255,18 @@ static inline size_t audio_ring_dist(size_t dst, size_t src, size_t len)
     return (dst >= src) ? (dst - src) : (len - src + dst);
 }
 
-static inline void *advance(void *p, int incr)
+static inline void *advance_in(void *p, int incr)
 {
     uint8_t *d = p;
     return d + incr;
 }
 
+static inline const void *advance_out(const void *p, int incr)
+{
+    const uint8_t *d = p;
+    return d + incr;
+}
+
 #define dolog(fmt, ...) AUD_log(AUDIO_CAP, fmt, ## __VA_ARGS__)
 
 #ifdef DEBUG
diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
index a8e62542f9..a32db8ff39 100644
--- a/audio/alsaaudio.c
+++ b/audio/alsaaudio.c
@@ -615,7 +615,7 @@ static size_t alsa_write(HWVoiceOut *hw, void *buf, size_t len)
     size_t len_frames = len / hw->info.bytes_per_frame;
 
     while (len_frames) {
-        char *src = advance(buf, pos);
+        const char *src = advance_out(buf, pos);
         snd_pcm_sframes_t written;
 
         written = snd_pcm_writei(alsa->handle, src, len_frames);
@@ -809,7 +809,7 @@ static size_t alsa_read(HWVoiceIn *hw, void *buf, size_t len)
     size_t pos = 0;
 
     while (len) {
-        void *dst = advance(buf, pos);
+        void *dst = advance_in(buf, pos);
         snd_pcm_sframes_t nread;
 
         nread = snd_pcm_readi(
diff --git a/audio/audio.c b/audio/audio.c
index 7a9e680355..c75455bbb5 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -571,7 +571,7 @@ static void audio_pcm_hw_clip_out(HWVoiceOut *hw, void *pcm_buf, size_t len)
 
     while (len) {
         st_sample *src = hw->mix_buf->samples + pos;
-        uint8_t *dst = advance(pcm_buf, clipped * hw->info.bytes_per_frame);
+        uint8_t *dst = advance_in(pcm_buf, clipped * hw->info.bytes_per_frame);
         size_t samples_till_end_of_buf = hw->mix_buf->size - pos;
         size_t samples_to_clip = MIN(len, samples_till_end_of_buf);
 
diff --git a/audio/ossaudio.c b/audio/ossaudio.c
index f88d076ec2..7778138df5 100644
--- a/audio/ossaudio.c
+++ b/audio/ossaudio.c
@@ -439,7 +439,7 @@ static size_t oss_write(HWVoiceOut *hw, void *buf, size_t len)
     pos = 0;
     while (len) {
         ssize_t bytes_written;
-        void *pcm = advance(buf, pos);
+        const void *pcm = advance_out(buf, pos);
 
         bytes_written = write(oss->fd, pcm, len);
         if (bytes_written < 0) {
@@ -678,7 +678,7 @@ static size_t oss_read(HWVoiceIn *hw, void *buf, size_t len)
     while (len) {
         ssize_t nread;
 
-        void *dst = advance(buf, pos);
+        void *dst = advance_in(buf, pos);
         nread = read(oss->fd, dst, len);
 
         if (nread == -1) {
-- 
2.21.3

[PATCH 5/7] audio: Let HWVoice write() handlers take a const buffer

[Philippe Mathieu-Daudé]

The various write() and put_buffer() handlers should not
modify their buffer argument. Make it const.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 audio/audio_int.h   | 13 +++++++------
 audio/alsaaudio.c   |  2 +-
 audio/audio.c       | 11 ++++++-----
 audio/coreaudio.c   |  5 +++--
 audio/dsoundaudio.c |  2 +-
 audio/noaudio.c     |  2 +-
 audio/ossaudio.c    |  4 ++--
 audio/paaudio.c     |  4 ++--
 audio/sdlaudio.c    |  6 ++++--
 audio/spiceaudio.c  |  2 +-
 audio/wavaudio.c    |  2 +-
 11 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/audio/audio_int.h b/audio/audio_int.h
index 62829de4e8..d23722ae7c 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -152,7 +152,7 @@ struct audio_driver {
 struct audio_pcm_ops {
     int    (*init_out)(HWVoiceOut *hw, audsettings *as, void *drv_opaque);
     void   (*fini_out)(HWVoiceOut *hw);
-    size_t (*write)   (HWVoiceOut *hw, void *buf, size_t size);
+    size_t (*write)   (HWVoiceOut *hw, const void *buf, size_t size);
     void   (*run_buffer_out)(HWVoiceOut *hw);
     /*
      * get a buffer that after later can be passed to put_buffer_out; optional
@@ -165,7 +165,7 @@ struct audio_pcm_ops {
      * buf must be equal the pointer returned by get_buffer_out,
      * size may be smaller
      */
-    size_t (*put_buffer_out)(HWVoiceOut *hw, void *buf, size_t size);
+    size_t (*put_buffer_out)(HWVoiceOut *hw, const void *buf, size_t size);
     void   (*enable_out)(HWVoiceOut *hw, bool enable);
     void   (*volume_out)(HWVoiceOut *hw, Volume *vol);
 
@@ -173,17 +173,18 @@ struct audio_pcm_ops {
     void   (*fini_in) (HWVoiceIn *hw);
     size_t (*read)    (HWVoiceIn *hw, void *buf, size_t size);
     void  *(*get_buffer_in)(HWVoiceIn *hw, size_t *size);
-    void   (*put_buffer_in)(HWVoiceIn *hw, void *buf, size_t size);
+    void   (*put_buffer_in)(HWVoiceIn *hw, const void *buf, size_t size);
     void   (*enable_in)(HWVoiceIn *hw, bool enable);
     void   (*volume_in)(HWVoiceIn *hw, Volume *vol);
 };
 
 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_put_buffer_in(HWVoiceIn *hw, const void *buf, size_t size);
 void audio_generic_run_buffer_out(HWVoiceOut *hw);
 void *audio_generic_get_buffer_out(HWVoiceOut *hw, size_t *size);
-size_t audio_generic_put_buffer_out(HWVoiceOut *hw, void *buf, size_t size);
-size_t audio_generic_write(HWVoiceOut *hw, void *buf, size_t size);
+size_t audio_generic_put_buffer_out(HWVoiceOut *hw,
+                                    const void *buf, size_t size);
+size_t audio_generic_write(HWVoiceOut *hw, const void *buf, size_t size);
 size_t audio_generic_read(HWVoiceIn *hw, void *buf, size_t size);
 
 struct capture_callback {
diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
index a32db8ff39..7692ee5524 100644
--- a/audio/alsaaudio.c
+++ b/audio/alsaaudio.c
@@ -608,7 +608,7 @@ static int alsa_open(bool in, struct alsa_params_req *req,
     return -1;
 }
 
-static size_t alsa_write(HWVoiceOut *hw, void *buf, size_t len)
+static size_t alsa_write(HWVoiceOut *hw, const void *buf, size_t len)
 {
     ALSAVoiceOut *alsa = (ALSAVoiceOut *) hw;
     size_t pos = 0;
diff --git a/audio/audio.c b/audio/audio.c
index c75455bbb5..e2932da4f0 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1253,7 +1253,7 @@ static size_t audio_pcm_hw_run_in(HWVoiceIn *hw, size_t samples)
     while (samples) {
         size_t proc;
         size_t size = samples * hw->info.bytes_per_frame;
-        void *buf = hw->pcm_ops->get_buffer_in(hw, &size);
+        const void *buf = hw->pcm_ops->get_buffer_in(hw, &size);
 
         assert(size % hw->info.bytes_per_frame == 0);
         if (size == 0) {
@@ -1425,7 +1425,7 @@ void *audio_generic_get_buffer_in(HWVoiceIn *hw, size_t *size)
     return hw->buf_emul + start;
 }
 
-void audio_generic_put_buffer_in(HWVoiceIn *hw, void *buf, size_t size)
+void audio_generic_put_buffer_in(HWVoiceIn *hw, const void *buf, size_t size)
 {
     assert(size <= hw->pending_emul);
     hw->pending_emul -= size;
@@ -1468,7 +1468,8 @@ void *audio_generic_get_buffer_out(HWVoiceOut *hw, size_t *size)
     return hw->buf_emul + hw->pos_emul;
 }
 
-size_t audio_generic_put_buffer_out(HWVoiceOut *hw, void *buf, size_t size)
+size_t audio_generic_put_buffer_out(HWVoiceOut *hw,
+                                    const void *buf, size_t size)
 {
     assert(buf == hw->buf_emul + hw->pos_emul &&
            size + hw->pending_emul <= hw->size_emul);
@@ -1479,7 +1480,7 @@ size_t audio_generic_put_buffer_out(HWVoiceOut *hw, void *buf, size_t size)
     return size;
 }
 
-size_t audio_generic_write(HWVoiceOut *hw, void *buf, size_t size)
+size_t audio_generic_write(HWVoiceOut *hw, const void *buf, size_t size)
 {
     size_t dst_size, copy_size;
     void *dst = hw->pcm_ops->get_buffer_out(hw, &dst_size);
@@ -1491,7 +1492,7 @@ size_t audio_generic_write(HWVoiceOut *hw, void *buf, size_t size)
 
 size_t audio_generic_read(HWVoiceIn *hw, void *buf, size_t size)
 {
-    void *src = hw->pcm_ops->get_buffer_in(hw, &size);
+    const void *src = hw->pcm_ops->get_buffer_in(hw, &size);
 
     memcpy(buf, src, size);
     hw->pcm_ops->put_buffer_in(hw, src, size);
diff --git a/audio/coreaudio.c b/audio/coreaudio.c
index 4b4365660f..5258871c9c 100644
--- a/audio/coreaudio.c
+++ b/audio/coreaudio.c
@@ -412,9 +412,10 @@ static int coreaudio_unlock (coreaudioVoiceOut *core, const char *fn_name)
 COREAUDIO_WRAPPER_FUNC(get_buffer_out, void *, (HWVoiceOut *hw, size_t *size),
                        (hw, size))
 COREAUDIO_WRAPPER_FUNC(put_buffer_out, size_t,
-                       (HWVoiceOut *hw, void *buf, size_t size),
+                       (HWVoiceOut *hw, const void *buf, size_t size),
                        (hw, buf, size))
-COREAUDIO_WRAPPER_FUNC(write, size_t, (HWVoiceOut *hw, void *buf, size_t size),
+COREAUDIO_WRAPPER_FUNC(write, size_t,
+                       (HWVoiceOut *hw, const void *buf, size_t size),
                        (hw, buf, size))
 #undef COREAUDIO_WRAPPER_FUNC
 
diff --git a/audio/dsoundaudio.c b/audio/dsoundaudio.c
index 4cdf19ab67..bba6bafda4 100644
--- a/audio/dsoundaudio.c
+++ b/audio/dsoundaudio.c
@@ -454,7 +454,7 @@ static void *dsound_get_buffer_out(HWVoiceOut *hw, size_t *size)
     return ret;
 }
 
-static size_t dsound_put_buffer_out(HWVoiceOut *hw, void *buf, size_t len)
+static size_t dsound_put_buffer_out(HWVoiceOut *hw, const void *buf, size_t len)
 {
     DSoundVoiceOut *ds = (DSoundVoiceOut *) hw;
     LPDIRECTSOUNDBUFFER dsb = ds->dsound_buffer;
diff --git a/audio/noaudio.c b/audio/noaudio.c
index 05798ea210..21995c7d9b 100644
--- a/audio/noaudio.c
+++ b/audio/noaudio.c
@@ -41,7 +41,7 @@ typedef struct NoVoiceIn {
     RateCtl rate;
 } NoVoiceIn;
 
-static size_t no_write(HWVoiceOut *hw, void *buf, size_t len)
+static size_t no_write(HWVoiceOut *hw, const void *buf, size_t len)
 {
     NoVoiceOut *no = (NoVoiceOut *) hw;
     return audio_rate_get_bytes(&hw->info, &no->rate, len);
diff --git a/audio/ossaudio.c b/audio/ossaudio.c
index 7778138df5..97bde0256e 100644
--- a/audio/ossaudio.c
+++ b/audio/ossaudio.c
@@ -402,7 +402,7 @@ static void *oss_get_buffer_out(HWVoiceOut *hw, size_t *size)
     }
 }
 
-static size_t oss_put_buffer_out(HWVoiceOut *hw, void *buf, size_t size)
+static size_t oss_put_buffer_out(HWVoiceOut *hw, const void *buf, size_t size)
 {
     OSSVoiceOut *oss = (OSSVoiceOut *) hw;
     if (oss->mmapped) {
@@ -415,7 +415,7 @@ static size_t oss_put_buffer_out(HWVoiceOut *hw, void *buf, size_t size)
     }
 }
 
-static size_t oss_write(HWVoiceOut *hw, void *buf, size_t len)
+static size_t oss_write(HWVoiceOut *hw, const void *buf, size_t len)
 {
     OSSVoiceOut *oss = (OSSVoiceOut *) hw;
     size_t pos;
diff --git a/audio/paaudio.c b/audio/paaudio.c
index b052084698..b50df15ea7 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -125,7 +125,7 @@ unlock_and_fail:
     return NULL;
 }
 
-static void qpa_put_buffer_in(HWVoiceIn *hw, void *buf, size_t size)
+static void qpa_put_buffer_in(HWVoiceIn *hw, const void *buf, size_t size)
 {
     PAVoiceIn *p = (PAVoiceIn *) hw;
     PAConnection *c = p->g->conn;
@@ -228,7 +228,7 @@ unlock_and_fail:
     return NULL;
 }
 
-static size_t qpa_write(HWVoiceOut *hw, void *data, size_t length)
+static size_t qpa_write(HWVoiceOut *hw, const void *data, size_t length)
 {
     PAVoiceOut *p = (PAVoiceOut *) hw;
     PAConnection *c = p->g->conn;
diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c
index 21b7a0484b..9d740186cc 100644
--- a/audio/sdlaudio.c
+++ b/audio/sdlaudio.c
@@ -256,10 +256,12 @@ static void sdl_callback (void *opaque, Uint8 *buf, int len)
 SDL_WRAPPER_FUNC(get_buffer_out, void *, (HWVoiceOut *hw, size_t *size),
                  (hw, size), *size = 0, sdl_unlock)
 SDL_WRAPPER_FUNC(put_buffer_out, size_t,
-                 (HWVoiceOut *hw, void *buf, size_t size), (hw, buf, size),
+                 (HWVoiceOut *hw, const void *buf, size_t size),
+                 (hw, buf, size),
                  /*nothing*/, sdl_unlock_and_post)
 SDL_WRAPPER_FUNC(write, size_t,
-                 (HWVoiceOut *hw, void *buf, size_t size), (hw, buf, size),
+                 (HWVoiceOut *hw, const void *buf, size_t size),
+                 (hw, buf, size),
                  /*nothing*/, sdl_unlock_and_post)
 
 #undef SDL_WRAPPER_FUNC
diff --git a/audio/spiceaudio.c b/audio/spiceaudio.c
index b6b5da4812..0aa6a0a671 100644
--- a/audio/spiceaudio.c
+++ b/audio/spiceaudio.c
@@ -139,7 +139,7 @@ static void *line_out_get_buffer(HWVoiceOut *hw, size_t *size)
     return out->frame + out->fpos;
 }
 
-static size_t line_out_put_buffer(HWVoiceOut *hw, void *buf, size_t size)
+static size_t line_out_put_buffer(HWVoiceOut *hw, const void *buf, size_t size)
 {
     SpiceVoiceOut *out = container_of(hw, SpiceVoiceOut, hw);
 
diff --git a/audio/wavaudio.c b/audio/wavaudio.c
index 20e6853f85..64d7142a97 100644
--- a/audio/wavaudio.c
+++ b/audio/wavaudio.c
@@ -39,7 +39,7 @@ typedef struct WAVVoiceOut {
     int total_samples;
 } WAVVoiceOut;
 
-static size_t wav_write_out(HWVoiceOut *hw, void *buf, size_t len)
+static size_t wav_write_out(HWVoiceOut *hw, const void *buf, size_t len)
 {
     WAVVoiceOut *wav = (WAVVoiceOut *) hw;
     int64_t bytes = audio_rate_get_bytes(&hw->info, &wav->rate, len);
-- 
2.21.3

Re: [PATCH 5/7] audio: Let HWVoice write() handlers take a const buffer

[Volker Rümelin]

> diff --git a/audio/dsoundaudio.c b/audio/dsoundaudio.c
> index 4cdf19ab67..bba6bafda4 100644
> --- a/audio/dsoundaudio.c
> +++ b/audio/dsoundaudio.c
> @@ -454,7 +454,7 @@ static void *dsound_get_buffer_out(HWVoiceOut *hw, size_t *size)
>      return ret;
>  }
>  
> -static size_t dsound_put_buffer_out(HWVoiceOut *hw, void *buf, size_t len)
> +static size_t dsound_put_buffer_out(HWVoiceOut *hw, const void *buf, size_t len)
>  {
>      DSoundVoiceOut *ds = (DSoundVoiceOut *) hw;
>      LPDIRECTSOUNDBUFFER dsb = ds->dsound_buffer;

You forgot to make the buffer const in dsound_put_buffer_in().

I had to cast buf to LPVOID in dsound_get_buffer_in() and dsound_put_buffer_in() because otherwise I see:

C:/usr/msys64/home/ruemelin/git/qemu/audio/dsoundaudio.c: In function 'dsound_put_buffer_out':
C:/usr/msys64/home/ruemelin/git/qemu/audio/dsoundaudio.c:466:38: error: passing argument 2 of 'dsound_unlock_out' discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]
  466 |     int err = dsound_unlock_out(dsb, buf, NULL, len, 0);
      |                                      ^~~
In file included from C:/usr/msys64/home/ruemelin/git/qemu/audio/dsoundaudio.c:266:
C:/usr/msys64/home/ruemelin/git/qemu/audio/dsound_template.h:48:12: note: expected 'LPVOID' {aka 'void *'} but argument is of type 'const void *'
   48 |     LPVOID p1,
      |     ~~~~~~~^~
C:/usr/msys64/home/ruemelin/git/qemu/audio/dsoundaudio.c: In function 'dsound_put_buffer_in':
C:/usr/msys64/home/ruemelin/git/qemu/audio/dsoundaudio.c:571:38: error: passing argument 2 of 'dsound_unlock_in' discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]
  571 |     int err = dsound_unlock_in(dscb, buf, NULL, len, 0);
      |                                      ^~~
In file included from C:/usr/msys64/home/ruemelin/git/qemu/audio/dsoundaudio.c:268:
C:/usr/msys64/home/ruemelin/git/qemu/audio/dsound_template.h:48:12: note: expected 'LPVOID' {aka 'void *'} but argument is of type 'const void *'
   48 |     LPVOID p1,
      |     ~~~~~~~^~

With best regards,
Volker

Re: [PATCH 5/7] audio: Let HWVoice write() handlers take a const buffer

[Philippe Mathieu-Daudé]

On 5/6/20 8:22 AM, Volker Rümelin wrote:
> 
>> diff --git a/audio/dsoundaudio.c b/audio/dsoundaudio.c
>> index 4cdf19ab67..bba6bafda4 100644
>> --- a/audio/dsoundaudio.c
>> +++ b/audio/dsoundaudio.c
>> @@ -454,7 +454,7 @@ static void *dsound_get_buffer_out(HWVoiceOut *hw, size_t *size)
>>       return ret;
>>   }
>>   
>> -static size_t dsound_put_buffer_out(HWVoiceOut *hw, void *buf, size_t len)
>> +static size_t dsound_put_buffer_out(HWVoiceOut *hw, const void *buf, size_t len)
>>   {
>>       DSoundVoiceOut *ds = (DSoundVoiceOut *) hw;
>>       LPDIRECTSOUNDBUFFER dsb = ds->dsound_buffer;
> 
> You forgot to make the buffer const in dsound_put_buffer_in().
> 
> I had to cast buf to LPVOID in dsound_get_buffer_in() and dsound_put_buffer_in() because otherwise I see:
> 
> C:/usr/msys64/home/ruemelin/git/qemu/audio/dsoundaudio.c: In function 'dsound_put_buffer_out':
> C:/usr/msys64/home/ruemelin/git/qemu/audio/dsoundaudio.c:466:38: error: passing argument 2 of 'dsound_unlock_out' discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]
>    466 |     int err = dsound_unlock_out(dsb, buf, NULL, len, 0);
>        |                                      ^~~
> In file included from C:/usr/msys64/home/ruemelin/git/qemu/audio/dsoundaudio.c:266:
> C:/usr/msys64/home/ruemelin/git/qemu/audio/dsound_template.h:48:12: note: expected 'LPVOID' {aka 'void *'} but argument is of type 'const void *'
>     48 |     LPVOID p1,
>        |     ~~~~~~~^~
> C:/usr/msys64/home/ruemelin/git/qemu/audio/dsoundaudio.c: In function 'dsound_put_buffer_in':
> C:/usr/msys64/home/ruemelin/git/qemu/audio/dsoundaudio.c:571:38: error: passing argument 2 of 'dsound_unlock_in' discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]
>    571 |     int err = dsound_unlock_in(dscb, buf, NULL, len, 0);
>        |                                      ^~~
> In file included from C:/usr/msys64/home/ruemelin/git/qemu/audio/dsoundaudio.c:268:
> C:/usr/msys64/home/ruemelin/git/qemu/audio/dsound_template.h:48:12: note: expected 'LPVOID' {aka 'void *'} but argument is of type 'const void *'
>     48 |     LPVOID p1,
>        |     ~~~~~~~^~

OK thanks for testing. This is unfortunate, because a single backend 
invalidates the whole series.
I don't understand why the DirectSound API requires a writable buffer 
for locking.

> 
> With best regards,
> Volker
> 

[PATCH 6/7] audio: Let AUD_write() use a const buffer argument

[Philippe Mathieu-Daudé]

AUD_write() is to let backends process an existing data.
As we are not supposed to modify this buffer, make the
argument const.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 audio/audio.h | 2 +-
 audio/audio.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/audio/audio.h b/audio/audio.h
index 9d759d644b..7956b32d0a 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -113,7 +113,7 @@ SWVoiceOut *AUD_open_out (
     );
 
 void AUD_close_out (QEMUSoundCard *card, SWVoiceOut *sw);
-size_t AUD_write (SWVoiceOut *sw, void *pcm_buf, size_t size);
+size_t AUD_write (SWVoiceOut *sw, const void *pcm_buf, size_t size);
 int  AUD_get_buffer_size_out (SWVoiceOut *sw);
 void AUD_set_active_out (SWVoiceOut *sw, int on);
 int  AUD_is_active_out (SWVoiceOut *sw);
diff --git a/audio/audio.c b/audio/audio.c
index e2932da4f0..2672cf0885 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -703,7 +703,7 @@ static size_t audio_pcm_hw_get_live_out (HWVoiceOut *hw, int *nb_live)
 /*
  * Soft voice (playback)
  */
-static size_t audio_pcm_sw_write(SWVoiceOut *sw, void *buf, size_t size)
+static size_t audio_pcm_sw_write(SWVoiceOut *sw, const void *buf, size_t size)
 {
     size_t hwsamples, samples, isamp, osamp, wpos, live, dead, left, swlim, blck;
     size_t ret = 0, pos = 0, total = 0;
@@ -850,7 +850,7 @@ static void audio_timer (void *opaque)
 /*
  * Public API
  */
-size_t AUD_write(SWVoiceOut *sw, void *buf, size_t size)
+size_t AUD_write(SWVoiceOut *sw, const void *buf, size_t size)
 {
     HWVoiceOut *hw;
 
-- 
2.21.3

[PATCH 7/7] audio: Expose 'audio/audio.h' under the include/ directory

[Philippe Mathieu-Daudé]

The 'audio/audio.h' header is consumed by the hardware models:

  $ git grep -l audio/audio.h hw/
  hw/arm/omap2.c
  hw/arm/palm.c
  hw/arm/spitz.c
  hw/arm/z2.c
  hw/audio/ac97.c
  hw/audio/adlib.c
  hw/audio/cs4231a.c
  hw/audio/es1370.c
  hw/audio/gus.c
  hw/audio/hda-codec.c
  hw/audio/lm4549.c
  hw/audio/lm4549.h
  hw/audio/marvell_88w8618.c
  hw/audio/milkymist-ac97.c
  hw/audio/pcspk.c
  hw/audio/sb16.c
  hw/audio/wm8750.c
  hw/core/qdev-properties-system.c
  hw/input/tsc210x.c
  hw/mips/mips_fulong2e.c
  hw/usb/dev-audio.c

Move it to the public include/ directory.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 {audio => include/audio}/audio.h | 2 +-
 audio/alsaaudio.c                | 2 +-
 audio/audio.c                    | 2 +-
 audio/audio_legacy.c             | 2 +-
 audio/audio_win_int.c            | 2 +-
 audio/coreaudio.c                | 2 +-
 audio/dsoundaudio.c              | 2 +-
 audio/mixeng.c                   | 2 +-
 audio/noaudio.c                  | 2 +-
 audio/ossaudio.c                 | 2 +-
 audio/paaudio.c                  | 2 +-
 audio/sdlaudio.c                 | 2 +-
 audio/spiceaudio.c               | 2 +-
 audio/wavaudio.c                 | 2 +-
 audio/wavcapture.c               | 2 +-
 MAINTAINERS                      | 1 +
 16 files changed, 16 insertions(+), 15 deletions(-)
 rename {audio => include/audio}/audio.h (100%)

diff --git a/audio/audio.h b/include/audio/audio.h
similarity index 100%
rename from audio/audio.h
rename to include/audio/audio.h
index 7956b32d0a..b5c51d87b6 100644
--- a/audio/audio.h
+++ b/include/audio/audio.h
@@ -95,6 +95,7 @@ void AUD_log (const char *cap, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
 
 void AUD_register_card (const char *name, QEMUSoundCard *card);
 void AUD_remove_card (QEMUSoundCard *card);
+
 CaptureVoiceOut *AUD_add_capture(
     AudioState *s,
     struct audsettings *as,
@@ -111,7 +112,6 @@ SWVoiceOut *AUD_open_out (
     audio_callback_fn callback_fn,
     struct audsettings *settings
     );
-
 void AUD_close_out (QEMUSoundCard *card, SWVoiceOut *sw);
 size_t AUD_write (SWVoiceOut *sw, const void *pcm_buf, size_t size);
 int  AUD_get_buffer_size_out (SWVoiceOut *sw);
diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
index 7692ee5524..72b0a37dad 100644
--- a/audio/alsaaudio.c
+++ b/audio/alsaaudio.c
@@ -26,7 +26,7 @@
 #include <alsa/asoundlib.h>
 #include "qemu/main-loop.h"
 #include "qemu/module.h"
-#include "audio.h"
+#include "audio/audio.h"
 #include "trace.h"
 
 #pragma GCC diagnostic ignored "-Waddress"
diff --git a/audio/audio.c b/audio/audio.c
index 2672cf0885..d0fb794378 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -23,7 +23,7 @@
  */
 
 #include "qemu/osdep.h"
-#include "audio.h"
+#include "audio/audio.h"
 #include "migration/vmstate.h"
 #include "monitor/monitor.h"
 #include "qemu/timer.h"
diff --git a/audio/audio_legacy.c b/audio/audio_legacy.c
index ebd7d9fa0d..4321a84b47 100644
--- a/audio/audio_legacy.c
+++ b/audio/audio_legacy.c
@@ -22,7 +22,7 @@
  * THE SOFTWARE.
  */
 #include "qemu/osdep.h"
-#include "audio.h"
+#include "audio/audio.h"
 #include "audio_int.h"
 #include "qemu/cutils.h"
 #include "qemu/timer.h"
diff --git a/audio/audio_win_int.c b/audio/audio_win_int.c
index b938fd667b..cd8cde527b 100644
--- a/audio/audio_win_int.c
+++ b/audio/audio_win_int.c
@@ -7,7 +7,7 @@
 #include <windows.h>
 #include <mmsystem.h>
 
-#include "audio.h"
+#include "audio/audio.h"
 #include "audio_int.h"
 #include "audio_win_int.h"
 
diff --git a/audio/coreaudio.c b/audio/coreaudio.c
index 5258871c9c..f1656bb98b 100644
--- a/audio/coreaudio.c
+++ b/audio/coreaudio.c
@@ -27,7 +27,7 @@
 #include <pthread.h>            /* pthread_X */
 
 #include "qemu/module.h"
-#include "audio.h"
+#include "audio/audio.h"
 
 #define AUDIO_CAP "coreaudio"
 #include "audio_int.h"
diff --git a/audio/dsoundaudio.c b/audio/dsoundaudio.c
index bba6bafda4..3c0b4f0f9b 100644
--- a/audio/dsoundaudio.c
+++ b/audio/dsoundaudio.c
@@ -27,7 +27,7 @@
  */
 
 #include "qemu/osdep.h"
-#include "audio.h"
+#include "audio/audio.h"
 
 #define AUDIO_CAP "dsound"
 #include "audio_int.h"
diff --git a/audio/mixeng.c b/audio/mixeng.c
index 75f000e49f..5a47594fa0 100644
--- a/audio/mixeng.c
+++ b/audio/mixeng.c
@@ -25,7 +25,7 @@
 #include "qemu/osdep.h"
 #include "qemu/bswap.h"
 #include "qemu/error-report.h"
-#include "audio.h"
+#include "audio/audio.h"
 
 #define AUDIO_CAP "mixeng"
 #include "audio_int.h"
diff --git a/audio/noaudio.c b/audio/noaudio.c
index 21995c7d9b..e6a5e35060 100644
--- a/audio/noaudio.c
+++ b/audio/noaudio.c
@@ -25,7 +25,7 @@
 #include "qemu/osdep.h"
 #include "qemu/host-utils.h"
 #include "qemu/module.h"
-#include "audio.h"
+#include "audio/audio.h"
 #include "qemu/timer.h"
 
 #define AUDIO_CAP "noaudio"
diff --git a/audio/ossaudio.c b/audio/ossaudio.c
index 97bde0256e..7dddbd11d1 100644
--- a/audio/ossaudio.c
+++ b/audio/ossaudio.c
@@ -28,7 +28,7 @@
 #include "qemu/main-loop.h"
 #include "qemu/module.h"
 #include "qemu/host-utils.h"
-#include "audio.h"
+#include "audio/audio.h"
 #include "trace.h"
 
 #define AUDIO_CAP "oss"
diff --git a/audio/paaudio.c b/audio/paaudio.c
index b50df15ea7..63b97c93df 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -3,7 +3,7 @@
 #include "qemu/osdep.h"
 #include "qemu/module.h"
 #include "qemu-common.h"
-#include "audio.h"
+#include "audio/audio.h"
 #include "qapi/opts-visitor.h"
 
 #include <pulse/pulseaudio.h>
diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c
index 9d740186cc..4c4d58457d 100644
--- a/audio/sdlaudio.c
+++ b/audio/sdlaudio.c
@@ -26,7 +26,7 @@
 #include <SDL.h>
 #include <SDL_thread.h>
 #include "qemu/module.h"
-#include "audio.h"
+#include "audio/audio.h"
 
 #ifndef _WIN32
 #ifdef __sun__
diff --git a/audio/spiceaudio.c b/audio/spiceaudio.c
index 0aa6a0a671..5802f60d1d 100644
--- a/audio/spiceaudio.c
+++ b/audio/spiceaudio.c
@@ -25,7 +25,7 @@
 #include "ui/qemu-spice.h"
 
 #define AUDIO_CAP "spice"
-#include "audio.h"
+#include "audio/audio.h"
 #include "audio_int.h"
 
 #if SPICE_INTERFACE_PLAYBACK_MAJOR > 1 || SPICE_INTERFACE_PLAYBACK_MINOR >= 3
diff --git a/audio/wavaudio.c b/audio/wavaudio.c
index 64d7142a97..4241f1ad96 100644
--- a/audio/wavaudio.c
+++ b/audio/wavaudio.c
@@ -27,7 +27,7 @@
 #include "qemu/module.h"
 #include "qemu/timer.h"
 #include "qapi/opts-visitor.h"
-#include "audio.h"
+#include "audio/audio.h"
 
 #define AUDIO_CAP "wav"
 #include "audio_int.h"
diff --git a/audio/wavcapture.c b/audio/wavcapture.c
index 17e87ed6f4..c0ad928e80 100644
--- a/audio/wavcapture.c
+++ b/audio/wavcapture.c
@@ -2,7 +2,7 @@
 #include "monitor/monitor.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
-#include "audio.h"
+#include "audio/audio.h"
 
 typedef struct {
     FILE *f;
diff --git a/MAINTAINERS b/MAINTAINERS
index 8aa8efaf1d..b332611717 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1927,6 +1927,7 @@ M: Gerd Hoffmann <kraxel@redhat.com>
 S: Maintained
 F: audio/
 F: hw/audio/
+F: include/audio/
 F: include/hw/audio/
 F: tests/qtest/ac97-test.c
 F: tests/qtest/es1370-test.c
-- 
2.21.3

Re: [PATCH 0/7] audio: Spring cleaning

[Philippe Mathieu-Daudé]

On 5/5/20 3:25 PM, Philippe Mathieu-Daudé wrote:
> Cleaning old branches, salvaging what seems worthwhile...
> This series is from the time I wanted cleaner buffer handling
> to avoid abuses, started with chardev/ but got lost with reviews.
> audio/ is smaller, so easier.
> 
> - Convert various prototypes to use const buffers
> - Expose 'audio/audio.h' via public include directory.
> 
> Philippe Mathieu-Daudé (7):
>   audio: Let audio_sample_to_uint64() use const samples argument
>   audio: Let capture_callback handler use const buffer argument

Ping for 1 & 2 maybe?

>   audio: Move advance() helper to 'audio_int.h'
>   audio: Split advance() helper as in() and out()
>   audio: Let HWVoice write() handlers take a const buffer
>   audio: Let AUD_write() use a const buffer argument
>   audio: Expose 'audio/audio.h' under the include/ directory
> 
>  audio/audio_int.h                | 25 +++++++++++++++++++------
>  {audio => include/audio}/audio.h | 14 ++++----------
>  audio/alsaaudio.c                |  8 ++++----
>  audio/audio.c                    | 19 ++++++++++---------
>  audio/audio_legacy.c             |  2 +-
>  audio/audio_win_int.c            |  2 +-
>  audio/coreaudio.c                |  7 ++++---
>  audio/dsoundaudio.c              |  4 ++--
>  audio/mixeng.c                   |  6 +++---
>  audio/noaudio.c                  |  4 ++--
>  audio/ossaudio.c                 | 10 +++++-----
>  audio/paaudio.c                  |  6 +++---
>  audio/sdlaudio.c                 |  8 +++++---
>  audio/spiceaudio.c               |  4 ++--
>  audio/wavaudio.c                 |  4 ++--
>  audio/wavcapture.c               |  4 ++--
>  ui/vnc.c                         |  2 +-
>  MAINTAINERS                      |  1 +
>  18 files changed, 71 insertions(+), 59 deletions(-)
>  rename {audio => include/audio}/audio.h (95%)
> 

Re: [PATCH 0/7] audio: Spring cleaning

[Philippe Mathieu-Daudé]

Hi Gerd,

On 5/5/20 3:25 PM, Philippe Mathieu-Daudé wrote:
> Cleaning old branches, salvaging what seems worthwhile...
> This series is from the time I wanted cleaner buffer handling
> to avoid abuses, started with chardev/ but got lost with reviews.
> audio/ is smaller, so easier.
> 
> - Convert various prototypes to use const buffers
> - Expose 'audio/audio.h' via public include directory.

You didn't commented on this series.

We might use a LPVOID in dsound_get_buffer_in() as suggested
by Volker but I don't like it much. What do you think otherwise?

> 
> Philippe Mathieu-Daudé (7):
>   audio: Let audio_sample_to_uint64() use const samples argument
>   audio: Let capture_callback handler use const buffer argument
>   audio: Move advance() helper to 'audio_int.h'
>   audio: Split advance() helper as in() and out()
>   audio: Let HWVoice write() handlers take a const buffer
>   audio: Let AUD_write() use a const buffer argument
>   audio: Expose 'audio/audio.h' under the include/ directory

Re: [PATCH 0/7] audio: Spring cleaning

[Gerd Hoffmann]

On Mon, Jul 06, 2020 at 08:09:47PM +0200, Philippe Mathieu-Daudé wrote:
> Hi Gerd,
> 
> On 5/5/20 3:25 PM, Philippe Mathieu-Daudé wrote:
> > Cleaning old branches, salvaging what seems worthwhile...
> > This series is from the time I wanted cleaner buffer handling
> > to avoid abuses, started with chardev/ but got lost with reviews.
> > audio/ is smaller, so easier.
> > 
> > - Convert various prototypes to use const buffers
> > - Expose 'audio/audio.h' via public include directory.
> 
> You didn't commented on this series.
> 
> We might use a LPVOID in dsound_get_buffer_in() as suggested
> by Volker but I don't like it much. What do you think otherwise?

Well, your reply to Volker was "This is unfortunate, because a single
backend invalidates the whole series.", so I dropped it from my
TODO/review/merge list ...

Tagging the buffers as const, then cast the const away (even if only
in dsound) isn't a good idea IMHO.

take care,
  Gerd