From bb022c1b84d8c3f272ba01d89dd239e2e1465d0e Mon Sep 17 00:00:00 2001 From: Carlos Rafael Giani Date: Thu, 17 Jul 2025 12:52:09 +0200 Subject: [PATCH] node-driver: Handle realtime clock modifications If the user alters the realtime clock (for example by using the "date" command in the shell), and the node driver uses the realtime clock as the timerfd clock, then the scheduled graph cycle invocation may not take place, or may take place much later than planned, because the timestamp that was passed to spa_system_timerfd_settime() is now invalid. Configure the timer to automatically be canceled if the realtime clock is modified so that the graph cycle can be rescheduled with an updated timestamp that is actually usable with the altered realtime clock. --- spa/plugins/support/node-driver.c | 80 ++++++++++++++++++++++++++++--- 1 file changed, 74 insertions(+), 6 deletions(-) diff --git a/spa/plugins/support/node-driver.c b/spa/plugins/support/node-driver.c index 701a5b7bf..1ca186a49 100644 --- a/spa/plugins/support/node-driver.c +++ b/spa/plugins/support/node-driver.c @@ -154,11 +154,25 @@ static const char *clock_id_to_name(clockid_t id) static void set_timeout(struct impl *this, uint64_t next_time) { + /* The realtime system clock may be modified by the user. In such a case, + * a scheduled timer must be canceled, since its timeout is no longer + * correctly corresponding to the duration of a graph cycle. Worse, if + * for example the user resets the realtime clock way back to the past, + * then the timeout may now be far in the future, meaning that the next + * graph cycle never takes place. The SPA_FD_TIMER_CANCEL_ON_SET flag is + * used here to automatically cancel the timer if the user sets the realtime + * clock so that the driver can reschedule the cycle. (Timer cancelation + * will trigger an on_timeout() invocation with spa_system_timerfd_read() + * returning -ECANCELED.) + * If timerfd is used with a non-realtime clock, the flag is ignored. + * (Note that the flag only works in combination with SPA_FD_TIMER_ABSTIME.) */ + spa_log_trace(this->log, "set timeout %"PRIu64, next_time); this->timerspec.it_value.tv_sec = next_time / SPA_NSEC_PER_SEC; this->timerspec.it_value.tv_nsec = next_time % SPA_NSEC_PER_SEC; spa_system_timerfd_settime(this->data_system, - this->timer_source.fd, SPA_FD_TIMER_ABSTIME, &this->timerspec, NULL); + this->timer_source.fd, SPA_FD_TIMER_ABSTIME | + SPA_FD_TIMER_CANCEL_ON_SET, &this->timerspec, NULL); } static inline uint64_t gettime_nsec(struct impl *this, clockid_t clock_id) @@ -329,14 +343,25 @@ static void on_timeout(struct spa_source *source) uint32_t rate; double corr = 1.0, err = 0.0; int res; + bool timer_was_canceled = false; + + /* See set_timeout() for an explanation about timer cancelation. */ if ((res = spa_system_timerfd_read(this->data_system, this->timer_source.fd, &expirations)) < 0) { - if (res != -EAGAIN) + if (res == -EAGAIN) { + return; + } else if (res == -ECANCELED) { + spa_log_debug(this->log, "%p: timer was canceled; " + "rescheduling graph cycle", this); + timer_was_canceled = true; + } else { spa_log_error(this->log, "%p: timerfd error: %s", this, spa_strerror(res)); - return; + return; + } } + if (SPA_LIKELY(this->position)) { duration = this->position->clock.target_duration; rate = this->position->clock.target_rate.denom; @@ -344,24 +369,62 @@ static void on_timeout(struct spa_source *source) duration = 1024; rate = 48000; } - if (this->props.freewheel) + + /* In freewheel mode, graph cycles are run as fast as possible, + * especially if the "freewheel.wait" period is 0. In such a case, + * as soon as the mainloop encounters the scheduled timer timeout, + * it will execute it immediately. Since it is not possible to + * measure how long it takes the mainloop to do that, it is not + * possible to rely on this->next_time as the nsec value in + * freewheel mode (this->next_time does not factor in the mainloop + * invocation time mentioned earlier). Instead, sample the current + * monotonic time when freewheel mode is active, to account for + * that invocation time. + * + * Also, if the timer was canceled, the graph cycle needs to be + * rescheduled, and it cannot be assumed that the this->next_time + * and this->clock->position values are correct anymore. (Timer + * cancellations happen when the realtime clock is being used by + * this driver and the user modified the realtime clock for example.) + */ + if (this->props.freewheel || SPA_UNLIKELY(timer_was_canceled)) nsec = gettime_nsec(this, this->timer_clockid); else nsec = this->next_time; + /* "tracking" means that the driver is following a clock that is not + * usable by timerfd. It is an entirely separate clock, for example, + * a network interface PHC. If tracking is true, timer_clockid is + * always the monotonic clock, and this->props.clock_id is that entirely + * separate clock. If tracking is false, then this->props.clock_id + * equals timer_clockid, so "nsec" can directly be used as the current + * driver clock time in that case. */ if (this->tracking) - /* we are actually following another clock */ current_time = gettime_nsec(this, this->props.clock_id); else current_time = nsec; current_position = scale_u64(current_time, rate, SPA_NSEC_PER_SEC); - if (this->last_time == 0) { + if ((this->last_time == 0) || SPA_UNLIKELY(timer_was_canceled)) { spa_dll_set_bw(&this->dll, SPA_DLL_BW_MIN, duration, rate); this->max_error = rate * MAX_ERROR_MS / 1000; this->max_resync = rate * this->props.resync_ms / 1000; position = current_position; + + /* If the timer was canceled, then it is assumed that a + * discontinuity occurred. Accumulated nsec_offset values + * cannot be relied upon anymore, and need to be reset. + * Also, base_time is set back to 0 to make sure the log line + * further below (which prints current stats) continues to + * be printed. (If for example the clock was set to an + * earlier time, then the base_time might contain a future + * timestamp that the clock won't reach for a long while.) */ + if (timer_was_canceled) { + this->base_time = 0; + this->nsec_offset.offset = get_nsec_offset(this, NULL); + this->nsec_offset.err = 0; + } } else if (SPA_LIKELY(this->clock)) { position = this->clock->position + this->clock->duration; } else { @@ -415,6 +478,11 @@ static void on_timeout(struct spa_source *source) this->clock->delay = 0; this->clock->rate_diff = corr; this->clock->next_nsec = this->next_time + nsec_offset; + + if (SPA_UNLIKELY(timer_was_canceled)) { + SPA_FLAG_UPDATE(this->clock->flags, + SPA_IO_CLOCK_FLAG_DISCONT, true); + } } spa_node_call_ready(&this->callbacks,