[alsa-devel] [alsa-lib][PATCH 0/2] ctl: deprecate APIs of dimension information
Hi,
In ALSA control interface of asound.h, 'struct snd_ctl_elem_info' has 'dimen' member to deliver information for multi-dimensional array, however there's no common way to handle the member. As a result, drivers can force userspace applications to handle the information by inconsistent ways. A series of echoaudio drivers is an actual example of the inconsistent usage.
This issue was addressed in a commit 51db452df07b ('Revert "ALSA: echoaudio: purge contradictions between dimension matrix members and total number of members"') to Linux kernel[1]. Fortunately, at present, this feature of interface is just used by the drivers, and there's a way to obsolete usage of the feature. We can obsolete it without large impacts to userland.
As a result of discussion at Linux miniconference 2017, usage of 'dimen' member of 'struct snd_ctl_elem_info' is going to be deprecated for future removal. This patchset is for proposed tasks at a development period of Linux kernel v4.15[3]. Some APIs of dimension information are deprecated.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/so...
[2] [alsa-devel] [ANNOUNCE] Audio Mini Summit 2017 at Prague, Oct 27 http://mailman.alsa-project.org/pipermail/alsa-devel/2017-July/123110.html
[3] https://github.com/takaswie/presentations/blob/master/20171027/contents.md
Takashi Sakamoto (2): ctl: deprecate APIs of dimension information test: obsolete usage of APIs of dimension information
src/control/control.c | 12 ++++++++++++ test/user-ctl-element-set.c | 31 ------------------------------- 2 files changed, 12 insertions(+), 31 deletions(-)
In ALSA control interface of asound.h, 'struct snd_ctl_elem_info' has 'dimen' member to deliver information for multi-dimensional array, however there's no common way to handle the member. As a result, drivers can force userspace applications to handle the information by inconsistent ways.
This issue was reported by a commit 51db452df07b ('Revert "ALSA: echoaudio: purge contradictions between dimension matrix members and total number of members"') to Linux kernel. As a result of discussion at Linux miniconference 2017, usage of 'dimen' member of 'struct snd_ctl_elem_info' is going to be deprecated for future removal.
This commit deprecates some APIs related to the dimension information. They are planned to be removed in a development period for Linux kernel v4.21.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- src/control/control.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/src/control/control.c b/src/control/control.c index 6439b294..88828f54 100644 --- a/src/control/control.c +++ b/src/control/control.c @@ -2503,6 +2503,10 @@ const char *snd_ctl_elem_info_get_item_name(const snd_ctl_elem_info_t *obj) * \brief Get count of dimensions for given element * \param obj CTL element id/info * \return zero value if no dimensions are defined, otherwise positive value with count of dimensions + * + * \deprecated Since 1.1.5 + * #snd_ctl_elem_info_get_dimensions is deprecated without any replacement, + * under a decision to drop corresponding kernel ABI at Linux v4.21. */ #ifndef DOXYGEN int INTERNAL(snd_ctl_elem_info_get_dimensions)(const snd_ctl_elem_info_t *obj) @@ -2525,6 +2529,10 @@ use_default_symbol_version(__snd_ctl_elem_info_get_dimensions, snd_ctl_elem_info * \param obj CTL element id/info * \param idx The dimension index * \return zero value if no dimension width is defined, otherwise positive value with with of specified dimension + * + * \deprecated Since 1.1.5 + * #snd_ctl_elem_info_get_dimension is deprecated without any replacement, + * under a decision to drop corresponding kernel ABI at Linux v4.21. */ #ifndef DOXYGEN int INTERNAL(snd_ctl_elem_info_get_dimension)(const snd_ctl_elem_info_t *obj, unsigned int idx) @@ -2553,6 +2561,10 @@ use_default_symbol_version(__snd_ctl_elem_info_get_dimension, snd_ctl_elem_info_ * * \par Compatibility: * This function is added in version 1.1.2. + * + * \deprecated Since 1.1.5 + * #snd_ctl_elem_info_set_dimension is deprecated without any replacement, + * under a decision to drop corresponding kernel ABI at Linux v4.21. */ int snd_ctl_elem_info_set_dimension(snd_ctl_elem_info_t *info, const int dimension[4])
On Sat, 04 Nov 2017 03:24:25 +0100, Takashi Sakamoto wrote:
In ALSA control interface of asound.h, 'struct snd_ctl_elem_info' has 'dimen' member to deliver information for multi-dimensional array, however there's no common way to handle the member. As a result, drivers can force userspace applications to handle the information by inconsistent ways.
Well, the text is slightly misleading. Actually the API itself wasn't bad and you can implement it consistently. But practically seen, we haven't got *any* driver using the feature in the correct way for over decades. The only user was echoaudio drivers but it was obviously wrong. So, judging from the situation, keeping this doesn't make much sense -- that's the reason for dropping.
This issue was reported by a commit 51db452df07b ('Revert "ALSA: echoaudio: purge contradictions between dimension matrix members and total number of members"') to Linux kernel. As a result of discussion at Linux miniconference 2017, usage of 'dimen' member of 'struct snd_ctl_elem_info' is going to be deprecated for future removal.
This commit deprecates some APIs related to the dimension information. They are planned to be removed in a development period for Linux kernel v4.21.
IMO, the version to deprecate the feature may depend on LTS kernel version. 4.21 looks OK, but we may shift it.
thanks,
Takashi
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp
src/control/control.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/src/control/control.c b/src/control/control.c index 6439b294..88828f54 100644 --- a/src/control/control.c +++ b/src/control/control.c @@ -2503,6 +2503,10 @@ const char *snd_ctl_elem_info_get_item_name(const snd_ctl_elem_info_t *obj)
- \brief Get count of dimensions for given element
- \param obj CTL element id/info
- \return zero value if no dimensions are defined, otherwise positive value with count of dimensions
- \deprecated Since 1.1.5
- #snd_ctl_elem_info_get_dimensions is deprecated without any replacement,
*/
- under a decision to drop corresponding kernel ABI at Linux v4.21.
#ifndef DOXYGEN int INTERNAL(snd_ctl_elem_info_get_dimensions)(const snd_ctl_elem_info_t *obj) @@ -2525,6 +2529,10 @@ use_default_symbol_version(__snd_ctl_elem_info_get_dimensions, snd_ctl_elem_info
- \param obj CTL element id/info
- \param idx The dimension index
- \return zero value if no dimension width is defined, otherwise positive value with with of specified dimension
- \deprecated Since 1.1.5
- #snd_ctl_elem_info_get_dimension is deprecated without any replacement,
*/
- under a decision to drop corresponding kernel ABI at Linux v4.21.
#ifndef DOXYGEN int INTERNAL(snd_ctl_elem_info_get_dimension)(const snd_ctl_elem_info_t *obj, unsigned int idx) @@ -2553,6 +2561,10 @@ use_default_symbol_version(__snd_ctl_elem_info_get_dimension, snd_ctl_elem_info_
- \par Compatibility:
- This function is added in version 1.1.2.
- \deprecated Since 1.1.5
- #snd_ctl_elem_info_set_dimension is deprecated without any replacement,
*/
- under a decision to drop corresponding kernel ABI at Linux v4.21.
int snd_ctl_elem_info_set_dimension(snd_ctl_elem_info_t *info, const int dimension[4]) -- 2.11.0
On Nov 05 2017 19:03, Takashi Iwai wrote:
On Sat, 04 Nov 2017 03:24:25 +0100, Takashi Sakamoto wrote:
In ALSA control interface of asound.h, 'struct snd_ctl_elem_info' has 'dimen' member to deliver information for multi-dimensional array, however there's no common way to handle the member. As a result, drivers can force userspace applications to handle the information by inconsistent ways.
Well, the text is slightly misleading. Actually the API itself wasn't bad and you can implement it consistently.
Hm.
I can assume that a series of echoaudio driver attempts to deliver information about elements in the same element via the 'dimen' member. This usage might not be so wrong.
The 'dimen' member is supplemental information. It has an effect to the way to parse any array relevant to control elements. Current ALSA control interface includes two types of the array; value array and element array in the same element set. This is a cause of this issue. The double nature of meaning of the member brings inconsistent handling of these arrays according to the value of 'dimen' member. Thus this is an issue of the interface.
If we had any document about usage of the member (e.g. just a few more words in asound.h), we would avoid this issue. We've failed to lead developers to a consistent usage of the interface.
But practically seen, we haven't got *any* driver using the feature in the correct way for over decades. The only user was echoaudio drivers but it was obviously wrong. So, judging from the situation, keeping this doesn't make much sense -- that's the reason for dropping.
No objections for the above sentence just for our judgment of the removal.
This issue was reported by a commit 51db452df07b ('Revert "ALSA: echoaudio: purge contradictions between dimension matrix members and total number of members"') to Linux kernel. As a result of discussion at Linux miniconference 2017, usage of 'dimen' member of 'struct snd_ctl_elem_info' is going to be deprecated for future removal.
This commit deprecates some APIs related to the dimension information. They are planned to be removed in a development period for Linux kernel v4.21.
IMO, the version to deprecate the feature may depend on LTS kernel version. 4.21 looks OK, but we may shift it.
I still get no merit of your suggestion. Do you mean that the removal of ABI should be done in several releases before target LTS will be actually released, to produce stable ABI? If so, we need to estimate the timing of next LTS. However, it's hard because for recent LTS we got no official announcement from stable maintainers till LTS version of Linux kernel is actually released. At least, I won't work with such uncertain estimations.
Regards
Takashi Sakamoto
On Tue, 07 Nov 2017 01:34:02 +0100, Takashi Sakamoto wrote:
This commit deprecates some APIs related to the dimension information. They are planned to be removed in a development period for Linux kernel v4.21.
IMO, the version to deprecate the feature may depend on LTS kernel version. 4.21 looks OK, but we may shift it.
I still get no merit of your suggestion. Do you mean that the removal of ABI should be done in several releases before target LTS will be actually released, to produce stable ABI? If so, we need to estimate the timing of next LTS. However, it's hard because for recent LTS we got no official announcement from stable maintainers till LTS version of Linux kernel is actually released. At least, I won't work with such uncertain estimations.
The feature removal isn't what we can guarantee at a certain kernel version. As long as we have a real (not theoretical) user, it shouldn't be dropped. It's still unwritten "no regression" rule.
That said, we may announce and prepare / plan the deprecation, but we can't define the exact kernel version in general. And considering LTS release is one of the decision factors.
Takashi
On Nov 7 2017 16:59, Takashi Iwai wrote:
On Tue, 07 Nov 2017 01:34:02 +0100, Takashi Sakamoto wrote:
This commit deprecates some APIs related to the dimension information. They are planned to be removed in a development period for Linux kernel v4.21.
IMO, the version to deprecate the feature may depend on LTS kernel version. 4.21 looks OK, but we may shift it.
I still get no merit of your suggestion. Do you mean that the removal of ABI should be done in several releases before target LTS will be actually released, to produce stable ABI? If so, we need to estimate the timing of next LTS. However, it's hard because for recent LTS we got no official announcement from stable maintainers till LTS version of Linux kernel is actually released. At least, I won't work with such uncertain estimations.
The feature removal isn't what we can guarantee at a certain kernel version. As long as we have a real (not theoretical) user, it shouldn't be dropped. It's still unwritten "no regression" rule.
That said, we may announce and prepare / plan the deprecation, but we can't define the exact kernel version in general. And considering LTS release is one of the decision factors.
If it comes from a general theory, I'm willing to follow it.
But in this case, we can be optimistic. As you know, the minor feature is just used in drivers/software related to models produced by Echo Digital Audio corp. The other probability is usage by user-defined control element set on any userspace application. As my recent work declared, this feature has been long-abandoned and no application exists (if such application exists, developers have already found such bugs which I've fixed recently before I worked...).
If the feature were used by majority, it would be more difficult to deprecate/remove it, and we need to be deliberate. Depending on cases, deprecation is impossible itself. But this feature is quite minor and got little popularity. Practically, we can safely remove the minor feature in any timing after modified version of echoaudio stuffs are disseminated. My proposed schedule takes 6 kernel releases for it. In the schedule, v4.21 release is the deadline of the feature, but if you'd like to keep the deadline undecided for such theory/rule, I have no objection to it. To me, deprecation of this feature for future removal is more important to avoid developers' misuses to which I addressed, than actual removal.
[1] https://github.com/takaswie/presentations/blob/master/20171027/contents.md
Regards
Takashi Sakamoto
On Tue, 07 Nov 2017 23:36:28 +0100, Takashi Sakamoto wrote:
On Nov 7 2017 16:59, Takashi Iwai wrote:
On Tue, 07 Nov 2017 01:34:02 +0100, Takashi Sakamoto wrote:
This commit deprecates some APIs related to the dimension information. They are planned to be removed in a development period for Linux kernel v4.21.
IMO, the version to deprecate the feature may depend on LTS kernel version. 4.21 looks OK, but we may shift it.
I still get no merit of your suggestion. Do you mean that the removal of ABI should be done in several releases before target LTS will be actually released, to produce stable ABI? If so, we need to estimate the timing of next LTS. However, it's hard because for recent LTS we got no official announcement from stable maintainers till LTS version of Linux kernel is actually released. At least, I won't work with such uncertain estimations.
The feature removal isn't what we can guarantee at a certain kernel version. As long as we have a real (not theoretical) user, it shouldn't be dropped. It's still unwritten "no regression" rule.
That said, we may announce and prepare / plan the deprecation, but we can't define the exact kernel version in general. And considering LTS release is one of the decision factors.
If it comes from a general theory, I'm willing to follow it.
But in this case, we can be optimistic. As you know, the minor feature is just used in drivers/software related to models produced by Echo Digital Audio corp. The other probability is usage by user-defined control element set on any userspace application. As my recent work declared, this feature has been long-abandoned and no application exists (if such application exists, developers have already found such bugs which I've fixed recently before I worked...).
If the feature were used by majority, it would be more difficult to deprecate/remove it, and we need to be deliberate. Depending on cases, deprecation is impossible itself. But this feature is quite minor and got little popularity. Practically, we can safely remove the minor feature in any timing after modified version of echoaudio stuffs are disseminated. My proposed schedule takes 6 kernel releases for it. In the schedule, v4.21 release is the deadline of the feature, but if you'd like to keep the deadline undecided for such theory/rule, I have no objection to it. To me, deprecation of this feature for future removal is more important to avoid developers' misuses to which I addressed, than actual removal.
I have no objection of removal of API in alsa-lib, but my concern is that unnecessary announcement of the exact kernel version as if it's an ultimately fixed deadline. It's rather a "hopefully working" deadline, and we don't need to show it in alsa-lib side.
We can update the comment once after it's really deprecated, if any.
thanks,
Takashi
On Nov 8 2017 16:05, Takashi Iwai wrote:
On Tue, 07 Nov 2017 23:36:28 +0100, Takashi Sakamoto wrote:
On Nov 7 2017 16:59, Takashi Iwai wrote:
On Tue, 07 Nov 2017 01:34:02 +0100, Takashi Sakamoto wrote:
This commit deprecates some APIs related to the dimension information. They are planned to be removed in a development period for Linux kernel v4.21.
IMO, the version to deprecate the feature may depend on LTS kernel version. 4.21 looks OK, but we may shift it.
I still get no merit of your suggestion. Do you mean that the removal of ABI should be done in several releases before target LTS will be actually released, to produce stable ABI? If so, we need to estimate the timing of next LTS. However, it's hard because for recent LTS we got no official announcement from stable maintainers till LTS version of Linux kernel is actually released. At least, I won't work with such uncertain estimations.
The feature removal isn't what we can guarantee at a certain kernel version. As long as we have a real (not theoretical) user, it shouldn't be dropped. It's still unwritten "no regression" rule.
That said, we may announce and prepare / plan the deprecation, but we can't define the exact kernel version in general. And considering LTS release is one of the decision factors.
If it comes from a general theory, I'm willing to follow it.
But in this case, we can be optimistic. As you know, the minor feature is just used in drivers/software related to models produced by Echo Digital Audio corp. The other probability is usage by user-defined control element set on any userspace application. As my recent work declared, this feature has been long-abandoned and no application exists (if such application exists, developers have already found such bugs which I've fixed recently before I worked...).
If the feature were used by majority, it would be more difficult to deprecate/remove it, and we need to be deliberate. Depending on cases, deprecation is impossible itself. But this feature is quite minor and got little popularity. Practically, we can safely remove the minor feature in any timing after modified version of echoaudio stuffs are disseminated. My proposed schedule takes 6 kernel releases for it. In the schedule, v4.21 release is the deadline of the feature, but if you'd like to keep the deadline undecided for such theory/rule, I have no objection to it. To me, deprecation of this feature for future removal is more important to avoid developers' misuses to which I addressed, than actual removal.
I have no objection of removal of API in alsa-lib, but my concern is that unnecessary announcement of the exact kernel version as if it's an ultimately fixed deadline. It's rather a "hopefully working" deadline, and we don't need to show it in alsa-lib side.
Now I understand your concern.
We can update the comment once after it's really deprecated, if any.
I'm OK for this suggestion. Later, I'll post updated version of this patchset.
Thanks
Takashi Sakamoto
APIs of dimension information are deprecated for future removal. This commit removes test codes for user-defined element set in an aspect of the feature.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- test/user-ctl-element-set.c | 31 ------------------------------- 1 file changed, 31 deletions(-)
diff --git a/test/user-ctl-element-set.c b/test/user-ctl-element-set.c index f3710732..668831a8 100644 --- a/test/user-ctl-element-set.c +++ b/test/user-ctl-element-set.c @@ -19,7 +19,6 @@ struct elem_set_trial { unsigned int element_count;
snd_ctl_elem_id_t *id; - int dimension[4];
int (*add_elem_set)(struct elem_set_trial *trial, snd_ctl_elem_info_t *info); @@ -373,7 +372,6 @@ static int add_elem_set(struct elem_set_trial *trial) snd_ctl_elem_info_alloca(&info); snd_ctl_elem_info_set_interface(info, SND_CTL_ELEM_IFACE_MIXER); snd_ctl_elem_info_set_name(info, name); - snd_ctl_elem_info_set_dimension(info, trial->dimension);
err = trial->add_elem_set(trial, info); if (err >= 0) @@ -544,11 +542,6 @@ static int check_elem_set_props(struct elem_set_trial *trial) return -EIO; if (snd_ctl_elem_info_get_count(info) != trial->member_count) return -EIO; - for (j = 0; j < 4; ++j) { - if (snd_ctl_elem_info_get_dimension(info, j) != - trial->dimension[j]) - return -EIO; - }
/* * In a case of IPC, this is the others. But in this case, @@ -745,10 +738,6 @@ int main(void) case SND_CTL_ELEM_TYPE_BOOLEAN: trial.element_count = 900; trial.member_count = 128; - trial.dimension[0] = 4; - trial.dimension[1] = 4; - trial.dimension[2] = 8; - trial.dimension[3] = 0; trial.add_elem_set = add_bool_elem_set; trial.check_elem_props = NULL; trial.change_elem_members = change_bool_elem_members; @@ -759,10 +748,6 @@ int main(void) case SND_CTL_ELEM_TYPE_INTEGER: trial.element_count = 900; trial.member_count = 128; - trial.dimension[0] = 128; - trial.dimension[1] = 0; - trial.dimension[2] = 0; - trial.dimension[3] = 0; trial.add_elem_set = add_int_elem_set; trial.check_elem_props = check_int_elem_props; trial.change_elem_members = change_int_elem_members; @@ -773,10 +758,6 @@ int main(void) case SND_CTL_ELEM_TYPE_ENUMERATED: trial.element_count = 900; trial.member_count = 128; - trial.dimension[0] = 16; - trial.dimension[1] = 8; - trial.dimension[2] = 0; - trial.dimension[3] = 0; trial.add_elem_set = add_enum_elem_set; trial.check_elem_props = check_enum_elem_props; trial.change_elem_members = change_enum_elem_members; @@ -786,10 +767,6 @@ int main(void) case SND_CTL_ELEM_TYPE_BYTES: trial.element_count = 900; trial.member_count = 512; - trial.dimension[0] = 8; - trial.dimension[1] = 4; - trial.dimension[2] = 8; - trial.dimension[3] = 2; trial.add_elem_set = add_bytes_elem_set; trial.check_elem_props = NULL; trial.change_elem_members = change_bytes_elem_members; @@ -800,10 +777,6 @@ int main(void) case SND_CTL_ELEM_TYPE_IEC958: trial.element_count = 1; trial.member_count = 1; - trial.dimension[0] = 0; - trial.dimension[1] = 0; - trial.dimension[2] = 0; - trial.dimension[3] = 0; trial.add_elem_set = add_iec958_elem_set; trial.check_elem_props = NULL; trial.change_elem_members = change_iec958_elem_members; @@ -814,10 +787,6 @@ int main(void) default: trial.element_count = 900; trial.member_count = 64; - trial.dimension[0] = 0; - trial.dimension[1] = 0; - trial.dimension[2] = 0; - trial.dimension[3] = 0; trial.add_elem_set = add_int64_elem_set; trial.check_elem_props = check_int64_elem_props; trial.change_elem_members = change_int64_elem_members;
participants (2)
-
Takashi Iwai -
Takashi Sakamoto