qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] DO-NOT-MERGE: pipewire sample code


From: Dorinda Bassey
Subject: Re: [PATCH] DO-NOT-MERGE: pipewire sample code
Date: Tue, 14 Mar 2023 12:50:30 +0100

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_qemu@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_qemu@t-online.de>
> wrote:
>
>     > Based-on:<20230306171020.381116-1-dbassey@redhat.com" target="_blank">20230306171020.381116-1-dbassey@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_qemu@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.
>


reply via email to

[Prev in Thread] Current Thread [Next in Thread]