Hi Volker, Filling a buffer with zeros to produce silence still wrong for unsigned > samples. For example, a 0 in SPA_AUDIO_FORMAT_U8 format maps to -1.0 in > SPA_AUDIO_FORMAT_F32. > This is a bug. On a buffer underrun, the buffer filled with silence is > dropped. > What are your suggestions to improve this?
Why don't you need a lock here? Is pw_stream_set_active() thread safe? > I will put a lock there, Thanks. You only have the three volume levels 2.0, 1.0 and 0.0 while vol[i] has > 256 levels. > Ack. 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. > Ack <http://out.name> options. Please > Can you please clarify WYM here? Thanks, Dorinda On Mon, Apr 3, 2023 at 8:51 AM Volker Rümelin <vr_q...@t-online.de> wrote: > 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) "" > > > > > > >
