[v2,00/17] audio: improve callback interface for audio frontends

Message

Volker Rümelin Feb. 6, 2023, 6:47 p.m. UTC

Based-on: <[e-mail]>
([PATCH v2 00/11] audio: more improvements)

The callback interface for emulated audio devices is strange. The 
callback function has an 'avail' parameter that passes the number of 
bytes that can be written or read. Unfortunately, this value sometimes 
is only an imprecise estimate and the callback functions must check the 
actual bytes written or read. For playback devices, this means that they 
either need a ring buffer or have to write the unwritten bytes again the 
next time. For recording devices, things are a bit easier. They only 
need to continue with the actual number of bytes read.

After this patch series, the 'avail' argument for the -audiodev 
out.mixing-engine=on and in.mixing-engine=on cases is exact. Audio 
frontends only need a linear frame buffer and there's a guarantee they 
can write or read 'avail' bytes.

The -audiodev out.mixing-engine=off case is also mostly accurate. Only 
the D-Bus audio backend is still missing a required function. The 
-audiodev in.mixing-engine=off case always passes a much too large 
'avail' value. I haven't worked on this yet, because there was no reason 
for it so far.

The following logs show the improvements. Not only the audio frontends 
can write or read all needed or available bytes. The same is true for 
the audio backends. For playback, the first six lines in the logs are 
expected. Here you can see how quickly the guest fills the empty 
downstream buffers after playback starts.

QEMU was started with -device ich9-intel-hda,addr=0x1b -device 
hda-duplex,audiodev=audio0 -audiodev 
pa,out.frequency=96000,in.frequency=96000,id=audio0

playback guest 44100Hz => host 96000Hz

unpatched version:
hda_audio_output_cb: to write 8188, written 1704
audio_run_out: free 4458, played 926
hda_audio_output_cb: to write 6488, written 2384
audio_run_out: free 3532, played 1297
hda_audio_output_cb: to write 4104, written 2648
audio_run_out: free 2235, played 1441
audio_run_out: free 794, played 793
audio_run_out: free 897, played 896
audio_run_out: free 831, played 829
...
hda_audio_output_cb: could not write 4 bytes
hda_audio_output_cb: to write 1764, written 1760
audio_run_out: free 960, played 958
...

patched version:
hda_audio_output_cb: to write 8192, written 1620
audio_run_out: free 4458, played 880
hda_audio_output_cb: to write 6576, written 2508
audio_run_out: free 3578, played 1365
hda_audio_output_cb: to write 4068, written 2500
audio_run_out: free 2213, played 1360

record host 96000Hz => guest 44100Hz

unpatched version:
audio_run_in: avail 4458, acquired 4454
audio_run_in: avail 1574, acquired 1572
audio_run_in: avail 766, acquired 764
audio_run_in: avail 1052, acquired 1051
audio_run_in: avail 761, acquired 760
audio_run_in: avail 1123, acquired 1121
...
hda_audio_input_cb: could not read 4 bytes
hda_audio_input_cb: to read 1988, read 1984
audio_run_in: avail 1082, acquired 1080
...

patched version:
(no output)

QEMU was started with -device ich9-intel-hda,addr=0x1b -device 
hda-duplex,audiodev=audio0 -audiodev 
pa,out.frequency=32000,in.frequency=32000,id=audio0

playback guest 44100Hz => host 32000Hz

unpatched version:
hda_audio_output_cb: to write 8188, written 1620
audio_run_out: free 1486, played 294
hda_audio_output_cb: to write 6568, written 2512
audio_run_out: free 1192, played 455
hda_audio_output_cb: to write 4060, written 2504
audio_run_out: free 737, played 455
audio_run_out: free 282, played 281
audio_run_out: free 357, played 356
audio_run_out: free 314, played 313
...
hda_audio_output_cb: could not write 4 bytes
hda_audio_output_cb: to write 1416, written 1412
audio_run_out: free 257, played 256
...

patched version:
hda_audio_output_cb: to write 8192, written 1656
audio_run_out: free 1486, played 300
hda_audio_output_cb: to write 6536, written 2516
audio_run_out: free 1186, played 457
hda_audio_output_cb: to write 4020, written 2540
audio_run_out: free 729, played 460

record host 32000Hz => guest 44100Hz

unpatched version:
audio_run_in: avail 1486, acquired 1485
audio_run_in: avail 272, acquired 271
audio_run_in: avail 366, acquired 365
hda_audio_input_cb: could not read 4 bytes
hda_audio_input_cb: to read 1420, read 1416
audio_run_in: avail 258, acquired 257
audio_run_in: avail 375, acquired 374
hda_audio_input_cb: could not read 4 bytes
hda_audio_input_cb: to read 2056, read 2052
audio_run_in: avail 260, acquired 259
...

patched version:
(no output)

This is the debug code for the logs above.

