On Wed, 2017-03-29 at 10:36 +0200, Georg Chini wrote: > With the current code, the user can request any end-to-end latency. Because there > is no protection against underruns, setting the latency too small will result in > repetitive underruns. > > This patch tries to mitigate the problem by calculating the minimum possible latency > for the current combination of source and sink. The actual calculation has been put > in a separate function so it can easily be changed. To keep the values up to date, > changes in the latency ranges have to be tracked. > > The calculated minimum latency is used to limit the configured latency. > The minimum latency is only a "best guess", so the actual minimum may be much > larger (for example for USB devices) or much smaller than the calculated value. > > Port latency offsets are not yet handled properly, this will be done in a separate > patch. In what way are they not handled properly? Does it make sense to have any code related to latency offsets in this patch if it needs to be fixed later? > --- > src/modules/module-loopback.c | 199 ++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 191 insertions(+), 8 deletions(-) > > diff --git a/src/modules/module-loopback.c b/src/modules/module-loopback.c > index 9b4ea49..e2cd986 100644 > --- a/src/modules/module-loopback.c > +++ b/src/modules/module-loopback.c > @@ -65,10 +65,14 @@ PA_MODULE_USAGE( > > #define DEFAULT_ADJUST_TIME_USEC (10*PA_USEC_PER_SEC) > > +typedef struct loopback_msg loopback_msg; https://www.freedesktop.org/wiki/Software/PulseAudio/Documentation/Developer/CodingStyle/ "Exported structs should be typedef'ed so they can be used without typing "struct" each time. Structs that are used only inside a single source file should not be typedef'ed that way." > +enum { > + LOOPBACK_MESSAGE_SOURCE_LATENCY_CHANGED, > + LOOPBACK_MESSAGE_SINK_LATENCY_CHANGED I'd name these using "LATENCY_RANGE" instead of just "LATENCY", for clarity. > @@ -751,6 +860,12 @@ static int sink_input_process_msg_cb(pa_msgobject *obj, int code, void *data, in > u->output_thread_info.effective_source_latency = (pa_usec_t)offset; > > return 0; > + > + case SINK_INPUT_MESSAGE_UPDATE_MIN_LATENCY: > + > + u->output_thread_info.minimum_latency = (pa_usec_t)offset; If the minimum latency grows, and the target latency grows too because of that, I think we should push some silence to the memblockq to account for the change in the target latency. Otherwise I think there's a risk of getting many underruns before the latency controller adjusts to the new target. > +/* Called from main context */ > +static int loopback_process_msg_cb(pa_msgobject *o, int code, void *userdata, int64_t offset, pa_memchunk *chunk) { > + struct loopback_msg *msg; > + struct userdata *u; > + pa_usec_t current_latency; > + > + pa_assert(o); > + pa_assert_ctl_context(); > + > + msg = LOOPBACK_MSG(o); > + pa_assert_se(u = msg->userdata); > + > + switch (code) { > + > + case LOOPBACK_MESSAGE_SOURCE_LATENCY_CHANGED: > + > + update_effective_source_latency(u, u->source_output->source, u->sink_input->sink); > + current_latency = pa_source_get_requested_latency(u->source_output->source); > + if (current_latency > u->configured_source_latency) { > + /* The minimum latency has changed to a value larger than the configured latency. so > + * the source latency has been increased. The case that the minimum latency changes > + * back to a smaller value is not handled because this is currently not implemented */ At the end of the comment, I think "because that never happens with the current source implementations" would be clearer than "because this is currently not implemented". > + pa_log_warn("Source minimum latency increased to %0.2f ms", (double)current_latency / PA_USEC_PER_MSEC); > + u->configured_source_latency = current_latency; > + update_latency_boundaries(u, u->source_output->source, u->sink_input->sink, false); > + } > + > + return 0; > + > + case LOOPBACK_MESSAGE_SINK_LATENCY_CHANGED: > + > + current_latency = pa_sink_get_requested_latency(u->sink_input->sink); > + if (current_latency > u->configured_sink_latency) { > + /* The minimum latency has changed to a value larger than the configured latency, so > + * the sink latency has been increased. The case that the minimum latency changes back > + * to a smaller value is not handled because this is currently not implemented */ Same comment as above. -- Tanu https://www.patreon.com/tanuk
