On 09.03.2017 16:35, Arun Raghavan wrote:
>
> On Thu, 9 Mar 2017, at 04:01 PM, Georg Chini wrote:
>> On 09.03.2017 06:53, Arun Raghavan wrote:
>>> On systems with constrained CPUs, we might run into a situation where
>>> the master source/sink is configured to have too high a latency.
>>>
>>> On the source side, this would cause us to wake up with a large chunk of
>>> data to process, which might cause us to exhust our RT limit and thus be
>>> killed.
>>>
>>> So it makes sense to limit the overall latency that we request from the
>>> source (and correspondingly, the sink, so we don't starve for playback
>>> data on the source side).
>>>
>>> The 10 blocks maximum is somewhat arbitrary (I'm assuming the system has
>>> enough headroom to process 10 chunks through the canceller without
>>> getting close to the RT limit). This might make sense to make tunable in
>>> the future.
>>> ---
>>> src/modules/echo-cancel/module-echo-cancel.c | 32 +++++++++++++++++++++-------
>>> 1 file changed, 24 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/src/modules/echo-cancel/module-echo-cancel.c b/src/modules/echo-cancel/module-echo-cancel.c
>>> index ed75e0c..0a9f290 100644
>>> --- a/src/modules/echo-cancel/module-echo-cancel.c
>>> +++ b/src/modules/echo-cancel/module-echo-cancel.c
>>> @@ -145,6 +145,8 @@ static const pa_echo_canceller ec_table[] = {
>>>
>>> #define MEMBLOCKQ_MAXLENGTH (16*1024*1024)
>>>
>>> +#define MAX_LATENCY_BLOCKS 10
>>> +
>>> /* Can only be used in main context */
>>> #define IS_ACTIVE(u) ((pa_source_get_state((u)->source) == PA_SOURCE_RUNNING) && \
>>> (pa_sink_get_state((u)->sink) == PA_SINK_RUNNING))
>>> @@ -515,6 +517,7 @@ static int sink_set_state_cb(pa_sink *s, pa_sink_state_t state) {
>>> /* Called from source I/O thread context */
>>> static void source_update_requested_latency_cb(pa_source *s) {
>>> struct userdata *u;
>>> + pa_usec_t latency;
>>>
>>> pa_source_assert_ref(s);
>>> pa_assert_se(u = s->userdata);
>>> @@ -525,15 +528,17 @@ static void source_update_requested_latency_cb(pa_source *s) {
>>>
>>> pa_log_debug("Source update requested latency");
>>>
>>> - /* Just hand this one over to the master source */
>>> - pa_source_output_set_requested_latency_within_thread(
>>> - u->source_output,
>>> - pa_source_get_requested_latency_within_thread(s));
>>> + /* Cap the maximum latency so we don't have to process too large chunks */
>>> + latency = PA_MIN(pa_source_get_requested_latency_within_thread(s),
>>> + pa_bytes_to_usec(u->source_blocksize, &s->sample_spec) * MAX_LATENCY_BLOCKS);
>>> +
>>> + pa_source_output_set_requested_latency_within_thread(u->source_output, latency);
>>> }
>>>
>>> /* Called from sink I/O thread context */
>>> static void sink_update_requested_latency_cb(pa_sink *s) {
>>> struct userdata *u;
>>> + pa_usec_t latency;
>>>
>>> pa_sink_assert_ref(s);
>>> pa_assert_se(u = s->userdata);
>>> @@ -544,10 +549,11 @@ static void sink_update_requested_latency_cb(pa_sink *s) {
>>>
>>> pa_log_debug("Sink update requested latency");
>>>
>>> - /* Just hand this one over to the master sink */
>>> - pa_sink_input_set_requested_latency_within_thread(
>>> - u->sink_input,
>>> - pa_sink_get_requested_latency_within_thread(s));
>>> + /* Cap the maximum latency so we don't have to process too large chunks */
>>> + latency = PA_MIN(pa_sink_get_requested_latency_within_thread(s),
>>> + pa_bytes_to_usec(u->sink_blocksize, &s->sample_spec) * MAX_LATENCY_BLOCKS);
>>> +
>>> + pa_sink_input_set_requested_latency_within_thread(u->sink_input, latency);
>>> }
>>>
>>> /* Called from sink I/O thread context */
>>> @@ -1658,6 +1664,7 @@ int pa__init(pa_module*m) {
>>> uint32_t temp;
>>> uint32_t nframes = 0;
>>> bool use_master_format;
>>> + pa_usec_t blocksize_usec;
>>>
>>> pa_assert(m);
>>>
>>> @@ -2020,6 +2027,15 @@ int pa__init(pa_module*m) {
>>>
>>> u->thread_info.current_volume = u->source->reference_volume;
>>>
>>> + /* We don't want to deal with too many chunks at a time */
>>> + blocksize_usec = pa_bytes_to_usec(u->source_blocksize, &u->source->sample_spec);
>>> + pa_source_set_latency_range(u->source, blocksize_usec, blocksize_usec * MAX_LATENCY_BLOCKS);
>>> + pa_source_output_set_requested_latency(u->source_output, blocksize_usec * MAX_LATENCY_BLOCKS);
>>> +
>>> + blocksize_usec = pa_bytes_to_usec(u->sink_blocksize, &u->sink->sample_spec);
>>> + pa_sink_set_latency_range(u->sink, blocksize_usec, blocksize_usec * MAX_LATENCY_BLOCKS);
>>> + pa_sink_input_set_requested_latency(u->sink_input, blocksize_usec * MAX_LATENCY_BLOCKS);
>>> +
>>> pa_sink_put(u->sink);
>>> pa_source_put(u->source);
>>>
>> You should save the initial latency ranges and restore them when the
>> module is unloaded.
> The latency range is set on the echo-cancel sink and source (not the
> master), so when the module is unloaded, that sink and source go away as
> well.
>
> -- Arun
Sorry, looks good then.
