Re: [PATCH] ALSA: control: remove limitation on the number of user-defined control element set per card

[Takashi Sakamoto <o-takashi@sakamocchi.jp>]

Hi,

On Wed, Jan 20, 2021 at 10:38:36AM +0100, Jaroslav Kysela wrote:
> Dne 20. 01. 21 v 10:27 Takashi Iwai napsal(a):
> > On Wed, 20 Jan 2021 09:55:41 +0100,
> > Takashi Sakamoto wrote:
> >>
> >> ALSA control core allows usespace application to register control element
> >> set by call of ioctl(2) with SNDRV_CTL_IOCTL_ELEM_ADD request. The added
> >> control element set is called as 'user-defined'. Currently sound card has
> >> limitation on the number of the user-defined control element set up
> >> to 32.
> >>
> >> The limitation is inconvenient to drivers in ALSA firewire stack since
> >> the drivers expect userspace applications to implement function to
> >> control device functionalities such as mixing and routing. As the
> >> userspace application, snd-firewire-ctl-services project starts:
> >> https://github.com/alsa-project/snd-firewire-ctl-services/
> >>
> >> The project supports many devices supported by ALSA firewire stack. The
> >> limitation is mostly good since routing and mixing controls can be
> >> represented by control element set, which includes control element with
> >> the same parameters. Nevertheless, it's actually inconvenient to device
> >> which has many varied functionalities. For example, plugin effect such as
> >> channel strip and reverb has many parameters. For the case, many control
> >> elements are required to configure the parameters and control element set
> >> cannot aggregates controls for the parameters. At present, below models
> >> are implemented with the control elements and actually add control element
> >> sets over 32:
> >>
> >>  * Apogee Emsemble (snd-bebob-ctl-service)
> >>  * TC Electronic Konnekt 24d (snd-dice-ctl-service)
> >>  * TC Electronic Studio Konnekt 48 (snd-dice-ctl-service)
> >>  * TC Electronic Konnekt Live (snd-dice-ctl-service)
> >>  * TC Electronic Impact Twin (snd-dice-ctl-service)
> >>
> >> It could be investigated to increase the number; e.g. quadruple to the
> >> current (=128), however it's hard to find criteria about the number for
> >> existent sound card. This commit just removes the limitation for the
> >> reason. ALSA control core uses UINT_MAX as the maximum number of control
> >> elements added to a sound card. It's limitation for both in-kernel driver
> >> and userspace application.
> >>
> >> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> > 
> > I'm still worried by the unlimited number of possible additions.
> > Did you check what would happen if you run a test program to add
> > user-space ctls (with the max count) in a loop repeatedly?  If that
> > doesn't blow up too much, it might be OK.  Otherwise we have to add
> > some practical limits.
> > 
> > So, let's prove that it's absolutely safe to release the limit at
> > first.
> 
> I agree. The UINT_MAX limit is really big in my eyes. The condition was added
> to check for the insane allocations. I basically agree to increase this limit
> (512, 1024?), but it should not be UINT_MAX.

Thanks for your comments. I have no objection to relax current limitation
instead of removal.

Either 512 or 1024 (of course 32), it just comes from multiples of 2 and
seems not to be reasonable for the practical limits. However, as I
note in the commit message, I cannot find no criteria for the limitation.
In this time, 40-50 control element sets are enough for the service program,
thus I'll post v2 patch with 128 as limitation enough large to the
requirement.


Thanks

Takashi Sakamoto