From 7b6680ba5704c92933cf005d92e5bdf29d26bf37 Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Fri, 24 Mar 2023 11:20:01 +0100 Subject: [PATCH] plugins: simplify target_ handling Drivers should only read the target_ values in the timeout, update the timeout with the new duration and then update the position. For the position we simply need to add the previous duration to the position and then set the new duration + rate. Otherwise, everything else should read the duration/rate and not use the target_ values. --- spa/plugins/alsa/alsa-compress-offload-sink.c | 17 ++++--- spa/plugins/alsa/alsa-pcm.c | 33 +++++-------- spa/plugins/alsa/alsa-seq.c | 26 +++++----- spa/plugins/avb/avb-pcm.c | 49 ++++++++++--------- spa/plugins/bluez5/media-sink.c | 10 ++-- spa/plugins/bluez5/media-source.c | 45 ++++++++--------- spa/plugins/bluez5/midi-node.c | 24 ++++----- spa/plugins/bluez5/sco-sink.c | 10 ++-- spa/plugins/bluez5/sco-source.c | 43 +++++++--------- spa/plugins/support/node-driver.c | 13 ++--- spa/plugins/support/null-audio-sink.c | 10 ++-- src/pipewire/filter.c | 3 +- src/pipewire/stream.c | 3 +- 13 files changed, 132 insertions(+), 154 deletions(-) diff --git a/spa/plugins/alsa/alsa-compress-offload-sink.c b/spa/plugins/alsa/alsa-compress-offload-sink.c index 7bb069d43..b31105933 100644 --- a/spa/plugins/alsa/alsa-compress-offload-sink.c +++ b/spa/plugins/alsa/alsa-compress-offload-sink.c @@ -702,19 +702,24 @@ static void on_driver_timeout(struct spa_source *source) return; } } - if (SPA_LIKELY(this->node_position_io != NULL)) { - this->node_position_io->clock.duration = this->node_position_io->clock.target_duration; - this->node_position_io->clock.rate = this->node_position_io->clock.target_rate; - } - check_position_and_clock_config(this); + if (SPA_LIKELY(this->node_position_io != NULL)) { + this->cycle_duration = this->node_position_io->clock.target_duration; + this->cycle_rate = this->node_position_io->clock.target_rate.denom; + } else { + /* This can happen at the very beginning if node_position_io + * isn't passed to this node in time. */ + this->cycle_duration = 1024; + this->cycle_rate = 48000; + } current_time = this->next_driver_time; this->next_driver_time += ((uint64_t)(this->cycle_duration)) * 1000000000ULL / this->cycle_rate; if (this->node_clock_io != NULL) { this->node_clock_io->nsec = current_time; - this->node_clock_io->position += this->cycle_duration; + this->node_clock_io->rate = this->node_clock_io->target_rate; + this->node_clock_io->position += this->node_clock_io->duration; this->node_clock_io->duration = this->cycle_duration; this->node_clock_io->delay = 0; this->node_clock_io->rate_diff = 1.0; diff --git a/spa/plugins/alsa/alsa-pcm.c b/spa/plugins/alsa/alsa-pcm.c index 35a9047ab..52c46e948 100644 --- a/spa/plugins/alsa/alsa-pcm.c +++ b/spa/plugins/alsa/alsa-pcm.c @@ -1952,7 +1952,8 @@ static int update_time(struct state *state, uint64_t current_time, snd_pcm_sfram if (SPA_LIKELY(!follower && state->clock)) { state->clock->nsec = current_time; - state->clock->position += state->duration; + state->clock->rate = state->clock->target_rate; + state->clock->position += state->clock->duration; state->clock->duration = state->duration; state->clock->delay = delay + state->delay; state->clock->rate_diff = corr; @@ -1995,13 +1996,19 @@ static int setup_matching(struct state *state) static inline int check_position_config(struct state *state) { + uint64_t target_duration; + struct spa_fraction target_rate; + if (SPA_UNLIKELY(state->position == NULL)) return 0; - if (SPA_UNLIKELY((state->duration != state->position->clock.duration) || - (state->rate_denom != state->position->clock.rate.denom))) { - state->duration = state->position->clock.duration; - state->rate_denom = state->position->clock.rate.denom; + target_duration = state->position->clock.target_duration; + target_rate = state->position->clock.target_rate; + + if (SPA_UNLIKELY((state->duration != target_duration) || + (state->rate_denom != target_rate.denom))) { + state->duration = target_duration; + state->rate_denom = target_rate.denom; if (state->rate_denom == 0 || state->duration == 0) return -EIO; state->threshold = SPA_SCALE32_UP(state->duration, state->rate, state->rate_denom); @@ -2452,17 +2459,6 @@ static int handle_capture(struct state *state, uint64_t current_time, return 0; } -static void update_target(struct state *state) -{ - struct spa_io_position *pos; - - if (SPA_UNLIKELY((pos = state->position) == NULL)) - return; - - pos->clock.duration = pos->clock.target_duration; - pos->clock.rate = pos->clock.target_rate; -} - static void alsa_on_timeout_event(struct spa_source *source) { struct state *state = source->data; @@ -2483,8 +2479,6 @@ static void alsa_on_timeout_event(struct spa_source *source) } } - update_target(state); - if (SPA_UNLIKELY((res = check_position_config(state)) < 0)) { spa_log_warn(state->log, "%p: error invalid position: %s", state, spa_strerror(res)); @@ -2573,9 +2567,6 @@ int spa_alsa_start(struct state *state) state->following = is_following(state); - if (!state->following) - update_target(state); - if (check_position_config(state) < 0) { spa_log_error(state->log, "%s: invalid position config", state->props.device); return -EIO; diff --git a/spa/plugins/alsa/alsa-seq.c b/spa/plugins/alsa/alsa-seq.c index beb7e6c80..83265ae30 100644 --- a/spa/plugins/alsa/alsa-seq.c +++ b/spa/plugins/alsa/alsa-seq.c @@ -682,15 +682,6 @@ static int process_write(struct seq_state *state) return res; } -static void update_target(struct seq_state *state) -{ - if (SPA_LIKELY(state->position)) { - struct spa_io_clock *clock = &state->position->clock; - clock->duration = clock->target_duration; - clock->rate = clock->target_rate; - } -} - static void update_position(struct seq_state *state) { if (SPA_LIKELY(state->position)) { @@ -753,7 +744,8 @@ static int update_time(struct seq_state *state, uint64_t nsec, bool follower) if (!follower && state->clock) { state->clock->nsec = nsec; - state->clock->position += state->duration; + state->clock->rate = state->rate; + state->clock->position += state->clock->duration; state->clock->duration = state->duration; state->clock->delay = state->duration * corr; state->clock->rate_diff = corr; @@ -798,13 +790,21 @@ static void alsa_on_timeout_event(struct spa_source *source) } } - update_target(state); - state->current_time = state->next_time; spa_log_trace(state->log, "timeout %"PRIu64, state->current_time); - update_position(state); + if (SPA_LIKELY(state->position)) { + struct spa_io_clock *clock = &state->position->clock; + state->rate = clock->target_rate; + if (state->rate.num == 0 || state->rate.denom == 0) + state->rate = SPA_FRACTION(1, 48000); + state->duration = clock->target_duration; + } else { + state->rate = SPA_FRACTION(1, 48000); + state->duration = 1024; + } + state->threshold = state->duration; update_time(state, state->current_time, false); diff --git a/spa/plugins/avb/avb-pcm.c b/spa/plugins/avb/avb-pcm.c index 323fe4725..92be4da59 100644 --- a/spa/plugins/avb/avb-pcm.c +++ b/spa/plugins/avb/avb-pcm.c @@ -832,6 +832,17 @@ static void set_timeout(struct state *state, uint64_t next_time) state->timer_source.fd, SPA_FD_TIMER_ABSTIME, &ts, NULL); } +static void update_position(struct state *state) +{ + if (state->position) { + state->duration = state->position->clock.duration; + state->rate_denom = state->position->clock.rate.denom; + } else { + state->duration = 1024; + state->rate_denom = state->rate; + } +} + static int flush_write(struct state *state, uint64_t current_time) { int32_t avail, wanted; @@ -884,6 +895,8 @@ int spa_avb_write(struct state *state) uint32_t index, to_write; struct port *port = &state->ports[0]; + update_position(state); + filled = spa_ringbuffer_get_write_index(&state->ring, &index); if (filled < 0) { spa_log_warn(state->log, "underrun %d", filled); @@ -933,14 +946,16 @@ int spa_avb_write(struct state *state) } spa_ringbuffer_write_update(&state->ring, index); - if (state->following) { + if (state->following) flush_write(state, state->position->clock.nsec); - } + return 0; } static int handle_play(struct state *state, uint64_t current_time) { + update_position(state); + flush_write(state, current_time); spa_node_call_ready(&state->callbacks, SPA_STATUS_NEED_DATA); return 0; @@ -955,8 +970,7 @@ int spa_avb_read(struct state *state) struct spa_data *d; uint32_t n_bytes; - if (state->position) - state->duration = state->position->clock.duration; + update_position(state); avail = spa_ringbuffer_get_read_index(&state->ring, &index); wanted = state->duration * state->stride; @@ -1028,7 +1042,7 @@ static void avb_on_timeout_event(struct spa_source *source) { struct state *state = source->data; uint64_t expirations, current_time, duration; - uint32_t rate; + struct spa_fraction rate; int res; spa_log_trace(state->log, "timeout"); @@ -1042,27 +1056,24 @@ static void avb_on_timeout_event(struct spa_source *source) current_time = state->next_time; if (SPA_LIKELY(state->position)) { - state->position->clock.duration = state->position->clock.target_duration; - state->position->clock.rate = state->position->clock.target_rate; - - duration = state->position->clock.duration; - rate = state->position->clock.rate.denom; + duration = state->position->clock.target_duration; + rate = state->position->clock.target_rate; } else { duration = 1024; - rate = 48000; + rate = SPA_FRACTION(1, 48000); } - state->duration = duration; + + state->next_time = current_time + duration * SPA_NSEC_PER_SEC / rate.denom; if (state->ports[0].direction == SPA_DIRECTION_INPUT) handle_play(state, current_time); else handle_capture(state, current_time); - state->next_time = current_time + duration * SPA_NSEC_PER_SEC / rate; - if (SPA_LIKELY(state->clock)) { state->clock->nsec = current_time; - state->clock->position += duration; + state->clock->rate = rate; + state->clock->position += state->clock->duration; state->clock->duration = duration; state->clock->delay = 0; state->clock->rate_diff = 1.0; @@ -1137,13 +1148,7 @@ int spa_avb_start(struct state *state) if (state->started) return 0; - if (state->position) { - state->duration = state->position->clock.duration; - state->rate_denom = state->position->clock.rate.denom; - } else { - state->duration = 1024; - state->rate_denom = state->rate; - } + update_position(state); spa_dll_init(&state->dll); state->max_error = (256.0 * state->rate) / state->rate_denom; diff --git a/spa/plugins/bluez5/media-sink.c b/spa/plugins/bluez5/media-sink.c index 4651421d8..ec6cb9c8b 100644 --- a/spa/plugins/bluez5/media-sink.c +++ b/spa/plugins/bluez5/media-sink.c @@ -989,11 +989,8 @@ static void media_on_timeout(struct spa_source *source) now_time, now_time - prev_time); if (SPA_LIKELY(this->position)) { - this->position->clock.duration = this->position->clock.target_duration; - this->position->clock.rate = this->position->clock.target_rate; - - duration = this->position->clock.duration; - rate = this->position->clock.rate.denom; + duration = this->position->clock.target_duration; + rate = this->position->clock.target_rate.denom; } else { duration = 1024; rate = 48000; @@ -1005,7 +1002,8 @@ static void media_on_timeout(struct spa_source *source) int64_t delay_nsec = 0; this->clock->nsec = now_time; - this->clock->position += duration; + this->clock->rate = this->clock->target_rate; + this->clock->position += this->clock->duration; this->clock->duration = duration; this->clock->rate_diff = 1.0f; this->clock->next_nsec = this->next_time; diff --git a/spa/plugins/bluez5/media-source.c b/spa/plugins/bluez5/media-source.c index 7c9c153e2..6765183c1 100644 --- a/spa/plugins/bluez5/media-source.c +++ b/spa/plugins/bluez5/media-source.c @@ -612,28 +612,26 @@ static void media_on_timeout(struct spa_source *source) now_time, now_time - prev_time); if (SPA_LIKELY(this->position)) { - this->position->clock.duration = this->position->clock.target_duration; - this->position->clock.rate = this->position->clock.target_rate; - - duration = this->position->clock.duration; - rate = this->position->clock.rate.denom; + duration = this->position->clock.target_duration; + rate = this->position->clock.target_rate.denom; } else { duration = 1024; rate = 48000; } - setup_matching(this); - this->next_time = now_time + duration * SPA_NSEC_PER_SEC / port->buffer.corr / rate; if (SPA_LIKELY(this->clock)) { this->clock->nsec = now_time; - this->clock->position += duration; + this->clock->rate = this->clock->target_rate; + this->clock->position += this->clock->duration; this->clock->duration = duration; this->clock->rate_diff = port->buffer.corr; this->clock->next_nsec = this->next_time; } + setup_matching(this); + if (port->io) { int io_status = port->io->status; int status = produce_buffer(this); @@ -1326,30 +1324,27 @@ static int impl_node_port_reuse_buffer(void *object, uint32_t port_id, uint32_t return 0; } -static uint32_t get_samples(struct impl *this, uint32_t *duration) +static uint32_t get_samples(struct impl *this, uint32_t *result_duration) { struct port *port = &this->port; - uint32_t samples; + uint32_t samples, rate_denom; + uint64_t duration; + + if (SPA_LIKELY(this->position)) { + duration = this->position->clock.duration; + rate_denom = this->position->clock.rate.denom; + } else { + duration = 1024; + rate_denom = port->current_format.info.raw.rate; + } + + *result_duration = duration * port->current_format.info.raw.rate / rate_denom; if (SPA_LIKELY(port->rate_match) && this->resampling) { samples = port->rate_match->size; } else { - if (SPA_LIKELY(this->position)) - samples = this->position->clock.duration * port->current_format.info.raw.rate - / this->position->clock.rate.denom; - else - samples = 1024; + samples = *result_duration; } - - if (SPA_LIKELY(this->position)) - *duration = this->position->clock.duration * port->current_format.info.raw.rate - / this->position->clock.rate.denom; - else if (SPA_LIKELY(this->clock)) - *duration = this->clock->duration * port->current_format.info.raw.rate - / this->clock->rate.denom; - else - *duration = 1024 * port->current_format.info.raw.rate / 48000; - return samples; } diff --git a/spa/plugins/bluez5/midi-node.c b/spa/plugins/bluez5/midi-node.c index b465b8a17..da080977c 100644 --- a/spa/plugins/bluez5/midi-node.c +++ b/spa/plugins/bluez5/midi-node.c @@ -838,14 +838,6 @@ static int process_input(struct impl *this) return SPA_STATUS_HAVE_DATA; } -static void update_target(struct impl *this) -{ - if (SPA_LIKELY(this->position)) { - this->position->clock.duration = this->position->clock.target_duration; - this->position->clock.rate = this->position->clock.target_rate; - } -} - static void update_position(struct impl *this) { if (SPA_LIKELY(this->position)) { @@ -876,15 +868,20 @@ static void on_timeout(struct spa_source *source) spa_log_trace(this->log, "%p: timer %"PRIu64" %"PRIu64"", this, now_time, now_time - prev_time); - update_target(this); - - update_position(this); + if (SPA_LIKELY(this->position)) { + this->duration = this->position->clock.target_duration; + this->rate = this->position->clock.target_rate.denom; + } else { + this->duration = 1024; + this->rate = 48000; + } this->next_time = now_time + this->duration * SPA_NSEC_PER_SEC / this->rate; if (SPA_LIKELY(this->clock)) { this->clock->nsec = now_time; - this->clock->position += this->duration; + this->clock->rate = this->clock->target_rate; + this->clock->position += this->clock->duration; this->clock->duration = this->duration; this->clock->rate_diff = 1.0f; this->clock->next_nsec = this->next_time; @@ -1171,9 +1168,6 @@ static int do_start(struct impl *this) this->following = is_following(this); - if (!this->following) - update_target(this); - update_position(this); spa_log_debug(this->log, "%p: start following:%d", diff --git a/spa/plugins/bluez5/sco-sink.c b/spa/plugins/bluez5/sco-sink.c index 16dde673c..8856a179b 100644 --- a/spa/plugins/bluez5/sco-sink.c +++ b/spa/plugins/bluez5/sco-sink.c @@ -603,11 +603,8 @@ static void sco_on_timeout(struct spa_source *source) now_time, now_time - prev_time); if (SPA_LIKELY(this->position)) { - this->position->clock.duration = this->position->clock.target_duration; - this->position->clock.rate = this->position->clock.target_rate; - - duration = this->position->clock.duration; - rate = this->position->clock.rate.denom; + duration = this->position->clock.target_duration; + rate = this->position->clock.target_rate.denom; } else { duration = 1024; rate = 48000; @@ -617,7 +614,8 @@ static void sco_on_timeout(struct spa_source *source) if (SPA_LIKELY(this->clock)) { this->clock->nsec = now_time; - this->clock->position += duration; + this->clock->rate = this->clock->target_rate; + this->clock->position += this->clock->duration; this->clock->duration = duration; this->clock->rate_diff = 1.0f; this->clock->next_nsec = this->next_time; diff --git a/spa/plugins/bluez5/sco-source.c b/spa/plugins/bluez5/sco-source.c index 49651bb2d..067056215 100644 --- a/spa/plugins/bluez5/sco-source.c +++ b/spa/plugins/bluez5/sco-source.c @@ -611,28 +611,26 @@ static void sco_on_timeout(struct spa_source *source) now_time, now_time - prev_time); if (SPA_LIKELY(this->position)) { - this->position->clock.duration = this->position->clock.target_duration; - this->position->clock.rate = this->position->clock.target_rate; - - duration = this->position->clock.duration; - rate = this->position->clock.rate.denom; + duration = this->position->clock.target_duration; + rate = this->position->clock.target_rate.denom; } else { duration = 1024; rate = 48000; } - setup_matching(this); - this->next_time = now_time + duration * SPA_NSEC_PER_SEC / port->buffer.corr / rate; if (SPA_LIKELY(this->clock)) { this->clock->nsec = now_time; - this->clock->position += duration; + this->clock->rate = this->clock->target_rate; + this->clock->position += this->clock->duration; this->clock->duration = duration; this->clock->rate_diff = port->buffer.corr; this->clock->next_nsec = this->next_time; } + setup_matching(this); + if (port->io) { int io_status = port->io->status; int status = produce_buffer(this); @@ -1281,29 +1279,26 @@ static int impl_node_port_reuse_buffer(void *object, uint32_t port_id, uint32_t return 0; } -static uint32_t get_samples(struct impl *this, uint32_t *duration) +static uint32_t get_samples(struct impl *this, uint32_t *result_duration) { struct port *port = &this->port; - uint32_t samples; + uint32_t samples, rate_denom; + uint64_t duration; - if (SPA_LIKELY(port->rate_match) && this->resampling) { - samples = port->rate_match->size; + if (SPA_LIKELY(this->position)) { + duration = this->position->clock.duration; + rate_denom = this->position->clock.rate.denom; } else { - if (SPA_LIKELY(this->position)) - samples = this->position->clock.duration * port->current_format.info.raw.rate - / this->position->clock.rate.denom; - else - samples = 1024; + duration = 1024; + rate_denom = port->current_format.info.raw.rate; } - if (SPA_LIKELY(this->position)) - *duration = this->position->clock.duration * port->current_format.info.raw.rate - / this->position->clock.rate.denom; - else if (SPA_LIKELY(this->clock)) - *duration = this->clock->duration * port->current_format.info.raw.rate - / this->clock->rate.denom; + *result_duration = duration * port->current_format.info.raw.rate / rate_denom; + + if (SPA_LIKELY(port->rate_match) && this->resampling) + samples = port->rate_match->size; else - *duration = 1024 * port->current_format.info.raw.rate / 48000; + samples = *result_duration; return samples; } diff --git a/spa/plugins/support/node-driver.c b/spa/plugins/support/node-driver.c index b406d35d5..466378e11 100644 --- a/spa/plugins/support/node-driver.c +++ b/spa/plugins/support/node-driver.c @@ -247,11 +247,8 @@ static void on_timeout(struct spa_source *source) return; } if (SPA_LIKELY(this->position)) { - this->position->clock.duration = this->position->clock.target_duration; - this->position->clock.rate = this->position->clock.target_rate; - - duration = this->position->clock.duration; - rate = this->position->clock.rate.denom; + duration = this->position->clock.target_duration; + rate = this->position->clock.target_rate.denom; } else { duration = 1024; rate = 48000; @@ -286,7 +283,6 @@ static void on_timeout(struct spa_source *source) else if (err < -this->max_error) err = -this->max_error; - position += duration; this->last_time = current_time; if (this->tracking) { @@ -294,7 +290,7 @@ static void on_timeout(struct spa_source *source) this->next_time = nsec + duration / corr * 1e9 / rate; } else { corr = 1.0; - this->next_time = scale_u64(position, SPA_NSEC_PER_SEC, rate); + this->next_time = scale_u64(position + duration, SPA_NSEC_PER_SEC, rate); } if (SPA_UNLIKELY((this->next_time - this->base_time) > BW_PERIOD)) { @@ -307,7 +303,8 @@ static void on_timeout(struct spa_source *source) if (SPA_LIKELY(this->clock)) { this->clock->nsec = nsec; - this->clock->position = position; + this->clock->rate = this->clock->target_rate; + this->clock->position += this->clock->duration; this->clock->duration = duration; this->clock->delay = 0; this->clock->rate_diff = corr; diff --git a/spa/plugins/support/null-audio-sink.c b/spa/plugins/support/null-audio-sink.c index 08372c880..ee4ffe362 100644 --- a/spa/plugins/support/null-audio-sink.c +++ b/spa/plugins/support/null-audio-sink.c @@ -280,11 +280,8 @@ static void on_timeout(struct spa_source *source) nsec = this->next_time; if (SPA_LIKELY(this->position)) { - this->position->clock.duration = this->position->clock.target_duration; - this->position->clock.rate = this->position->clock.target_rate; - - duration = this->position->clock.duration; - rate = this->position->clock.rate.denom; + duration = this->position->clock.target_duration; + rate = this->position->clock.target_rate.denom; } else { duration = 1024; rate = 48000; @@ -294,7 +291,8 @@ static void on_timeout(struct spa_source *source) if (SPA_LIKELY(this->clock)) { this->clock->nsec = nsec; - this->clock->position += duration; + this->clock->rate = this->clock->target_rate; + this->clock->position += this->clock->duration; this->clock->duration = duration; this->clock->delay = 0; this->clock->rate_diff = 1.0; diff --git a/src/pipewire/filter.c b/src/pipewire/filter.c index 10e14c55b..7bd8d698a 100644 --- a/src/pipewire/filter.c +++ b/src/pipewire/filter.c @@ -955,8 +955,9 @@ static inline void update_target(struct filter *impl) { struct spa_io_position *p = impl->rt.position; if (SPA_LIKELY(p != NULL)) { - p->clock.duration = p->clock.target_duration; p->clock.rate = p->clock.target_rate; + p->clock.position += p->clock.duration; + p->clock.duration = p->clock.target_duration; } } diff --git a/src/pipewire/stream.c b/src/pipewire/stream.c index ee0429d68..76de36597 100644 --- a/src/pipewire/stream.c +++ b/src/pipewire/stream.c @@ -576,8 +576,9 @@ static inline void update_target(struct stream *impl) { struct spa_io_position *p = impl->rt.position; if (SPA_LIKELY(p != NULL)) { - p->clock.duration = p->clock.target_duration; p->clock.rate = p->clock.target_rate; + p->clock.position += p->clock.duration; + p->clock.duration = p->clock.target_duration; } }