Lower minimal USB audio buffer size limit from 2ms to 1ms

[meka_tilda.center]

Summary

The limit was introduced in 4d76235fe3484e46f4746dbb1cc55712401b7beb on my request, but back then I didn't know enough about USB to make the best decision. Today I know better and based on https://www.usbmadesimple.co.uk/ums_6.htm#frames_and_microframes I would like to propose lowering minimal buffer size to match the USB frame, which is 1 millisecond, which is common for all USB standards.

[mav]

After your initial commit Hans reduced the default value from 8ms to the minimum of 2ms. I wonder if it is still OK or you'd like to reduce default also.

As a disclaimer, I know very little to nothing about USB.

[meka_tilda.center]

In D41627#949189, @mav wrote:

After your initial commit Hans reduced the default value from 8ms to the minimum of 2ms. I wonder if it is still OK or you'd like to reduce default also.

As a disclaimer, I know very little to nothing about USB.

If you look at the commit linked in description you'll see that that's the commit that made it possible to go to 2ms. Hans asked me what works for me and back then 2ms really worked well. That being said, I'm literally fixing my error from few years ago.

[mav]

In D41627#949513, @meka_tilda.center wrote:

In D41627#949189, @mav wrote:

After your initial commit Hans reduced the default value from 8ms to the minimum of 2ms. I wonder if it is still OK or you'd like to reduce default also.

If you look at the commit linked in description you'll see that that's the commit that made it possible to go to 2ms. Hans asked me what works for me and back then 2ms really worked well. That being said, I'm literally fixing my error from few years ago.

I am sorry, but once again: in this patch you change allowed minimum, but do not change the default on line 99, right?

[meka_tilda.center]

In D41627#949637, @mav wrote:

I am sorry, but once again: in this patch you change allowed minimum, but do not change the default on line 99, right?

I guess heat got to my head :) Thank you, good catch! Update is coming.

[dev_submerge.ch]

Chiming in here as I have some experience with the snd_uaudio module and its code. I do support this change, but I think the title is a bit misleading.

First of all the value of uaudio_buffer_ms has not much to do with any buffer size. It changes the cadence of isochronous USB transfers. Which means the lower this value, the shorter the intervals of sending and receiving audio data. This can be observed as the size of chunks that are processed in the corresponding pcm device.

Also it acts more like a multiplier than a time value in milliseconds. A value of 2 really doubles the interval length that is defined in the USB endpoint descriptor (bInterval). Often the interval defined in the endpoint descriptor is 1ms, but that's not necessarily true for all hardware.

Now this patch here changes the default multiplicator to 1. Which actually makes snd_uaudio adhere to what the hardware suggests as transfer interval. I suppose that's a good thing.

I did some brief testing with two uaudio devices, one full speed (USB 1.1) and one high speed (USB 2.0). Found no regressions so far.

[dev_submerge.ch]

After some thought I have a different proposal:

1. Stop messing with the USB transfer interval. Insert a 1 where uaudio_buffer_ms was used for that (uaudio_configure_msg_sub()).
2. Repurpose the buffer_ms sysctl to what a user expects it to do. Let it change the buffer size and thus latency of the uaudio driver.

Opinions on that? I have a patch coming up if we go that route.

For the first point I'd still want to do some more testing and cross-check the USB specs / the result in USB packets. I don't understand why there was a multiplier on the transfer interval at all. And for some reason Hans reduced the multiplier to 2, not to 1.

About the second point, by changing the result of uaudio_get_buffer_size() for a 2x1ms buffer size, I can get the roundtrip latency down to ~6ms. That's just in the range for live processing (yay!) but requires some extreme settings and the new JACK FreeBSD backend which is not merged yet. Anyway, the catch is that this buffer size should not be lower than the USB transfer interval, so we have to check that in the code.

[mav]

As I can see, uaudio_buffer_ms is primarily used to set intr_frames variable. I haven't dug deep, but looks like it controls the size of individual USB transfer, which I suppose affects USB controller interrupt rate, or at very least the rate of its DMA transfers. Frequent DMA transfers may prevent deep sleeps on CPU package-level. Frequent interrupts in addition wake up at least one CPU core. It all burns a lot of power, that may be critical for laptops and embedded systems, while not every application require so low latency. I generally don't like tunables, and I would be happy to remove this one, but I don't like the idea of getting 1KHz of USB interrupts for every sound application. Sound subsystem already has its own concept of buffer sizes and latency control. Can't those values be reused here without additional knobs to only have level of overhead required by specific application?

