alsa-lib's new API issue (snd_ctl_elem_id_compare)

[Jaroslav Kysela]

Dne 11. 03. 21 v 13:46 Takashi Sakamoto napsal(a):
> Hi,
> 
> Sorry to be late for reply but I have a bit busy for patchset to test
> programs of axfer in alsa-utils[1].
> 
> On Tue, Mar 09, 2021 at 01:37:26PM +0100, Jaroslav Kysela wrote:
>> Dne 09. 03. 21 v 1:38 Takashi Sakamoto napsal(a):
>>> On Mon, Mar 08, 2021 at 03:33:46PM +0100, Jaroslav Kysela wrote:
>>>> Dne 08. 03. 21 v 13:58 Takashi Sakamoto napsal(a):
>>>>> My concerns are:
>>>>>
>>>>> 1. evaluation of numid field is not covered.
>>>>>
>>>>> This is not preferable since ALSA control core implementation covers two
>>>>> types of comparison; numid only, and the combination iface, device,
>>>>> subdevice, name, and index fields. If the API is produced for general use
>>>>> cases, it should correctly handle the numid-only case, in my opinion.
>>>>
>>>> My motivation was to allow to use this function for qsort() for example. The
>>>> numid and full-field comparisons are two very different things.
>>>  
>>> Yep. I easily associated sort implementation in hcontrol API or simple
>>> mixer API from your implementation
>>>
>>> However, the new API is a part of control API and it just achieves things without
>>> any supplemental information given from userspace implementation.
>>
>> It's not required, if documented. Nobody forces to use this function in the
>> app code.
>>
>>> For the above comparison API, as I described, it's not appropriate. ID
>>> structure for control element is not comparable, thus it should be dropped
>>> or replaced with equality function such as 'snd_ctl_elem_id_equal()'.
>>
>> I don't require the numid match at this point. The numid is not known or may
>> change for the id entered by the user. So I need to forcefully ignore it.
>>
>> If we need a function which follow numid + full id comparison, then we can
>> introduce it. I agree that it should return only a boolean return value in
>> this case.
>>
>>> When you need any sorting algorithms, it should be implemented in
>>> application side or alsa-lib API in the other layer such as hcontrol and
>>> simple mixer since control API should follow to specification of ALSA
>>> control written in kernel land.
>>
>> I don't follow your arguments here. The numid and full field comparisons are
>> two different things. The caller must know things behind the scene.
>> The snd_ctl_elem_id_compare() function may be used as a quick backend for the
>> full field comparisons with the optimized execution (reduce app -> library calls).
>>
>> The enums conversion to integers: I think that we're safe here. The interface
>> enum numbers cannot change and we know the range (and the order), so it's safe.
>>
>> Lastly, the qsort() with snd_ctl_id_compare() as an argument will produce a
>> consistent, understandable result, right?
> 
> Hm. I believe that you agree with the fact that we can make various
> algorithms to compare a pair of IDs for control elements. When focusing
> on fields except for numid, we can make the other algorithms against your
> implementation, since the ID structure is compound one. Each of the
> algorithms can return different result.
> 
> Here, I'd like to shift the discussion to the name of new API. Although it
> has the most common name, 'snd_ctl_id_compare', it just has one of
> comparison algorithms. I have a concern that the name can gives wrong idea
> to users that the ID structure for control element had design to be able to
> be compared by itself and it would just be a single comparison algorithm.
> 
> I suggest to rename the new API to express that it implements one of
> comparison algorithm. In a case of numid comparison, it would be
> 'snd_ctl_id_compare_by_numid()'. For your case,
> 'snd_ctl_id_compare_by_name_arithmetic' or something suitable.

Perhaps, we can add a third argument defining the sorting algorithm, so we
don't bloat the symbol tables so much when we add a new sorting type (enum).
It would mean that the function cannot be used as a direct argument to
qsort(), but I think that the apps add usually an extra code to own callback
depending on containers, anyway. Is it more appropriate for you?

						Jaroslav

-- 
Jaroslav Kysela <perex at perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.