usb:gadget:f_uac2: RFC: allowing multiple altsetttings for channel/samplesize combinations
Hi,
I am considering implementation of multiple altsettings to f_uac2, so that multiple combinations of channels and samplesizes can be offered to the host.
Configuration: -------------- * each altsetting for each direction should define * channel mask * samplesize * hs_bint bInterval * c_sync type (for capture only)
Perhaps the easiest config would be allowing lists for the existing parameters (like the multiple samplerates were implemented). All the list params would have to have the same number of items - initial check. First values in the list would apply to altsetting 1, second to altsetting 2 etc.
Or the altsetting could be some structured configfs param - please is there any recommended standard for structured configfs params?
Should the config also adjust the list of allowed samplerates for each altsetting? Technically it makes sense as higher number of channels can decrease the max samplerate, e.g. for via a TDM interface. If so, it would need either the structured configuration or some "list of lists" format.
Implementation: ---------------
Parameters could be turned to arrays of fixed predefined sizes, like the p/s_srates. E.g. 5 max. altsettings in each direction would consume only 4 * (5-1) + 3* (5-1) = 28 extra ints (excluding the samplerates config).
Currently all descriptor structs are statically pre-allocated as there are only two hard-coded altsettings. IMO the descriptors specific for each altsetting could be allocated dynamically in a loop over all none-zero alsettings.
Please may I ask UAC2 gadget "stakeholders" for comments, suggestions, recommendations, so that my eventual initial version was in some generally acceptable direction?
Thanks a lot,
Pavel.
On 17. 04. 24 13:07, Pavel Hofman wrote:
I am considering implementation of multiple altsettings to f_uac2, so that multiple combinations of channels and samplesizes can be offered to the host.
Configuration:
- each altsetting for each direction should define
* channel mask * samplesize * hs_bint bInterval * c_sync type (for capture only)
Perhaps the easiest config would be allowing lists for the existing parameters (like the multiple samplerates were implemented). All the list params would have to have the same number of items - initial check. First values in the list would apply to altsetting 1, second to altsetting 2 etc.
Or the altsetting could be some structured configfs param - please is there any recommended standard for structured configfs params?
Should the config also adjust the list of allowed samplerates for each altsetting? Technically it makes sense as higher number of channels can decrease the max samplerate, e.g. for via a TDM interface. If so, it would need either the structured configuration or some "list of lists" format.
Implementation:
Parameters could be turned to arrays of fixed predefined sizes, like the p/s_srates. E.g. 5 max. altsettings in each direction would consume only 4 * (5-1) + 3* (5-1) = 28 extra ints (excluding the samplerates config).
Currently all descriptor structs are statically pre-allocated as there are only two hard-coded altsettings. IMO the descriptors specific for each altsetting could be allocated dynamically in a loop over all none-zero alsettings.
Please may I ask UAC2 gadget "stakeholders" for comments, suggestions, recommendations, so that my eventual initial version was in some generally acceptable direction?
This feature has coincidentally arisen in recent commits by Chris https://lore.kernel.org/lkml/c9928edb-8b2d-1948-40b8-c16e34cea3e2@ivitera.co...
Maybe Takashi's commits to the midi gadget could be a way https://patchwork.kernel.org/project/alsa-devel/list/?series=769151&stat... The midi gadget allows multiple configurations now, where configs are placed into a separate block.X configfs directory. That way the configfs recommendation to keep one value per item is adhered to and the configuration is nice and clean.
This method would nicely allow various samplerate lists for each altsetting, without having to use some obscure list of lists.
The f_uac2 tree config could have e.g. alt.1-X subdirs, to fit the altsetting ID. I am not sure the dot index not starting with 0 would be an issue.
Now the question would be what to do with the existing (and the new params added by Chris) flat-structure parameters which apply to (the only one) altsetting 1. Maybe they could be used as defaults for the other altsettings for unspecified parameters?
I very much appreciate any input, thank you all in advance.
With regards,
Pavel.
On Wed, 24 Apr 2024 09:40:52 +0200, Pavel Hofman wrote:
On 17. 04. 24 13:07, Pavel Hofman wrote:
I am considering implementation of multiple altsettings to f_uac2, so that multiple combinations of channels and samplesizes can be offered to the host.
Configuration:
- each altsetting for each direction should define
* channel mask * samplesize * hs_bint bInterval * c_sync type (for capture only)
Perhaps the easiest config would be allowing lists for the existing parameters (like the multiple samplerates were implemented). All the list params would have to have the same number of items - initial check. First values in the list would apply to altsetting 1, second to altsetting 2 etc.
Or the altsetting could be some structured configfs param - please is there any recommended standard for structured configfs params?
Should the config also adjust the list of allowed samplerates for each altsetting? Technically it makes sense as higher number of channels can decrease the max samplerate, e.g. for via a TDM interface. If so, it would need either the structured configuration or some "list of lists" format.
Implementation:
Parameters could be turned to arrays of fixed predefined sizes, like the p/s_srates. E.g. 5 max. altsettings in each direction would consume only 4 * (5-1) + 3* (5-1) = 28 extra ints (excluding the samplerates config).
Currently all descriptor structs are statically pre-allocated as there are only two hard-coded altsettings. IMO the descriptors specific for each altsetting could be allocated dynamically in a loop over all none-zero alsettings.
Please may I ask UAC2 gadget "stakeholders" for comments, suggestions, recommendations, so that my eventual initial version was in some generally acceptable direction?
This feature has coincidentally arisen in recent commits by Chris https://lore.kernel.org/lkml/c9928edb-8b2d-1948-40b8-c16e34cea3e2@ivitera.co...
Maybe Takashi's commits to the midi gadget could be a way https://patchwork.kernel.org/project/alsa-devel/list/?series=769151&stat... The midi gadget allows multiple configurations now, where configs are placed into a separate block.X configfs directory. That way the configfs recommendation to keep one value per item is adhered to and the configuration is nice and clean.
This method would nicely allow various samplerate lists for each altsetting, without having to use some obscure list of lists.
The f_uac2 tree config could have e.g. alt.1-X subdirs, to fit the altsetting ID. I am not sure the dot index not starting with 0 would be an issue.
Now the question would be what to do with the existing (and the new params added by Chris) flat-structure parameters which apply to (the only one) altsetting 1. Maybe they could be used as defaults for the other altsettings for unspecified parameters?
I very much appreciate any input, thank you all in advance.
IMO, a softer approach would be to use subdirs for alt1+ while flat files are kept used for alt0. Alternatively, we may allow creating alt0, too, and the values there will win over the flat values.
thanks,
Takashi
On 25. 04. 24 11:22, Takashi Iwai wrote:
On Wed, 24 Apr 2024 09:40:52 +0200, Pavel Hofman wrote:
On 17. 04. 24 13:07, Pavel Hofman wrote:
I am considering implementation of multiple altsettings to f_uac2, so that multiple combinations of channels and samplesizes can be offered to the host.
Configuration:
- each altsetting for each direction should define
* channel mask * samplesize * hs_bint bInterval * c_sync type (for capture only)
Perhaps the easiest config would be allowing lists for the existing parameters (like the multiple samplerates were implemented). All the list params would have to have the same number of items - initial check. First values in the list would apply to altsetting 1, second to altsetting 2 etc.
Or the altsetting could be some structured configfs param - please is there any recommended standard for structured configfs params?
Should the config also adjust the list of allowed samplerates for each altsetting? Technically it makes sense as higher number of channels can decrease the max samplerate, e.g. for via a TDM interface. If so, it would need either the structured configuration or some "list of lists" format.
Implementation:
Parameters could be turned to arrays of fixed predefined sizes, like the p/s_srates. E.g. 5 max. altsettings in each direction would consume only 4 * (5-1) + 3* (5-1) = 28 extra ints (excluding the samplerates config).
Currently all descriptor structs are statically pre-allocated as there are only two hard-coded altsettings. IMO the descriptors specific for each altsetting could be allocated dynamically in a loop over all none-zero alsettings.
Please may I ask UAC2 gadget "stakeholders" for comments, suggestions, recommendations, so that my eventual initial version was in some generally acceptable direction?
This feature has coincidentally arisen in recent commits by Chris https://lore.kernel.org/lkml/c9928edb-8b2d-1948-40b8-c16e34cea3e2@ivitera.co...
Maybe Takashi's commits to the midi gadget could be a way https://patchwork.kernel.org/project/alsa-devel/list/?series=769151&stat... The midi gadget allows multiple configurations now, where configs are placed into a separate block.X configfs directory. That way the configfs recommendation to keep one value per item is adhered to and the configuration is nice and clean.
This method would nicely allow various samplerate lists for each altsetting, without having to use some obscure list of lists.
The f_uac2 tree config could have e.g. alt.1-X subdirs, to fit the altsetting ID. I am not sure the dot index not starting with 0 would be an issue.
Now the question would be what to do with the existing (and the new params added by Chris) flat-structure parameters which apply to (the only one) altsetting 1. Maybe they could be used as defaults for the other altsettings for unspecified parameters?
I very much appreciate any input, thank you all in advance.
IMO, a softer approach would be to use subdirs for alt1+ while flat files are kept used for alt0.
Thanks a lot for your help. IIUC almost all existing configs for the UAC1/2 functions apply to the "run" altsetting - altsetting 1. The altsetting 0 is for stopping the operation, IIUC. Actually that's how the stream stop is detected on the gadget side - transition alt1 -> alt0.
Did you perhaps mean the first "running" altsetting?
Alternatively, we may allow creating alt0, too, and the values there will win over the flat values.
IIUC this would be the meaning of default configs, in case they are not specified in the subdir. I like that as it would make the configuration easier if not generated by some script.
Thanks,
Pavel.
Pavel,
Following up on this and discussions in this patch series
https://lore.kernel.org/linux-usb/CO1PR17MB54195BE778868AFDFE2DCB36E1112@CO1...
Example from that thread with c_alt_name changed to c_name as it lives in an "alt.x" directory and removal of the num_alt_modes in favor of just allowing "mkdir alt.x" to create new alt mode settings:
(all existing properties + the following new properties) c_it_name c_it_ch_name c_fu_name c_ot_name p_it_name p_it_ch_name p_fu_name p_ot_name
alt.0 c_name p_name alt.1 (for alt.1, alt.2, etc.) c_name p_name c_ssize p_ssize (Additional properties here for other things that are settable for each alt mode, but the only one I've implemented in my prototype so far is sample size.)
Here is a more detailed breakdown of the (current) proposal:
* Allow the user to create "alt.x" directories inside uac1 and uac2 function configfs * Prior to creation of any of these alt.x subdirectories, the function behaves the same as it does today. No "alt.x" directories would exist on creation of the function. * Creation of "alt.0" contains only "c_name" and "p_name" to set the USB string name for this alt mode, with the defaults as "Capture Inactive" and "Playback Inactive" * Creation of "alt.x" where x is an integer, contains the same name strings but with defaults of "Capture Active" and "Playback Active" and additional files for any per-alt-mode configurable settings (such as c_ssize, p_ssize, etc.) At the time of creation, values for those are copied from the corresponding settings in the function main configfs directory. * Creation of "alt.1" in particular changes the meaning of the files in the main function configfs dir so that they are only _default_ values used when creating "alt.x" directories and settings from "alt.1" will now be used for alt mode 1. (Alt mode 1 always exists, even when "alt.1" has not been created.)
* ALSA card interface behavior * Configuration of the ALSA card interface remains the same. It is configured when binding the function to match the settings in the main function configfs. "alt.x" settings have no effect on the created ALSA card setup * Sample size will be converted between ALSA and USB data by dropping extra bits or zero padding samples (eg 16->24 will zero pad a byte, 24->16 will drop a byte) * Channel count differences will ignore extra incoming channels and zero outgoing extra channels (eg 8->2 will just copy the first two and ignore the rest. 2->8 will copy the first two and zero the rest.) * Per-alt-mode configurable settings will have a read-only ALSA control (like sample rate does currently) that can be used to inform the program attached to the ALSA card what the current state of those settings are when the USB host selects an alt mode.
An simple example of how this could be used to create a second alt mode:
mkdir uac1.0 echo 24 > uac1.0/p_ssize echo 24 > uac1.0/c_ssize mkdir uac1.0/alt.2 echo 16 > uac1.0/alt.2/c_ssize
NOTE: Alt modes are separately selectable by the host for playback and capture so the host can pick and choose as desired. There is no need to make an alt mode for every combination of playback and capture settings. In this example the host can pick either 24 or 16 bit samples for capture, but is only allowed 24 bit samples for playback as both alt.1 (via uac1.0/p_ssize) and alt.2 (via default copied to uac1.0/alt.2/p_ssize) have been configured as 24-bit.
On Thu, Apr 25, 2024 at 11:08 AM Pavel Hofman pavel.hofman@ivitera.com wrote:
On 25. 04. 24 11:22, Takashi Iwai wrote:
On Wed, 24 Apr 2024 09:40:52 +0200, Pavel Hofman wrote:
On 17. 04. 24 13:07, Pavel Hofman wrote:
I am considering implementation of multiple altsettings to f_uac2, so that multiple combinations of channels and samplesizes can be offered to the host.
Configuration:
- each altsetting for each direction should define
- channel mask
- samplesize
- hs_bint bInterval
- c_sync type (for capture only)
Perhaps the easiest config would be allowing lists for the existing parameters (like the multiple samplerates were implemented). All the list params would have to have the same number of items - initial check. First values in the list would apply to altsetting 1, second to altsetting 2 etc.
Or the altsetting could be some structured configfs param - please is there any recommended standard for structured configfs params?
Should the config also adjust the list of allowed samplerates for each altsetting? Technically it makes sense as higher number of channels can decrease the max samplerate, e.g. for via a TDM interface. If so, it would need either the structured configuration or some "list of lists" format.
Implementation:
Parameters could be turned to arrays of fixed predefined sizes, like the p/s_srates. E.g. 5 max. altsettings in each direction would consume only 4 * (5-1) + 3* (5-1) = 28 extra ints (excluding the samplerates config).
Currently all descriptor structs are statically pre-allocated as there are only two hard-coded altsettings. IMO the descriptors specific for each altsetting could be allocated dynamically in a loop over all none-zero alsettings.
Please may I ask UAC2 gadget "stakeholders" for comments, suggestions, recommendations, so that my eventual initial version was in some generally acceptable direction?
This feature has coincidentally arisen in recent commits by Chris https://lore.kernel.org/lkml/c9928edb-8b2d-1948-40b8-c16e34cea3e2@ivitera.co...
Maybe Takashi's commits to the midi gadget could be a way https://patchwork.kernel.org/project/alsa-devel/list/?series=769151&stat... The midi gadget allows multiple configurations now, where configs are placed into a separate block.X configfs directory. That way the configfs recommendation to keep one value per item is adhered to and the configuration is nice and clean.
This method would nicely allow various samplerate lists for each altsetting, without having to use some obscure list of lists.
The f_uac2 tree config could have e.g. alt.1-X subdirs, to fit the altsetting ID. I am not sure the dot index not starting with 0 would be an issue.
Now the question would be what to do with the existing (and the new params added by Chris) flat-structure parameters which apply to (the only one) altsetting 1. Maybe they could be used as defaults for the other altsettings for unspecified parameters?
I very much appreciate any input, thank you all in advance.
IMO, a softer approach would be to use subdirs for alt1+ while flat files are kept used for alt0.
Thanks a lot for your help. IIUC almost all existing configs for the UAC1/2 functions apply to the "run" altsetting - altsetting 1. The altsetting 0 is for stopping the operation, IIUC. Actually that's how the stream stop is detected on the gadget side - transition alt1 -> alt0.
Did you perhaps mean the first "running" altsetting?
Alternatively, we may allow creating alt0, too, and the values there will win over the flat values.
IIUC this would be the meaning of default configs, in case they are not specified in the subdir. I like that as it would make the configuration easier if not generated by some script.
Thanks,
Pavel.
Apologies for the terrible formatting. Apparently the gmail client mangles plain text email. Server me right for trying to respond on the weekend. Here's the reply with fixed formatting. Pavel,
Following up on this and discussions in this patch series
https://lore.kernel.org/linux-usb/CO1PR17MB54195BE778868AFDFE2DCB36E1112@CO1...
Example from that thread with c_alt_name changed to c_name as it lives in an "alt.x" directory and removal of the num_alt_modes in favor of just allowing "mkdir alt.x" to create new alt mode settings:
(all existing properties + the following new properties) c_it_name c_it_ch_name c_fu_name c_ot_name p_it_name p_it_ch_name p_fu_name p_ot_name
alt.0 c_name p_name alt.1 (for alt.1, alt.2, etc.) c_name p_name c_ssize p_ssize (Additional properties here for other things that are settable for each alt mode, but the only one I've implemented in my prototype so far is sample size.)
Here is a more detailed breakdown of the (current) proposal:
* Allow the user to create "alt.x" directories inside uac1 and uac2 function configfs * Prior to creation of any of these alt.x subdirectories, the function behaves the same as it does today. No "alt.x" directories would exist on creation of the function. * Creation of "alt.0" contains only "c_name" and "p_name" to set the USB string name for this alt mode, with the defaults as "Capture Inactive" and "Playback Inactive" * Creation of "alt.x" where x is an integer, contains the same name strings but with defaults of "Capture Active" and "Playback Active" and additional files for any per-alt-mode configurable settings (such as c_ssize, p_ssize, etc.) At the time of creation, values for those are copied from the corresponding settings in the function main configfs directory. * Creation of "alt.1" in particular changes the meaning of the files in the main function configfs dir so that they are only _default_ values used when creating "alt.x" directories and settings from "alt.1" will now be used for alt mode 1. (Alt mode 1 always exists, even when "alt.1" has not been created.)
* ALSA card interface behavior * Configuration of the ALSA card interface remains the same. It is configured when binding the function to match the settings in the main function configfs. "alt.x" settings have no effect on the created ALSA card setup * Sample size will be converted between ALSA and USB data by dropping extra bits or zero padding samples (eg 16->24 will zero pad a byte, 24->16 will drop a byte) * Channel count differences will ignore extra incoming channels and zero outgoing extra channels (eg 8->2 will just copy the first two and ignore the rest. 2->8 will copy the first two and zero the rest.) * Per-alt-mode configurable settings will have a read-only ALSA control (like sample rate does currently) that can be used to inform the program attached to the ALSA card what the current state of those settings are when the USB host selects an alt mode.
An simple example of how this could be used to create a second alt mode:
mkdir uac1.0 echo 24 > uac1.0/p_ssize echo 24 > uac1.0/c_ssize mkdir uac1.0/alt.2 echo 16 > uac1.0/alt.2/c_ssize
NOTE: Alt modes are separately selectable by the host for playback and capture so the host can pick and choose as desired. There is no need to make an alt mode for every combination of playback and capture settings. In this example the host can pick either 24 or 16 bit samples for capture, but is only allowed 24 bit samples for playback as both alt.1 (via uac1.0/p_ssize) and alt.2 (via default copied to uac1.0/alt.2/p_ssize) have been configured as 24-bit.
On Thu, Apr 25, 2024 at 11:08 AM Pavel Hofman pavel.hofman@ivitera.com wrote:
On 25. 04. 24 11:22, Takashi Iwai wrote:
On Wed, 24 Apr 2024 09:40:52 +0200, Pavel Hofman wrote:
On 17. 04. 24 13:07, Pavel Hofman wrote:
I am considering implementation of multiple altsettings to f_uac2, so that multiple combinations of channels and samplesizes can be offered to the host.
Configuration:
- each altsetting for each direction should define
* channel mask * samplesize * hs_bint bInterval * c_sync type (for capture only)
Perhaps the easiest config would be allowing lists for the existing parameters (like the multiple samplerates were implemented). All the list params would have to have the same number of items - initial check. First values in the list would apply to altsetting 1, second to altsetting 2 etc.
Or the altsetting could be some structured configfs param - please is there any recommended standard for structured configfs params?
Should the config also adjust the list of allowed samplerates for each altsetting? Technically it makes sense as higher number of channels can decrease the max samplerate, e.g. for via a TDM interface. If so, it would need either the structured configuration or some "list of lists" format.
Implementation:
Parameters could be turned to arrays of fixed predefined sizes, like the p/s_srates. E.g. 5 max. altsettings in each direction would consume only 4 * (5-1) + 3* (5-1) = 28 extra ints (excluding the samplerates config).
Currently all descriptor structs are statically pre-allocated as there are only two hard-coded altsettings. IMO the descriptors specific for each altsetting could be allocated dynamically in a loop over all none-zero alsettings.
Please may I ask UAC2 gadget "stakeholders" for comments, suggestions, recommendations, so that my eventual initial version was in some generally acceptable direction?
This feature has coincidentally arisen in recent commits by Chris https://urldefense.com/v3/__https://lore.kernel.org/lkml/c9928edb-8b2d-1948-...
Maybe Takashi's commits to the midi gadget could be a way https://urldefense.com/v3/__https://patchwork.kernel.org/project/alsa-devel/... The midi gadget allows multiple configurations now, where configs are placed into a separate block.X configfs directory. That way the configfs recommendation to keep one value per item is adhered to and the configuration is nice and clean.
This method would nicely allow various samplerate lists for each altsetting, without having to use some obscure list of lists.
The f_uac2 tree config could have e.g. alt.1-X subdirs, to fit the altsetting ID. I am not sure the dot index not starting with 0 would be an issue.
Now the question would be what to do with the existing (and the new params added by Chris) flat-structure parameters which apply to (the only one) altsetting 1. Maybe they could be used as defaults for the other altsettings for unspecified parameters?
I very much appreciate any input, thank you all in advance.
IMO, a softer approach would be to use subdirs for alt1+ while flat files are kept used for alt0.
Thanks a lot for your help. IIUC almost all existing configs for the UAC1/2 functions apply to the "run" altsetting - altsetting 1. The altsetting 0 is for stopping the operation, IIUC. Actually that's how the stream stop is detected on the gadget side - transition alt1 -> alt0.
Did you perhaps mean the first "running" altsetting?
Alternatively, we may allow creating alt0, too, and the values there will win over the flat values.
IIUC this would be the meaning of default configs, in case they are not specified in the subdir. I like that as it would make the configuration easier if not generated by some script.
Thanks,
Pavel.
On 28. 04. 24 18:38, Chris Wulff wrote:
Pavel,
Following up on this and discussions in this patch series
https://lore.kernel.org/linux-usb/CO1PR17MB54195BE778868AFDFE2DCB36E1112@CO1...
Example from that thread with c_alt_name changed to c_name as it lives in an "alt.x" directory and removal of the num_alt_modes in favor of just allowing "mkdir alt.x" to create new alt mode settings:
(all existing properties + the following new properties) c_it_name c_it_ch_name c_fu_name c_ot_name p_it_name p_it_ch_name p_fu_name p_ot_name
alt.0 c_name p_name alt.1 (for alt.1, alt.2, etc.) c_name p_name c_ssize p_ssize (Additional properties here for other things that are settable for each alt mode, but the only one I've implemented in my prototype so far is sample size.)
Please let me a question regarding some of the proposed string configs. Currently we have one feature unit which implements volume and mute feature controls. IIUC more feature units can be added, with specific controls set, as specified by the UAC. IIUC the linux USB audio driver works with volume+mute specifically and other controls by creating corresponding alsa ctls, Windows UAC2 driver works with AGC too https://learn.microsoft.com/en-us/windows-hardware/drivers/audio/usb-2-0-aud... . That means there is a potential for adding more feature units to the gadget.
Would it make sense to name the p/c_fu_name properties specifically p/c_fu_volume_name, to leave name room for additional feature units?
Please what does p/c_it_ch_name stand for?
Here is a more detailed breakdown of the (current) proposal:
- Allow the user to create "alt.x" directories inside uac1 and uac2 function configfs
* Prior to creation of any of these alt.x subdirectories, the function behaves the same as it does today. No "alt.x" directories would exist on creation of the function. * Creation of "alt.0" contains only "c_name" and "p_name" to set the USB string name for this alt mode, with the defaults as "Capture Inactive" and "Playback Inactive" * Creation of "alt.x" where x is an integer, contains the same name strings but with defaults of "Capture Active" and "Playback Active" and additional files for any per-alt-mode configurable settings (such as c_ssize, p_ssize, etc.) At the time of creation, values for those are copied from the corresponding settings in the function main configfs directory. * Creation of "alt.1" in particular changes the meaning of the files in the main function configfs dir so that they are only _default_ values used when creating "alt.x" directories and settings from "alt.1" will now be used for alt mode 1. (Alt mode 1 always exists, even when "alt.1" has not been created.)
- ALSA card interface behavior
* Configuration of the ALSA card interface remains the same. It is configured when binding the function to match the settings in the main function configfs. "alt.x" settings have no effect on the created ALSA card setup * Sample size will be converted between ALSA and USB data by dropping extra bits or zero padding samples (eg 16->24 will zero pad a byte, 24->16 will drop a byte)
Is not this work for userspace, e.g. for the plug plugin? IMO the hw_params should reflect the requested format as is now, doing no conversions in the gadget driver. Currently the driver just copies the buffer from the packet area to the alsa area which is the correct way, IMO.
It also allows for future addition of FLOAT_LE format which is part of UAC specs and which (at least) the windows and linux host drivers support (for which I would already have a use case). Actually the linux and windows USB audio driver supports the TYPE III digital formats.
IIUC the gadget itself does not (and should not, IMO) care much about the actual format (apart of converting the USB format ID to the alsa format code for hw_params).
* Channel count differences will ignore extra incoming channels and zero outgoing extra channels (eg 8->2 will just copy the first two and ignore the rest. 2->8 will copy the first two and zero the rest.)
I think it's dtto. Either alsa automatic plug plugin, or detailed with the route plugin.
* Per-alt-mode configurable settings will have a read-only ALSA control (like sample rate does currently) that can be used to inform the program attached to the ALSA card what the current state of those settings are when the USB host selects an alt mode.
The fact is that samplerate is already reported in a separate ctl. But the main motivation for this control was not to report the rate, but to give some indication that USB host started streaming/requesting data. The rate ctl was actually handy to report both this change and the actual rate.
Actually there have been submitted patches (IMO not yet included) which report this change using uevents https://patchwork.kernel.org/project/linux-usb/list/?series=745790&state... . IMO a perfectly valid approach too.
Also the alsa loopback device is very similar from this POV. It reports (via ctl notifications) that the other/master side has been opened, and it's up to the userspace to read current hw_params to determine what format/channels/rate to use to successfully open the device.
Maybe we could leave it to the userspace here too. In fact there are already open-source clients which try to handle this master/slave principle of the loopback and gadget basically in the same way https://github.com/HEnquist/camilladsp/pull/341#issue-2267218348 https://github.com/HEnquist/camilladsp/issues/342
An simple example of how this could be used to create a second alt mode:
mkdir uac1.0 echo 24 > uac1.0/p_ssize echo 24 > uac1.0/c_ssize mkdir uac1.0/alt.2 echo 16 > uac1.0/alt.2/c_ssize
Currently the p/c_ssize values represent number of bytes (i.e. 1-4). IMO it would make sense to keep this meaning here. In the future e.g. negative numbers could be used to select some non-integer format types.
NOTE: Alt modes are separately selectable by the host for playback and capture so the host can pick and choose as desired. There is no need to make an alt mode for every combination of playback and capture settings. In this example the host can pick either 24 or 16 bit samples for capture, but is only allowed 24 bit samples for playback as both alt.1 (via uac1.0/p_ssize) and alt.2 (via default copied to uac1.0/alt.2/p_ssize) have been configured as 24-bit.
Are capture and playback alt modes dependent? I thought they were separate configurations but I may be wrong.
If they are separate, perhaps p_alt.X and c_alt.X dirs would make sense instead, with using non-prefixed properties within them (ssize, ch_mask, etc.). I.e. p/c_xxx on the main level (setting the defaults and default alt1 for each direction) and non-prefixed properties in the actual p/c_alt.X subdirs.
Thanks a lot for your great effort,
Pavel.
On 4.29.2024 11:02 Pavel Hofman wrote:
Example from that thread with c_alt_name changed to c_name as it lives in an "alt.x" directory and removal of the num_alt_modes in favor of just allowing "mkdir alt.x" to create new alt mode settings:
(all existing properties + the following new properties) c_it_name c_it_ch_name c_fu_name c_ot_name p_it_name p_it_ch_name p_fu_name p_ot_name
alt.0 c_name p_name alt.1 (for alt.1, alt.2, etc.) c_name p_name c_ssize p_ssize (Additional properties here for other things that are settable for each alt mode, but the only one I've implemented in my prototype so far is sample size.)
Please let me a question regarding some of the proposed string configs. Currently we have one feature unit which implements volume and mute feature controls. IIUC more feature units can be added, with specific controls set, as specified by the UAC. IIUC the linux USB audio driver works with volume+mute specifically and other controls by creating corresponding alsa ctls, Windows UAC2 driver works with AGC too https://learn.microsoft.com/en-us/windows-hardware/drivers/audio/usb-2-0-aud... . That means there is a potential for adding more feature units to the gadget.
Would it make sense to name the p/c_fu_name properties specifically p/c_fu_volume_name, to leave name room for additional feature units?
Yeah, I think that makes sense. I will change it to be p/c_fu_volume_name.
Please what does p/c_it_ch_name stand for?
This is the iChannelNames string from the Input Terminal descriptor "the name of the first logical channel" from the UAC1 spec.
Here is a more detailed breakdown of the (current) proposal:
- Allow the user to create "alt.x" directories inside uac1 and uac2
function configfs * Prior to creation of any of these alt.x subdirectories, the function behaves the same as it does today. No "alt.x" directories would exist on creation of the function. * Creation of "alt.0" contains only "c_name" and "p_name" to set the USB string name for this alt mode, with the defaults as "Capture Inactive" and "Playback Inactive" * Creation of "alt.x" where x is an integer, contains the same name strings but with defaults of "Capture Active" and "Playback Active" and additional files for any per-alt-mode configurable settings (such as c_ssize, p_ssize, etc.) At the time of creation, values for those are copied from the corresponding settings in the function main configfs directory. * Creation of "alt.1" in particular changes the meaning of the files in the main function configfs dir so that they are only _default_ values used when creating "alt.x" directories and settings from "alt.1" will now be used for alt mode 1. (Alt mode 1 always exists, even when "alt.1" has not been created.)
- ALSA card interface behavior
* Configuration of the ALSA card interface remains the same. It is configured when binding the function to match the settings in the main function configfs. "alt.x" settings have no effect on the created ALSA card setup * Sample size will be converted between ALSA and USB data by dropping extra bits or zero padding samples (eg 16->24 will zero pad a byte, 24->16 will drop a byte)
Is not this work for userspace, e.g. for the plug plugin? IMO the hw_params should reflect the requested format as is now, doing no conversions in the gadget driver. Currently the driver just copies the buffer from the packet area to the alsa area which is the correct way, IMO.
It also allows for future addition of FLOAT_LE format which is part of UAC specs and which (at least) the windows and linux host drivers support (for which I would already have a use case). Actually the linux and windows USB audio driver supports the TYPE III digital formats.
IIUC the gadget itself does not (and should not, IMO) care much about the actual format (apart of converting the USB format ID to the alsa format code for hw_params).
* Channel count differences will ignore extra incoming channels and zero outgoing extra channels (eg 8->2 will just copy the first two and ignore the rest. 2->8 will copy the first two and zero the rest.)
I think it's dtto. Either alsa automatic plug plugin, or detailed with the route plugin.
* Per-alt-mode configurable settings will have a read-only ALSA control (like sample rate does currently) that can be used to inform the program attached to the ALSA card what the current state of those settings are when the USB host selects an alt mode.
The fact is that samplerate is already reported in a separate ctl. But the main motivation for this control was not to report the rate, but to give some indication that USB host started streaming/requesting data. The rate ctl was actually handy to report both this change and the actual rate.
Actually there have been submitted patches (IMO not yet included) which report this change using uevents https://patchwork.kernel.org/project/linux-usb/list/?series=745790&state... . IMO a perfectly valid approach too.
Also the alsa loopback device is very similar from this POV. It reports (via ctl notifications) that the other/master side has been opened, and it's up to the userspace to read current hw_params to determine what format/channels/rate to use to successfully open the device.
Maybe we could leave it to the userspace here too. In fact there are already open-source clients which try to handle this master/slave principle of the loopback and gadget basically in the same way https://github.com/HEnquist/camilladsp/pull/341*issue-2267218348 https://github.com/HEnquist/camilladsp/issues/342
I will take a look at the references you provided. I may have solved this problem in a different way and maybe there is a better way to handle this.
The primary use case for the USB gadget interface of the products that I am working on is to interface with UC clients (like Microsoft Teams or Google Meet).
My basic problem is that I'm using alsaloop to connect the UAC gadget to a second ALSA interface. I ended up making modifications to alsaloop that keeps it running correctly when the host starts/stops audio on the USB interface. Without doing that I was having trouble with dropping the start of speaking or high latency when the USB host decides to start streaming audio because of the time required to get alsaloop running again after getting notified that a different alt mode was selected. I do still have the plug plugin in the pipeline as well, so this does result in a double conversion with how I have the UAC gadget driver currently doing sample size conversion.
If we allow userspace to handle all the rate/channel conversion (which does seem like a cleaner approach and is what I initially was trying to do), I think that would mean advertising multiple bit depths/channel counts in the hw_params. That part is pretty easy.
Then the userspace program can pick which to use, but what should we do with the sample data to/from USB if the userspace program picks a different combination than the alt mode currently selected by the host?
Perhaps just discarding audio when the ALSA and USB formats differ would be the right thing to do here. I might be able to solve my latency/data chopping issues instead by reconfiguring the ALSA pipeline in response to the change of the ssize ALSA control (or uevent). I think a fair bit of my time delay was re-launching alsaloop. I will experiment a bit with this and see what I can get.
An simple example of how this could be used to create a second alt mode:
mkdir uac1.0 echo 24 > uac1.0/p_ssize echo 24 > uac1.0/c_ssize mkdir uac1.0/alt.2 echo 16 > uac1.0/alt.2/c_ssize
Currently the p/c_ssize values represent number of bytes (i.e. 1-4). IMO it would make sense to keep this meaning here. In the future e.g. negative numbers could be used to select some non-integer format types.
My mistake. You are correct and it is in bytes. I was not intending to change it here. My example should have had 3 and 2 instead of 24 and 16.
NOTE: Alt modes are separately selectable by the host for playback and capture so the host can pick and choose as desired. There is no need to make an alt mode for every combination of playback and capture settings. In this example the host can pick either 24 or 16 bit samples for capture, but is only allowed 24 bit samples for playback as both alt.1 (via uac1.0/p_ssize) and alt.2 (via default copied to uac1.0/alt.2/p_ssize) have been configured as 24-bit.
Are capture and playback alt modes dependent? I thought they were separate configurations but I may be wrong.
If they are separate, perhaps p_alt.X and c_alt.X dirs would make sense instead, with using non-prefixed properties within them (ssize, ch_mask, etc.). I.e. p/c_xxx on the main level (setting the defaults and default alt1 for each direction) and non-prefixed properties in the actual p/c_alt.X subdirs.
They are indeed separate. I like this idea. I will separate these into c_alt.x and p_alt.x which can be created separately.
Thanks a lot for your great effort,
Pavel.
And thank you for your feedback. I'm happy to work towards a solution that works for more than just my particular use case.
-- Chris Wulff
On 30. 04. 24 20:51, Chris Wulff wrote:
Would it make sense to name the p/c_fu_name properties specifically p/c_fu_volume_name, to leave name room for additional feature units? Yeah, I think that makes sense. I will change it to be p/c_fu_volume_name.
Just a thought - maybe just p/c_fu_vol_name would be shorter without loosing meaning, maybe.
Please what does p/c_it_ch_name stand for?
This is the iChannelNames string from the Input Terminal descriptor "the name of the first logical channel" from the UAC1 spec.
Thanks!
Maybe we could leave it to the userspace here too. In fact there are already open-source clients which try to handle this master/slave principle of the loopback and gadget basically in the same way https://github.com/HEnquist/camilladsp/pull/341*issue-2267218348 https://github.com/HEnquist/camilladsp/issues/342
I will take a look at the references you provided. I may have solved this problem in a different way and maybe there is a better way to handle this.
The primary use case for the USB gadget interface of the products that I am working on is to interface with UC clients (like Microsoft Teams or Google Meet).
My basic problem is that I'm using alsaloop to connect the UAC gadget to a second ALSA interface. I ended up making modifications to alsaloop that keeps it running correctly when the host starts/stops audio on the USB interface. Without doing that I was having trouble with dropping the start of speaking or high latency when the USB host decides to start streaming audio because of the time required to get alsaloop running again after getting notified that a different alt mode was selected.
I see, this is a common problem. In fact that linked CamillaDSP discussion has partly the same motivation - to avoid having to restart the whole process, to keep the latency down and loose as few initial samples as possible.
I do still have the plug plugin in the pipeline as well, so this does result in a double conversion with how I have the UAC gadget driver currently doing sample size conversion.
If we allow userspace to handle all the rate/channel conversion (which does seem like a cleaner approach and is what I initially was trying to do), I think that would mean advertising multiple bit depths/channel counts in the hw_params. That part is pretty easy.
Then the userspace program can pick which to use, but what should we do with the sample data to/from USB if the userspace program picks a different combination than the alt mode currently selected by the host?
This is a very good point! I did not think about it. Using the plug plugin provides all the conversions necessary but hides the information about the original hw values.
That would perhaps suggest to really add the alsa read-only controls informing about required (and only allowed) channels and format, just like aloop does https://github.com/torvalds/linux/blob/0106679839f7c69632b3b9833c3268c316c0a...
Perhaps just discarding audio when the ALSA and USB formats differ would be the right thing to do here.
Can the USB format (incl. samplerate) change without going through altset 0 first, i.e. without explicit stop of the stream? A client subscribed to the Capture/Playback Rate ctl notifications will learn about this event first.
Currently the u_audio.c code just sends the ctl notification in set_active(). There are cases in alsa drivers where change in incoming spdif rate stops the stream too, e.g. https://github.com/torvalds/linux/blob/0106679839f7c69632b3b9833c3268c316c0a... . I vaguely remember this was discussed when implementing the current u_audio solution and no support for stopping the stream was given. Maybe it could be optionally enabled with some config parameter. It may make it easier for the clients, otherwise they get stuck and wait for timeout.
I might be able to solve my latency/data chopping issues instead by reconfiguring the ALSA pipeline in response to the change of the ssize ALSA control (or uevent). I think a fair bit of my time delay was re-launching alsaloop. I will experiment a bit with this and see what I can get.
IMO the final solution to the latency/data chopping issue should avoid the restart of the loopback software (be it simple alsaloop or complex CamillaDSP). Nevertheless IMO the device will need to be re-opened anyway because hw params were changed. The gadget should facilitate this goal, ideally.
Maybe a reasonable way would really be to offer the alsa ctls with required params and let user decide if the conversions will be provided by the plug plugin or by his client internally. E.g. a client natively capable of 48k-multiples could accept these rates directly and let the permanently-inserted plug do automatic rate conversion for the other rates, no change in client configuration needed -> no client restart.
The question is (if this path was chosen) whether all of the controls should notify (like they do in aloop), or only the rate one (which always gets changed at any format change as the gadget must go through set_active(false), IMO. I do not know what overhead the extra notifications bring but every reduction counts :-)
Are capture and playback alt modes dependent? I thought they were separate configurations but I may be wrong.
If they are separate, perhaps p_alt.X and c_alt.X dirs would make sense instead, with using non-prefixed properties within them (ssize, ch_mask, etc.). I.e. p/c_xxx on the main level (setting the defaults and default alt1 for each direction) and non-prefixed properties in the actual p/c_alt.X subdirs.
They are indeed separate. I like this idea. I will separate these into c_alt.x and p_alt.x which can be created separately.
Again - hats off to your fantastic effort.
Thanks a lot,
Pavel.
participants (4)
-
Chris Wulff -
Chris Wulff -
Pavel Hofman -
Takashi Iwai