Skip to contentSkip to conversation

snd-usb-audio with multiple audio functions

PATCH In review Rolf Meeser 4 participants 6 messages Jun 6, 2011 1 message not in the archive Sound

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 snd-usb-audio with multiple audio functions

Hello,

This patch fixes the currently broken support for multiple audio functions (AF) in snd-usb-audio. The patch is against the stable 2.6.39.1 kernel.

It addresses three issues:
1. Controls of the second and any further AF are currently not accessibly (controls of the first AF are duplicated!)
2. Multiple AF's are currently incorrectly combined into one sound card index.
3. Module usage count of snd-usb-audio does not drop to zero after disconnecting a multiple AF device.

All three issues are fixed by assigning a new card index for each AF in a USB device.

Rationale:

- A USB audio function (AF) consists of a control interface and zero or more streaming interfaces. USB devices can have multiple AF, where the individual AF are completely independent of each other. In particular, it cannot be assumed that the controls of one AF can be manipulated through the control interface of another (the first) AF.

- Older kernels (up to 2.6.32? at least before the major rewrite of USB audio code) supported multiple AF's. However, they combined the streams into one sound card: For instance hw:1,0 for the stream interface of the first AF, and hw:1,1 for the stream interface of the second AF. I believe this is incorrect, as different AF's must be treated as an independent entity. Each AF must get its own card index, for the example this would be hw:1,0 and hw:2,0. This is also the way THE OTHER OS implements it.

- Newer kernels have broken multiple AF support. In a dual AF setup these kernels detect the second AF, but they duplicate the controls of the first AF, and ignore the controls of the second one!

- The patch also fixes a problem with module usage: When you unplug a multiple AF device, the snd-usb-audio module usage count currently stays at 1, while you could expect it to go to zero.

This is the first time I'm sending a patch to this list. Please be patient with me if I did something in a wrong way :-)

Regards,
Rolf