---snip--
 > --- a/audio/audio.c    2022-12-13 19:14:31.793153558 +0100
 > +++ b/audio/audio.c    2022-12-11 16:24:48.842649711 +0100
 > @@ -1228,6 +1228,10 @@ static void audio_run_out (AudioState *s
 >  #ifdef DEBUG_OUT
 >          dolog("played=%zu\n", played);
 >  #endif
 > +        if (hw_free - played) {
 > +            fprintf(stderr, "%s: free %zu, played %zu\n",
 > +                    __func__, hw_free, played);
 > +        }
 >
 >          if (played) {
 >              hw->ts_helper += played;
 > @@ -1318,6 +1322,7 @@ static void audio_run_in (AudioState *s)
 >              if (sw->active) {
 >                  size_t sw_avail = audio_get_avail(sw);
 >                  size_t avail;
 > +                size_t prev_acquired = sw->total_hw_samples_acquired;
 >
 >                  avail = st_rate_frames_out(sw->rate, sw_avail);
 >                  if (avail > 0) {
 > @@ -1325,6 +1330,11 @@ static void audio_run_in (AudioState *s)
 > sw->callback.fn(sw->callback.opaque,
 >                                      avail * sw->info.bytes_per_frame);
 >                  }
 > +
 > +                if (sw_avail + prev_acquired - 
sw->total_hw_samples_acquired) {
 > +                    fprintf(stderr, "%s: avail %zu, acquired %zu\n", 
__func__,
 > +                            sw_avail, sw->total_hw_samples_acquired 
- prev_acquired);
 > +                }
 >              }
 >          }
 >      }
 > --- a/hw/audio/hda-codec.c    2023-01-04 14:07:31.954304889 +0100
 > +++ b/hw/audio/hda-codec.c    2023-01-04 13:57:47.687320406 +0100
 > @@ -265,20 +265,28 @@ static void hda_audio_input_cb(void *opa
 >      int64_t rpos = st->rpos;
 >
 >      int64_t to_transfer = MIN(B_SIZE - (wpos - rpos), avail);
 > +    unsigned int total_read = 0;
 >
 >      while (to_transfer) {
 >          uint32_t start = (uint32_t) (wpos & B_MASK);
 >          uint32_t chunk = (uint32_t) MIN(B_SIZE - start, to_transfer);
 >          uint32_t read = AUD_read(st->voice.in, st->buf + start, chunk);
 >          wpos += read;
 > +        total_read += read;
 >          to_transfer -= read;
 >          st->wpos += read;
 >          if (chunk != read) {
 > +            fprintf(stderr, "%s: could not read %u bytes\n", __func__,
 > +                    chunk - read);
 >              break;
 >          }
 >      }
 >
 >      hda_timer_sync_adjust(st, -((wpos - rpos) - (B_SIZE >> 1)));
 > +    if (avail != total_read) {
 > +        fprintf(stderr, "%s: to read %d, read %u\n", __func__,
 > +                avail, total_read);
 > +    }
 >  }
 >
 >  static void hda_audio_output_timer(void *opaque)
 > @@ -329,6 +337,7 @@ static void hda_audio_output_cb(void *op
 >      int64_t rpos = st->rpos;
 >
 >      int64_t to_transfer = MIN(wpos - rpos, avail);
 > +    unsigned int total_written = 0;
 >
 >      if (wpos - rpos == B_SIZE) {
 >          /* drop buffer, reset timer adjust */
 > @@ -343,15 +352,22 @@ static void hda_audio_output_cb(void *op
 >          uint32_t start = (uint32_t) (rpos & B_MASK);
 >          uint32_t chunk = (uint32_t) MIN(B_SIZE - start, to_transfer);
 >          uint32_t written = AUD_write(st->voice.out, st->buf + start, 
chunk);
 > +        total_written += written;
 >          rpos += written;
 >          to_transfer -= written;
 >          st->rpos += written;
 >          if (chunk != written) {
 > +            fprintf(stderr, "%s: could not write %u bytes\n", __func__,
 > +                    chunk - written);
 >              break;
 >          }
 >      }
 >
 >      hda_timer_sync_adjust(st, (wpos - rpos) - (B_SIZE >> 1));
 > +    if (avail != total_written) {
 > +        fprintf(stderr, "%s: to write %d, written %u\n", __func__,
 > +                avail, total_written);
 > +    }
 >  }
 >
 >  static void hda_audio_compat_input_cb(void *opaque, int avail)
---snip--

v2:
The patch series was rebased onto [PATCH v2 00/11] audio: more 
improvements. Patch 15/17 (audio: handle leftover audio frame from 
upsampling) and patch 17/17 (audio: remove sw->ratio) needed changes 
because of this.

Volker Rümelin (17):
   audio: change type of mix_buf and conv_buf
   audio: change type and name of the resample buffer
   audio: make the resampling code greedy
   audio: replace the resampling loop in audio_pcm_sw_write()
   audio: remove sw == NULL check
   audio: rename variables in audio_pcm_sw_write()
   audio: don't misuse audio_pcm_sw_write()
   audio: remove unused noop_conv() function
   audio/mixeng: calculate number of input frames
   audio: wire up st_rate_frames_in()
   audio: replace the resampling loop in audio_pcm_sw_read()
   audio: rename variables in audio_pcm_sw_read()
   audio/mixeng: calculate number of output frames
   audio: wire up st_rate_frames_out()
   audio: handle leftover audio frame from upsampling
   audio/audio_template: substitute sw->hw with hw
   audio: remove sw->ratio

  audio/audio.c          | 366 +++++++++++++++++++++--------------------
  audio/audio_int.h      |  12 +-
  audio/audio_template.h |  61 ++++---
  audio/mixeng.c         |  73 ++++++++
  audio/mixeng.h         |   2 +
  audio/rate_template.h  |  21 ++-
  6 files changed, 314 insertions(+), 221 deletions(-)