Re: [PATCH] ALSA: usb: refine delay information with USB frame counter

[Clemens Ladisch <clemens@ladisch.de>]

Pierre-Louis Bossart wrote:
> Existing code only updates the audio delay when URBs were
> submitted/retired. This can introduce an uncertainty of 8ms
> on the number of samples played out with the default settings,
> and a lot more when URBs convey more packets to reduce the
> interrupt rate and power consumption.
> 
> This patch relies on the USB frame counter to reduce the
> uncertainty to less than 2ms worst-case. The delay information
> essentially becomes independent of the URB size and number of
> packets. This should improve audio/video sync and help
> applications like PulseAudio which require accurate audio
> timing.

I haven't yet found the time to test this, but there are a few
theoretical nitpicks ...

> +	/* HCD implementations use different widths, use lower 8 bits.
> +	 * The delay will be managed up to 256ms, which is more than
> +	 * enough
> +	 */

/*
 * Multi-line comments are supposed to use this formatting style.
 */

> +	current_frame_number &= 0xFF;
> +	frame_diff = current_frame_number-subs->last_frame_number;
> +	if (frame_diff < 0)
> +		frame_diff += 256; /* handle 8-bit wrap-around */

This could be replaced with just:
	frame_diff = (current_frame_number - subs->last_frame_number) & 0xff;

> +	/* Report when delay estimate is off by more than 2ms.
> +	 * The error should be lower than 2ms since the estimate relies
> +	 * on two reads of a counter updated every ms */
> +	if (abs(est_delay-subs->last_delay) > (runtime->rate*2/1000L))
> +		snd_printk(KERN_DEBUG "ALSA usb.c: Delay %d actual delay %d\n",
> +			est_delay, subs->last_delay);

Do you know how large the difference actually becomes?

I'm not sure if it might be possible to construct a scenario where
rounding errors could accumulate ...


Regards,
Clemens