alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
From: Takashi Sakamoto <o-takashi@sakamocchi.jp>
To: perex@perex.cz
Cc: alsa-devel@alsa-project.org
Subject: alsa-lib's new API issue (snd_ctl_elem_id_compare)
Date: Mon, 8 Mar 2021 21:58:17 +0900	[thread overview]
Message-ID: <20210308125817.GA212288@workstation> (raw)

Hi Jaroslav,

I have some concern about your commit 2cfe6addaef6[1] for alsa-lib,

It adds new library API, 'snd_ctl_elem_id_compare()', to compare a pair of
IDs for control element. In the implementation, the API call returns 0 if
they are the same, else negative or positive numeric value according to
contents of the IDs.

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.

2. tri-state return value is semantically inconsistent

The ID structure includes three types of values; integer, enumeration, and
string. In my opinion, tri-state return value from them has different meaning
each other. It's better just to return comparison result by boolean value,
in my opinion.

The reason I post this message instead of posting any fix is that the fix
to the API affects to alsa-utils, in which the API is used by a patch you
applied a few days ago[2]. The patch also includes change to 'AM_PATH_ALSA'
declaration in configure.ac with bumped-up version to '1.2.5', and it
disables to rebuild alsa-utils on the latest toolchain. (alsa-lib 1.2.5 is
not released yet.)

I'd like to drop the usage of API from alsa-utils with equivalent
alternative codes in alsa-utils side at first, apart from the new API in
alsa-lib so that alsa-utils can be still build on the latest toolchains to
avoid any confusion in user side. Then I'd fix the issued API carefully
so that it can be used so long as exposed API without any issue.

[1] https://github.com/alsa-project/alsa-lib/commit/2cfe6addaef6a0afa72699ec07a45e7f2fa445ba
[2] https://github.com/alsa-project/alsa-utils/commit/17b4129e6c89d1a96d4d86dabea38389927e3cf4


Regards

Takashi Sakamoto

             reply	other threads:[~2021-03-08 12:59 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-08 12:58 Takashi Sakamoto [this message]
2021-03-08 14:33 ` alsa-lib's new API issue (snd_ctl_elem_id_compare) Jaroslav Kysela
2021-03-09  0:38   ` Takashi Sakamoto
2021-03-09 12:37     ` Jaroslav Kysela
2021-03-11 12:46       ` Takashi Sakamoto
2021-03-11 13:22         ` Jaroslav Kysela
2021-03-12  1:35           ` Takashi Sakamoto
2021-03-12 10:09             ` Jaroslav Kysela
2021-03-14  1:41               ` Takashi Sakamoto

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210308125817.GA212288@workstation \
    --to=o-takashi@sakamocchi.jp \
    --cc=alsa-devel@alsa-project.org \
    --cc=perex@perex.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).