Hi Volker,
Thank you for the clarification. I see the problem now.
So is it safe to say that:
@@ -104,8 +104,9 @@ playback_on_process(void *data)
/* calculate the total no of bytes to read data from buffer */
req = b->requested * v->frame_size;
if (req == 0) {
- req = 4096 * v->frame_size;
+ req = v->g->dev->timer_period * v->info.rate * v->frame_size * 1 /
2 / 1000000;
}
I used 50% of the timer-period and the frame_size which would give a close
margin to what the downstream audio device writes to the DAC.
Thanks,
Dorinda.
On Mon, Mar 13, 2023 at 9:06 PM Volker Rümelin <vr_q...@t-online.de> wrote:
> Am 13.03.23 um 13:28 schrieb Dorinda Bassey:
> > Hi Volker,
> >
> > Thanks for the patch, I've tested the patch and it works. I don't hear
> > the choppy audio with this option "qemu-system-x86_64 -device
> > ich9-intel-hda -device hda-duplex,audiodev=audio0 -audiodev
> > pipewire,id=audio0,out.frequency=96000,in.frequency=96000 ...."
> >
> > I don't understand how the req == 0 case can work at all.
> >
> > how this works is that b->requested could be zero when no suggestion
> > is provided. For playback streams, this field contains the suggested
> > amount of data to provide. hence the reason for this check.
>
> Hi Dorinda,
>
> there has to be a control mechanism that ensures that our write rate on
> average is exactly the frame rate that the down stream audio device
> writes to the DAC. My question was how can this work if we always write
> 4096 frames.
>
> The answer is, that after a 4096 frames write, the callback is delayed
> by 4096 frames / 44100 frames/s = 93ms. This ensures that our write rate
> is exactly 44100 frames/s.
>
> This means a fixed 4096 frames write is wrong for the req == 0 case. We
> have to write 75% of timer-period frames.
>
> If you want to test this yourself, just ignore req and assume it's 0.
>
> With best regards,
> Volker
>
> >
> > I suggest to use the same option names as the pulseaudio backend.
> > out.latency is the effective Pipewire buffer size.
> >
> > Ack.
> >
> > Thanks,
> > Dorinda.
> >
> >
> > On Sat, Mar 11, 2023 at 5:19 PM Volker Rümelin <vr_q...@t-online.de>
> > wrote:
> >
> > > Based-on:<20230306171020.381116-1-dbas...@redhat.com>
> > > ([PATCH v7] audio/pwaudio.c: Add Pipewire audio backend for QEMU)
> > >
> > > This is sample code for the review of the pipewire backed. The
> > > code actually works.
> > >
> > > An email with explanations for the changes will follow.
> > >
> > > Signed-off-by: Volker Rümelin<vr_q...@t-online.de>
> > > ---
> > > audio/pwaudio.c | 67
> > +++++++++++++++++++++++++++++++++----------------
> > > qapi/audio.json | 10 +++-----
> > > 2 files changed, 49 insertions(+), 28 deletions(-)
> > >
> > > diff --git a/audio/pwaudio.c b/audio/pwaudio.c
> > > index d357761152..8e2a38938f 100644
> > > --- a/audio/pwaudio.c
> > > +++ b/audio/pwaudio.c
> > > @@ -23,7 +23,6 @@
> > > #define AUDIO_CAP "pipewire"
> > > #define RINGBUFFER_SIZE (1u << 22)
> > > #define RINGBUFFER_MASK (RINGBUFFER_SIZE - 1)
> > > -#define BUFFER_SAMPLES 512
> > >
> > > #include "audio_int.h"
> > >
> > > @@ -48,6 +47,7 @@ typedef struct PWVoice {
> > > struct pw_stream *stream;
> > > struct spa_hook stream_listener;
> > > struct spa_audio_info_raw info;
> > > + uint32_t highwater_mark;
> > > uint32_t frame_size;
> > > struct spa_ringbuffer ring;
> > > uint8_t buffer[RINGBUFFER_SIZE];
> > > @@ -82,7 +82,7 @@ playback_on_process(void *data)
> > > void *p;
> > > struct pw_buffer *b;
> > > struct spa_buffer *buf;
> > > - uint32_t n_frames, req, index, n_bytes;
> > > + uint32_t req, index, n_bytes;
> > > int32_t avail;
> > >
> > > if (!v->stream) {
> > > @@ -105,8 +105,7 @@ playback_on_process(void *data)
> > > if (req == 0) {
> > > req = 4096 * v->frame_size;
> > > }
> >
> > I don't understand how the req == 0 case can work at all. The
> > downstream
> > audio device is the thinnest point in the playback stream. We can't
> > write more audio frames than the audio device will consume.
> >
>
>