[dev_submerge.ch]

Well, these are three different topics then:

1. USB compliance and device compatibility. Obviously most devices can cope with less frequent transfers, there must be a limit though (hardware buffer size). I don't know the impact of this, but we're operating out of spec.
2. Energy efficiency and wakeup frequency. From an older USB dump I see that e.g. Ubuntu does a transfer every millisecond. Maybe this can be offloaded to the USB host controller? Haven't looked into that.
3. Latency settings. AFAIK, these are communicated to sound drivers through the buffer fragment size (channel_setblocksize() method). Which is highly problematic, see below.

To my knowledge there's three ways to change the buffer fragment size: The system wide sysctl hw.snd.latency_profile, sysctl hw.snd.latency, and per application via OSS API. All of them produce very strange buffer fragments when channels or sample size are not a power of 2. For some soundcards changing the fragment size breaks playback completely. Now for the general case that is uaudio, we'd get a highly obfuscated (power of 2 rounded) indication of latency. Then interpret it in terms of channels, sample size and sample rate. And then shoehorn it into a millisecond buffer length, to match USB timing ... expect highly unpredictable results.

So until we have better ways to set and communicate latency settings, I'd still recommend a tunable specific to snd_uaudio.

[mav]

I didn't promise it to be easy, but I think USB transfer size should be as close to sound(4) fragment size, if it can't be equal, as for other sound cards.

[dev_submerge.ch]

I didn't promise it to be easy, but I think USB transfer size should be as close to sound(4) fragment size, if it can't be equal, as for other sound cards.

It's not about being easy, it's about not making sense. The uaudio driver gets a fragment size value which may already differ from the intended latency by a factor of 1.5 due to power-of-2 rounding. And then it has to round it to milliseconds, which may differ again by factor 1.5 due to ms never being a power of 2.

What we need here is a reliable way to set the buffer size in ms, for all combinations of channel count, sample rate, sample size. There's no way to achieve that through the current fragment size system. So I'm not going to invest any time in that direction, unless we're talking about replacing the fragment stuff with something usable.

I'll have a look if we could get less CPU wakeups with USB compliant transfer intervals, when I get around to.

[dev_submerge.ch]

Update: Looks like the current snd_uaudio implementation actually does transfers according to USB standard, it spreads the isochronous frames over the whole transfer interval. I was misled by parts of the code, and the fact that usbdump(8) only shows the first millisecond of longer transfers. Sorry for the noise.

This means we can use any transfer interval length from the range of 1ms to 8ms. To back up the discussion about frequent interrupts, I did some measurements of power consumption on my old ThinkPad X230.

State	Power Total	Power Inc
Idle	6.6 W
USB audio device	7.3 W
Playback 8ms interval	8.1 W	+0.8 W
Playback 4ms interval	8.2 W	+0.9 W
Playback 2ms interval	8.7 W	+1.4 W
Playback 1ms interval	9.7 W	+2.4 W

So while there's not much difference between 4ms and 8ms intervals, power consumption significantly increases for 2ms and 1ms intervals. Intuitively I would opt for a 4ms interval by default.

Now Hans changed the default to 2ms, his commit f190f8d1a268aeb386830efa5aacf1c8f29f1230 says:

Improves the audio experience when using applications like
audio/jamulus and audio/hpsjam .

I suppose this was about latency, but it only changed the transfer interval, not the buffer size. The patch I'm working on also does the latter. With a reduced buffer size, I'm confident that we can have less latency at a 4ms interval than currently at 2ms. I think that would make 4ms an acceptable default for everybody.

[mav]

In D41627#953185, @dev_submerge.ch wrote:

The patch I'm working on also does the latter. With a reduced buffer size, I'm confident that we can have less latency at a 4ms interval than currently at 2ms. I think that would make 4ms an acceptable default for everybody.

Thank you. It sounds promising.

[dev_submerge.ch]

FYI, my patch is here D41942: snd_uaudio(4): Adapt buffer length to buffer_ms tunable. for review. I'll add more info this weekend.

[meka_tilda.center]

Florian's patch is superior to this one, hence closing.