sound/usb/card.c +4 -1
diff -ur linux-2.6.39.1.orig/sound/usb/card.c linux-2.6.39.1/sound/usb/card.c
@@ -461,7 +461,10 @@
 	chip = NULL;
 	mutex_lock(&register_mutex);
 	for (i = 0; i < SNDRV_CARDS; i++) {
-		if (usb_chip[i] && usb_chip[i]->dev == dev) {
+		if (usb_chip[i] &&
+			(usb_chip[i]->dev == dev) &&
+			(usb_chip[i]->ctrl_intf) &&
+			(usb_chip[i]->ctrl_intf->desc.bInterfaceNumber == ifnum)) {
 			if (usb_chip[i]->shutdown) {
 				snd_printk(KERN_ERR "USB device is in the shutdown state, cannot create a card instance\n");
 				goto __error;
#1 lore raw

Hi Rolf,

Quoted 14 lines · Rolf Meeser

Hello,

This patch fixes the currently broken support for multiple audio functions (AF) in snd-usb-audio.
The patch is against the stable 2.6.39.1 kernel.

It addresses three issues:
1. Controls of the second and any further AF are currently not accessibly (controls of the first AF are duplicated!)
2. Multiple AF's are currently incorrectly combined into one sound card index.
3. Module usage count of snd-usb-audio does not drop to zero after disconnecting a multiple AF device.

Quoted 2 lines

All three issues are fixed by assigning a new card index for each AF in a USB device.

I assume you have such a device? Which model is that?

Quoted 21 lines

Rationale:

- A USB audio function (AF) consists of a control interface and zero or more streaming interfaces. USB devices can have multiple AF, where the individual AF are completely independent of each other. In particular, it cannot be assumed that the controls of one AF can be manipulated through the control interface of another (the first) AF.

- Older kernels (up to 2.6.32? at least before the major rewrite of USB audio code) supported multiple AF's. However, they combined the streams into one sound card: For instance hw:1,0 for the stream interface of the first AF, and hw:1,1 for the stream interface of the second AF.
I believe this is incorrect, as different AF's must be treated as an independent entity. Each AF must get its own card index, for the example this would be hw:1,0 and hw:2,0. This is also the way THE OTHER OS implements it.

- Newer kernels have broken multiple AF support. In a dual AF setup these kernels detect the second AF, but they duplicate the controls of the first AF, and ignore the controls of the second one!

Quoted 6 lines

- The patch also fixes a problem with module usage: When you unplug a multiple AF device, the snd-usb-audio module usage count currently stays at 1, while you could expect it to go to zero.

This is the first time I'm sending a patch to this list. Please be patient with me if I did something in a wrong way :-)

The patch itself looks reasonable to be, and I doubt it would break any existing functions.

However, please have a look at Documentation/SubmittingPatches. In particular, you need to have a comprehensive commit log (which can be taken from your explanations in this email) and your "Signed-off-by:" line above the actual patch. Would you care to resend?

Thanks a lot for sharing your work!

Daniel

#2 lore raw

At Mon, 6 Jun 2011 10:45:49 +0200,

Quoted 48 lines · Daniel Mack

Hi Rolf,

Quoted 14 lines · Rolf Meeser

Hello,

This patch fixes the currently broken support for multiple audio functions (AF) in snd-usb-audio.
The patch is against the stable 2.6.39.1 kernel.

It addresses three issues:
1. Controls of the second and any further AF are currently not accessibly (controls of the first AF are duplicated!)
2. Multiple AF's are currently incorrectly combined into one sound card index.
3. Module usage count of snd-usb-audio does not drop to zero after disconnecting a multiple AF device.

Quoted 2 lines

All three issues are fixed by assigning a new card index for each AF in a USB device.

I assume you have such a device? Which model is that?

Quoted 25 lines

Rationale:

- A USB audio function (AF) consists of a control interface and zero or more streaming interfaces. USB devices can have multiple AF, where the individual AF are completely independent of each other. In particular, it cannot be assumed that the controls of one AF can be manipulated through the control interface of another (the first) AF.

- Older kernels (up to 2.6.32? at least before the major rewrite of USB audio code) supported multiple AF's. However, they combined the streams into one sound card: For instance hw:1,0 for the stream interface of the first AF, and hw:1,1 for the stream interface of the second AF.
I believe this is incorrect, as different AF's must be treated as an independent entity. Each AF must get its own card index, for the example this would be hw:1,0 and hw:2,0. This is also the way THE OTHER OS implements it.

- Newer kernels have broken multiple AF support. In a dual AF setup these kernels detect the second AF, but they duplicate the controls of the first AF, and ignore the controls of the second one!

- The patch also fixes a problem with module usage: When you unplug a multiple AF device, the snd-usb-audio module usage count currently stays at 1, while you could expect it to go to zero.

Quoted 5 lines
Quoted 2 lines

This is the first time I'm sending a patch to this list. Please be patient with me if I did something in a wrong way :-)

The patch itself looks reasonable to be, and I doubt it would break any existing functions.

Yeah, I think so, too.

Quoted 4 lines

However, please have a look at Documentation/SubmittingPatches. In particular, you need to have a comprehensive commit log (which can be taken from your explanations in this email) and your "Signed-off-by:" line above the actual patch. Would you care to resend?

FYI, about sign-off (and in general about the kernel patches), refer to $LINUX/Documentation/SubmittingPatches document.

thanks,

Takashi

#3 lore raw

Hi Takashi,

Quoted 4 lines · Takashi Iwai
Quoted 2 lines

The patch itself looks reasonable to be, and I doubt it would break any existing functions.

Yeah, I think so, too.

I have to disagree :-)
Even worse, I think it is almost nonsense.

Let me figure out another solution which will have to take Clemens' concerns into account.

Quoted 7 lines
Quoted 4 lines

However, please have a look at Documentation/SubmittingPatches. In particular, you need to have a comprehensive commit log (which can be taken from your explanations in this email) and your "Signed-off-by:" line above the actual patch. Would you care to resend?

FYI, about sign-off (and in general about the kernel patches), refer to $LINUX/Documentation/SubmittingPatches document.

I'll submit a clean patch next time!

Regards,
Rolf

#4 lore raw
In reply to Rolf Meeser

sound/usb/card.c line 467

Quoted 4 lines · Rolf Meeser
sound/usb/card.c +4 -0
+		if (usb_chip[i] &&
+			(usb_chip[i]->dev == dev) &&
+			(usb_chip[i]->ctrl_intf) &&
+			(usb_chip[i]->ctrl_intf->desc.bInterfaceNumber == ifnum)) {

Many audio devices with multiple vendor-specific interfaces do not have a control interface. This code would split them into multiple cards.

Regards,
Clemens

A message that is not in the archive (sent off-list or not yet received): [email protected]

#6 lore raw
Quoted 6 lines · Rolf Meeser
Quoted 2 lines · Clemens Ladisch

Many audio devices with multiple vendor-specific interfaces do not have a control interface. This code would split them into multiple cards.

Breaking existing setups would indeed be bad.
Are that cards with audio interfaces not following the Audio Class spec?

Yes.

Quoted 2 lines

What would be the easiest way to identify them, so we could assign a new card index only in case of a compliant card?

Non-compliant devices don't have a control interface.

In theory, the USB core probes a device's interfaces in order, so the control interface usually is probed first. However, I'm not sure if this is really true for all devices. It's probably safest to check explicitly if the device has any interface marked as audio control 1/2.

Regards,
Clemens