On 19.11.2015 23:02, Tanu Kaskinen wrote:
On Wed, 2015-11-18 at 18:27 +0100, Georg Chini wrote:
On 18.11.2015 15:10, Tanu Kaskinen wrote:
On Wed, 2015-02-25 at 19:43 +0100, Georg Chini wrote:
Move the timer restart to the beginning of the callback function.
Please try to remember to always answer the question "why" in commit
messages.
I will in the future ...

---
   src/modules/module-loopback.c | 14 ++++++++------
   1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/modules/module-loopback.c b/src/modules/module-loopback.c
index f9255ee..79bd106 100644
--- a/src/modules/module-loopback.c
+++ b/src/modules/module-loopback.c
@@ -196,9 +196,6 @@ static void adjust_rates(struct userdata *u) {
       pa_assert(u);
       pa_assert_ctl_context();
- pa_asyncmsgq_send(u->sink_input->sink->asyncmsgq, PA_MSGOBJECT(u->sink_input), SINK_INPUT_MESSAGE_LATENCY_SNAPSHOT, NULL, 0, NULL);
-    pa_asyncmsgq_send(u->source_output->source->asyncmsgq, 
PA_MSGOBJECT(u->source_output), SOURCE_OUTPUT_MESSAGE_LATENCY_SNAPSHOT, NULL, 0, NULL);
-
       /* Rates and latencies*/
       old_rate = u->sink_input->sample_spec.rate;
       base_rate = u->source_output->sample_spec.rate;
@@ -236,8 +233,6 @@ static void adjust_rates(struct userdata *u) {
       /* Set rate */
       pa_sink_input_set_rate(u->sink_input, new_rate);
       pa_log_debug("[%s] Updated sampling rate to %lu Hz.", 
u->sink_input->sink->name, (unsigned long) new_rate);
-
-    pa_core_rttime_restart(u->core, u->time_event, pa_rtclock_now() + 
u->adjust_time);
   }
/* Called from main context */
@@ -248,6 +243,13 @@ static void time_callback(pa_mainloop_api *a, 
pa_time_event *e, const struct tim
       pa_assert(a);
       pa_assert(u->time_event == e);
+ /* Restart timer right away */
+    pa_core_rttime_restart(u->core, u->time_event, pa_rtclock_now() + 
u->adjust_time);
If the goal of this change is to make the wakeups happen exactly with
adjust_time intervals instead of adjust_time + time taken in servicing
the timer interrupt, wouldn't it be better to have a counter of the
timer events and use start_time + counter * adjust_time as the time
value here? That way the average interval should become exactly
adjust_time.
Yes, the goal is to minimize the influence of the interrupt service time.
It is more important that two consecutive interrupts are spaced as
exactly as possible by adjust_time, than that the average time interval
is correct.
Why is that so? I don't know what problem the patch is trying to solve,
so I can't judge myself which quality is more important. (Note also
that if the scheduling delay is constant, my method is more accurate in
both senses. If the delay is jittery, like it probably is, your method
may have lower average error.)

Obviously the patch improves the accuracy from the status quo, so I'm
not against applying it, and I also expect that choosing between the
two methods doesn't make meaningful difference, so maybe discussing
this further is pointless...


I tested it, the two methods are more or less equivalent. I also found that
the precision of the interval is about 0.1 percent in both cases.
_______________________________________________
pulseaudio-discuss mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Reply via email to