usb:gadget:f_uac2: RFC: allowing multiple altsetttings for channel/samplesize combinations

usb:gadget:f_uac2: RFC: allowing multiple altsetttings for channel/samplesize combinations

[Pavel Hofman]

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.

Re: usb:gadget:f_uac2: RFC: allowing multiple altsetttings for channel/samplesize combinations

[Pavel Hofman]

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.com/T/

Maybe Takashi's commits to the midi gadget could be a way
https://patchwork.kernel.org/project/alsa-devel/list/?series=769151&state=%2A&archive=both
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.

Re: usb:gadget:f_uac2: RFC: allowing multiple altsetttings for channel/samplesize combinations

[Takashi Iwai]

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.com/T/
> 
> Maybe Takashi's commits to the midi gadget could be a way
> https://patchwork.kernel.org/project/alsa-devel/list/?series=769151&state=%2A&archive=both
> 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

Re: usb:gadget:f_uac2: RFC: allowing multiple altsetttings for channel/samplesize combinations

[Pavel Hofman]

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.com/T/
>>
>> Maybe Takashi's commits to the midi gadget could be a way
>> https://patchwork.kernel.org/project/alsa-devel/list/?series=769151&state=%2A&archive=both
>> 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.

Re: usb:gadget:f_uac2: RFC: allowing multiple altsetttings for channel/samplesize combinations

[Chris Wulff]

Pavel,

  Following up on this and discussions in this patch series

https://lore.kernel.org/linux-usb/CO1PR17MB54195BE778868AFDFE2DCB36E1112@CO1PR17MB5419.namprd17.prod.outlook.com/

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.com/T/
> >>
> >> Maybe Takashi's commits to the midi gadget could be a way
> >> https://patchwork.kernel.org/project/alsa-devel/list/?series=769151&state=%2A&archive=both
> >> 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.
>
>

Re: usb:gadget:f_uac2: RFC: allowing multiple altsetttings for channel/samplesize combinations

[Chris Wulff]

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@CO1PR17MB5419.namprd17.prod.outlook.com/

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-40b8-c16e34cea3e2@ivitera.com/T/__;!!HBnMciuwfVSXJQ!TBgtW5Lkn4FBTBWxYDZ-3I_EoVR-Q8mwtyeY_PWIGOmfKHW2oW76gMhOWF5l8Uo9yIDPJFhgVI2ZUthXfA$
> >>
> >> Maybe Takashi's commits to the midi gadget could be a way
> >> https://urldefense.com/v3/__https://patchwork.kernel.org/project/alsa-devel/list/?series=769151&state=*2A&archive=both__;JQ!!HBnMciuwfVSXJQ!TBgtW5Lkn4FBTBWxYDZ-3I_EoVR-Q8mwtyeY_PWIGOmfKHW2oW76gMhOWF5l8Uo9yIDPJFhgVI1DoWiKXQ$
> >> 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.
>
>

Re: usb:gadget:f_uac2: RFC: allowing multiple altsetttings for channel/samplesize combinations

[Pavel Hofman]

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@CO1PR17MB5419.namprd17.prod.outlook.com/
> 
> 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-audio-drivers#class-requests-and-interrupt-data-messages
. 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=%2A&archive=both
. 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.

Re: usb:gadget:f_uac2: RFC: allowing multiple altsetttings for channel/samplesize combinations

[Pavel Hofman]

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/0106679839f7c69632b3b9833c3268c316c0a9fc/sound/drivers/aloop.c#L1588-L1611


> 
> 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/0106679839f7c69632b3b9833c3268c316c0a9fc/sound/i2c/other/ak4114.c#L589-L597
. 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.