On Tue, 2012-12-04 at 14:54 +0100, Peter Meerwald wrote:
> From: Stefan Huber <s.huber at bct-electronic.com>
>
> In case that source and sink use different sample specs (e.g., different
> number of channels) the computation of the latency difference fails.
> To fix this, we obtain the corresponding latencies in terms of time using
> the respective sample specs instead of buffer sizes.
>
> Signed-off-by: Stefan Huber <s.huber at bct-electronic.com>
> Acked-by: Peter Meerwald <p.meerwald at bct-electronic.com>
> ---
> src/modules/echo-cancel/module-echo-cancel.c | 26 +++++++++++++++-----------
> 1 file changed, 15 insertions(+), 11 deletions(-)
Disclaimer: I don't understand the code that is being changed, so all I
can do is to check that the conversion from bytes to usec looks
superficially correct.
> diff --git a/src/modules/echo-cancel/module-echo-cancel.c b/src/modules/echo-cancel/module-echo-cancel.c
> index 26ac30b..8d89d2d 100644
> --- a/src/modules/echo-cancel/module-echo-cancel.c
> +++ b/src/modules/echo-cancel/module-echo-cancel.c
> @@ -294,24 +294,28 @@ enum {
> };
>
> static int64_t calc_diff(struct userdata *u, struct snapshot *snapshot) {
> - int64_t buffer, diff_time, buffer_latency;
> + int64_t diff_time, buffer_latency;
> + int64_t plen, rlen, source_delay, sink_delay, recv_counter, send_counter;
I would prefer pa_usec_t for the variables that contain (non-negative)
time values.
>
> /* get the number of samples between capture and playback */
Isn't this comment now out of date? Instead of number of samples, it
should talk about latency or something like that.
> - if (snapshot->plen > snapshot->rlen)
> - buffer = snapshot->plen - snapshot->rlen;
> + plen = pa_bytes_to_usec(snapshot->plen, &u->sink_input->sample_spec);
> + rlen = pa_bytes_to_usec(snapshot->plen, &u->source_output->sample_spec);
In that last line, I guess the parameter to pa_bytes_to_usec() be
snapshot->rlen instead of snapshot->plen?
> + if (plen > rlen)
> + buffer_latency = plen-rlen;
> else
> - buffer = 0;
> + buffer_latency = 0;
>
> - buffer += snapshot->source_delay + snapshot->sink_delay;
> + source_delay = pa_bytes_to_usec(snapshot->source_delay, &u->source_output->sample_spec);
> + sink_delay = pa_bytes_to_usec(snapshot->sink_delay, &u->sink_input->sample_spec);
> + buffer_latency += source_delay + sink_delay;
>
> /* add the amount of samples not yet transferred to the source context */
Again, the comment talks about samples while the code works with time
units.
--
Tanu
