Am 28.03.23 um 13:56 schrieb Dorinda Bassey:
Hi Dorinda,
Hi Volker,
Thanks for the feedback.
This term is constant for the lifetime of the playback stream. It
could
be precalculated in qpw_init_out().
It's still constant even when precalculated in qpw_init_out().
It's an optimization. Evaluating req = (uint64_t)v->g->dev->timer_period
* v->info.rate * 1 / 2 / 1000000 * v->frame_size once in qpw_init_out()
vs. a lot of needless evaluations every few milliseconds in the callback.
With best regards,
Volker
The if (!v->enabled) block isn't needed. When the guest stops the
playback stream, it won't write new samples. After the pipewire
ringbuffer is drained, avail is always 0. It's better to drain the
ringbuffer, otherwise the first thing you will hear after playback
starts again will be stale audio samples.
You removed the code to play silence on a buffer underrun. I
suggest to
add it again. Use a trace point with the "simple" trace backend to
see
how often pipewire now calls the callback in short succession for a
disabled stream before giving up. Please read again Marc-André's
comments for the v7 version of the
pipewire backend. When the guest enables/disables an audio stream,
pipewire should know this. It's unnecessary that pipewire calls the
callback code for disabled streams. Don't forget to connect the
stream
with the flag PW_STREAM_FLAG_INACTIVE. Every QEMU audio device
enables
the stream before playback/recording starts. The pcm_ops functions
volume_out and volume_in are missing. Probably
SPA_PROP_channelVolumes can be used to adjust the stream volumes.
Without these functions the guest can adjust the stream volume and
the
host has an independent way to adjust the stream volume. This is
sometimes irritating.
The pipewire backend code doesn't use the in|out.name
<http://out.name> options. Please
either remove the name options or add code to connect to the
specified
source/sink. I would prefer the latter. PW_KEY_TARGET_OBJECT looks
promising.
Ack.
Thanks,
Dorinda.
On Mon, Mar 20, 2023 at 7:31 AM Volker Rümelin <vr_q...@t-online.de>
wrote:
> diff --git a/audio/trace-events b/audio/trace-events
> index e1ab643add..e0acf9ac56 100644
> --- a/audio/trace-events
> +++ b/audio/trace-events
> @@ -18,6 +18,13 @@ dbus_audio_register(const char *s, const char
*dir) "sender = %s, dir = %s"
> dbus_audio_put_buffer_out(size_t len) "len = %zu"
> dbus_audio_read(size_t len) "len = %zu"
>
> +# pwaudio.c
> +pw_state_changed(const char *s) "stream state: %s"
> +pw_node(int nodeid) "node id: %d"
> +pw_read(int32_t avail, uint32_t index, size_t len) "avail=%u
index=%u len=%zu"
> +pw_write(int32_t filled, int32_t avail, uint32_t index, size_t
len) "filled=%u avail=%u index=%u len=%zu"
> +pw_audio_init(void) "Initialize Pipewire context"
> +
Hi Dorinda,
the format specifiers and parameter types don't always match.
With best regards,
Volker
> # audio.c
> audio_timer_start(int interval) "interval %d ms"
> audio_timer_stop(void) ""
>
