Re: alsa-lib's new API issue (snd_ctl_elem_id_compare)

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

On Thu, Mar 11, 2021 at 02:22:45PM +0100, Jaroslav Kysela wrote:
> > 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?

I've already investigated the idea you describe, however I concluded
that it has more complexity than convenience.

For example, the prototype would be:

```
int new_api(const snd_ctl_elem_id_t *l, const snd_ctl_elem_id_t *r,
            int (*algorithm)(const snd_ctl_elem_id_t *,
                             const snd_ctl_elem_id_t *));
```

For usage with qsort_r(3), programmer should do:

```
int my_algo(const snd_ctl_elem_id_t *l, snd_ctl_elem_id_t *r)
{
   ...
}

qsort_r(base, nmemb, size, new_api, my_algo);
```

On the other hand, the API has name to express itself appropriately and
we have some of such APIs:

```
int the_api_by_algo_a(const snd_ctl_elem_id_t *l,
                      const snd_ctl_elem_id_t *r);
int the_api_by_algo_b(const snd_ctl_elem_id_t *l,
                      const snd_ctl_elem_id_t *r);
int the_api_by_algo_c(const snd_ctl_elem_id_t *l,
                      const snd_ctl_elem_id_t *r);
...
```

The programmer selects one of them, then:

```
qsort(base, nmemb, size, the_api_by_algo_a);
```

Or select one of them dynamically if need:

```
int (*algo)(const snd_ctl_elem_id_t *, const snd_ctl_elem_id_t *);

switch (cond) {
case A:
    algo = the_api_by_algo_a;
    break;
case B:
    algo = the_api_by_algo_b;
    break;
case C:
    algo = the_api_by_algo_c;
    break;
default:
    return -EINVAL;
}

qsort(base, nmemb, size, algo);
```

For the case of hctl/mixer container about which you mentioned,
qsort_r(3) style is convenient for the case that programmer need to
re-implement own comparison algorithm. However the decision is still
up to the programmer, in short:

```
int my_algo(const snd_ctl_elem_id_t *l,
            const snd_ctl_elem_id_t *r,
            void *arg);
qsort_r(base, nmemb, size, my_algo, my_arg);
```

Here, I think it more worth to share algorithms than keeping less entries
in symbol table in shared library. Just the thought of it, I can devise
some algorithms below:

 * by numid
 * by name arithmetic (=your implementation)
 * by the words 'playback' and 'capture', case-insensitive or sensitive
 * by device and subdevice


Regards

Takashi Sakamoto