[alsa-devel] UCM extensions
Hi all,
I make some internal ucm code cleanups in alsa-lib and added three major extensions to allow more complex configurations which we require for the SOF kernel driver.
The first thing is the added substitution for the value strings:
https://github.com/alsa-project/alsa-lib/commit/f1e637b285e8e04e6761248a070f...
The second thing is the If block:
https://github.com/alsa-project/alsa-lib/commit/985715ce8148dc7ef62c8e3d8ce5...
The third thing is the card / hardware like specifier passed as the ucm name to snd_use_case_mgr_open() to support multiple card instances:
https://github.com/alsa-project/alsa-lib/commit/60164fc5886cdc6ca55eeed0c2e3...
All those patches (with other cleanups) are in the ucm2 branch on github for comments:
https://github.com/alsa-project/alsa-lib/commits/ucm2
The proposed SOF UCM config diff is here:
https://github.com/alsa-project/alsa-ucm-conf/commit/723b6da881721488229154e... https://github.com/alsa-project/alsa-ucm-conf/commits/ucm2
I added everything to keep the interface backward compatible, so the current applications should not observe any different behavior. The applications like pulseaudio should use the 'hw:CARD_INDEX' specifier for the open call in the future and snd_use_case_parse_ctl_elem_id() helper for the element control names.
If you have another ideas to address those issues, please, let me know.
BTW, Mark: The SOF UCM configs relies on the driver name changes, so it might be worth to send "ASoC: intel - fix the card names" patch to 5.4 to make things stable more quickly:
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/commit/?h=...
Thanks, Jaroslav
On 11/5/19 1:36 PM, Jaroslav Kysela wrote:
Hi all,
I make some internal ucm code cleanups in alsa-lib and added three major extensions to allow more complex configurations which we require for the SOF kernel driver.
Great, thanks for working on this!
The first thing is the added substitution for the value strings:
https://github.com/alsa-project/alsa-lib/commit/f1e637b285e8e04e6761248a070f...
The second thing is the If block:
https://github.com/alsa-project/alsa-lib/commit/985715ce8148dc7ef62c8e3d8ce5...
is it possible for the Condition to check if the card contains the SOF prefix?
For Baytrail/Cherrytrail support, we have a ton of existing UCM files, and the only thing needed is e.g. - to change from hw:bytcrrt5640 to hw:sofbytcrt5640 - make the controls for the legacy driver conditional
SectionVerb { EnableSequence [ if (card name does not contain SOF) <platforms/bytcr/PlatformEnableSeq.conf> endif <codecs/rt5640/EnableSeq.conf> ]
Most of the information that matters for those UCM files is the type of peripherals and the matching mixer configuration for the codec driver, and we should reuse this information between legacy and SOF.
The third thing is the card / hardware like specifier passed as the ucm name to snd_use_case_mgr_open() to support multiple card instances:
https://github.com/alsa-project/alsa-lib/commit/60164fc5886cdc6ca55eeed0c2e3...
All those patches (with other cleanups) are in the ucm2 branch on github for comments:
https://github.com/alsa-project/alsa-lib/commits/ucm2
The proposed SOF UCM config diff is here:
https://github.com/alsa-project/alsa-ucm-conf/commit/723b6da881721488229154e...
https://github.com/alsa-project/alsa-ucm-conf/commits/ucm2
I added everything to keep the interface backward compatible, so the current applications should not observe any different behavior. The applications like pulseaudio should use the 'hw:CARD_INDEX' specifier for the open call in the future and snd_use_case_parse_ctl_elem_id() helper for the element control names.
If you have another ideas to address those issues, please, let me know.
BTW, Mark: The SOF UCM configs relies on the driver name changes, so it might be worth to send "ASoC: intel - fix the card names" patch to 5.4 to make things stable more quickly:
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/commit/?h=...
Thanks, Jaroslav
Dne 05. 11. 19 v 20:52 Pierre-Louis Bossart napsal(a):
On 11/5/19 1:36 PM, Jaroslav Kysela wrote:
Hi all,
I make some internal ucm code cleanups in alsa-lib and added three major extensions to allow more complex configurations which we require for the SOF kernel driver.
Great, thanks for working on this!
The first thing is the added substitution for the value strings:
https://github.com/alsa-project/alsa-lib/commit/f1e637b285e8e04e6761248a070f...
The second thing is the If block:
https://github.com/alsa-project/alsa-lib/commit/985715ce8148dc7ef62c8e3d8ce5...
is it possible for the Condition to check if the card contains the SOF prefix?
For Baytrail/Cherrytrail support, we have a ton of existing UCM files, and the only thing needed is e.g.
- to change from hw:bytcrrt5640 to hw:sofbytcrt5640
It is easy with the proposed substitution, just use hw:${CardId} here. It can for any values (like PCM device names etc.).
- make the controls for the legacy driver conditional
SectionVerb { EnableSequence [ if (card name does not contain SOF) <platforms/bytcr/PlatformEnableSeq.conf> endif <codecs/rt5640/EnableSeq.conf> ]
The If blocks cannot be used in sequences, but you can do basically this with the proposed If extension:
SectionVerb { If.1 { Condition { Type StringEqual String1 "${CardName}" String2 "bytcrt5640" } True { EnableSequence [ <platforms/bytcr/PlatformEnableSeq.conf> <codecs/rt5640/EnableSeq.conf> ] } False { EnableSequence [ <codecs/rt5640/EnableSeq.conf> ] } } }
The condition with "Type StringEqual" is not implemented yet, but it's easy to add new conditions to my proposed code. If you see something else to be compared, just let me know to add those conditions in the first phase.
Most of the information that matters for those UCM files is the type of peripherals and the matching mixer configuration for the codec driver, and we should reuse this information between legacy and SOF.
Yes, I see the demand to make the description more universal and easy to read.
Note that cdev is already initialized from the ValueDefaults {} section PlaybackCTL/CaptureCTL values even in the old code, so the most of cdev declarations are not necessary in the current UCM configs too. When the use-case configuration is tied to the one card, I added the code to use it implicitly, so cdev can be really omitted. With the proposed code, the PlaybackCTL/CaptureCTL are always available for the one card configs:
https://github.com/alsa-project/alsa-lib/commit/c099329ce6ffdb26fc7a4a6ffb6f...
Jaroslav
On 11/5/19 2:19 PM, Jaroslav Kysela wrote:
Dne 05. 11. 19 v 20:52 Pierre-Louis Bossart napsal(a):
On 11/5/19 1:36 PM, Jaroslav Kysela wrote:
Hi all,
I make some internal ucm code cleanups in alsa-lib and added three major extensions to allow more complex configurations which we require for the SOF kernel driver.
Great, thanks for working on this!
The first thing is the added substitution for the value strings:
https://github.com/alsa-project/alsa-lib/commit/f1e637b285e8e04e6761248a070f...
The second thing is the If block:
https://github.com/alsa-project/alsa-lib/commit/985715ce8148dc7ef62c8e3d8ce5...
is it possible for the Condition to check if the card contains the SOF prefix?
For Baytrail/Cherrytrail support, we have a ton of existing UCM files, and the only thing needed is e.g.
- to change from hw:bytcrrt5640 to hw:sofbytcrt5640
It is easy with the proposed substitution, just use hw:${CardId} here. It can for any values (like PCM device names etc.).
thanks for confirming, I thought it'd be fine indeed.
- make the controls for the legacy driver conditional
SectionVerb { EnableSequence [
if (card name does not contain SOF) <platforms/bytcr/PlatformEnableSeq.conf> endif <codecs/rt5640/EnableSeq.conf> ]
The If blocks cannot be used in sequences, but you can do basically this with the proposed If extension:
SectionVerb { If.1 { Condition { Type StringEqual String1 "${CardName}" String2 "bytcrt5640" } True { EnableSequence [ <platforms/bytcr/PlatformEnableSeq.conf> <codecs/rt5640/EnableSeq.conf> ] } False { EnableSequence [ <codecs/rt5640/EnableSeq.conf> ] } } }
The condition with "Type StringEqual" is not implemented yet, but it's easy to add new conditions to my proposed code. If you see something else to be compared, just let me know to add those conditions in the first phase.
ok, that's great.
Maybe StringContains would help, so that we can use String2 "sof" in most cases.
Most of the information that matters for those UCM files is the type of peripherals and the matching mixer configuration for the codec driver, and we should reuse this information between legacy and SOF.
Yes, I see the demand to make the description more universal and easy to read.
Note that cdev is already initialized from the ValueDefaults {} section PlaybackCTL/CaptureCTL values even in the old code, so the most of cdev declarations are not necessary in the current UCM configs too. When the use-case configuration is tied to the one card, I added the code to use it implicitly, so cdev can be really omitted. With the proposed code, the PlaybackCTL/CaptureCTL are always available for the one card configs:
https://github.com/alsa-project/alsa-lib/commit/c099329ce6ffdb26fc7a4a6ffb6f...
Yes, that's a good change, it'll make it easier to manage configurations.
-Pierre
Hi Jaroslav,
On Tue, 5 Nov 2019, Jaroslav Kysela wrote:
I make some internal ucm code cleanups in alsa-lib and added three major extensions to allow more complex configurations which we require for the SOF kernel driver.
looks very good and pragmatic way to tackle some of the issues you hit with current UCM.
E.g. the If block would be also sufficient to tackle the recent HDMI codec driver change (with a single UCM file) -- i.e. use existence of the hdac-hdmi driver controls to select which enable-sequences to run. Hmm, I like this better than trying to select a whole different UCM file based on which drivers are used.
And same usage pattern can be applied to other mixer control name changes (like you already did for the HDA mic control).
That of course leads to the question do we soon need mechanisms to choose between more than two conditions (e.g. if mixer controls have changed multiple times in recent kernels, so covering for this in UCM would need a Switch, If-Else, or similar). But yeah, one can always define another UCM, so keeping-it-simple might be the right choice here.
I added everything to keep the interface backward compatible, so the current applications should not observe any different behavior. The applications like pulseaudio should use the 'hw:CARD_INDEX' specifier for the open call in the future and snd_use_case_parse_ctl_elem_id() helper for the element control names.
This sounds good as well. Some testing with common versions of e.g. Pulseaudio is probably in order to sanity check how this works.
Br, Kai
Dne 06. 11. 19 v 12:50 Kai Vehmanen napsal(a):
Hi Jaroslav,
On Tue, 5 Nov 2019, Jaroslav Kysela wrote:
I make some internal ucm code cleanups in alsa-lib and added three major extensions to allow more complex configurations which we require for the SOF kernel driver.
looks very good and pragmatic way to tackle some of the issues you hit with current UCM.
E.g. the If block would be also sufficient to tackle the recent HDMI codec driver change (with a single UCM file) -- i.e. use existence of the hdac-hdmi driver controls to select which enable-sequences to run. Hmm, I like this better than trying to select a whole different UCM file based on which drivers are used.
And same usage pattern can be applied to other mixer control name changes (like you already did for the HDA mic control).
That of course leads to the question do we soon need mechanisms to choose between more than two conditions (e.g. if mixer controls have changed multiple times in recent kernels, so covering for this in UCM would need a Switch, If-Else, or similar). But yeah, one can always define another UCM, so keeping-it-simple might be the right choice here.
I already implemented the nested If (so you may use another If in the True/False blocks).
Also 'String' (equal, substring) and 'RegexMatch' conditions were added.
For the substitution, I added ${CardComponents}, too. The driver might pass another component description strings to the user space for a better identification - there is snd_component_add() kernel function for this.
I added everything to keep the interface backward compatible, so the current applications should not observe any different behavior. The applications like pulseaudio should use the 'hw:CARD_INDEX' specifier for the open call in the future and snd_use_case_parse_ctl_elem_id() helper for the element control names.
This sounds good as well. Some testing with common versions of e.g. Pulseaudio is probably in order to sanity check how this works.
Yep, I will do more testing.
Do you have any progress with the pulseaudio volume UCM extension? Could you send me a link to the repository again? Thank you.
Jaroslav
Br, Kai
On Wed, 2019-11-06 at 14:10 +0100, Jaroslav Kysela wrote:
Dne 06. 11. 19 v 12:50 Kai Vehmanen napsal(a):
Hi Jaroslav,
On Tue, 5 Nov 2019, Jaroslav Kysela wrote:
I make some internal ucm code cleanups in alsa-lib and added three major extensions to allow more complex configurations which we require for the SOF kernel driver.
looks very good and pragmatic way to tackle some of the issues you hit with current UCM.
E.g. the If block would be also sufficient to tackle the recent HDMI codec driver change (with a single UCM file) -- i.e. use existence of the hdac-hdmi driver controls to select which enable-sequences to run. Hmm, I like this better than trying to select a whole different UCM file based on which drivers are used.
And same usage pattern can be applied to other mixer control name changes (like you already did for the HDA mic control).
That of course leads to the question do we soon need mechanisms to choose between more than two conditions (e.g. if mixer controls have changed multiple times in recent kernels, so covering for this in UCM would need a Switch, If-Else, or similar). But yeah, one can always define another UCM, so keeping-it-simple might be the right choice here.
I already implemented the nested If (so you may use another If in the True/False blocks).
Also 'String' (equal, substring) and 'RegexMatch' conditions were added.
For the substitution, I added ${CardComponents}, too. The driver might pass another component description strings to the user space for a better identification - there is snd_component_add() kernel function for this.
I added everything to keep the interface backward compatible, so the current applications should not observe any different behavior. The applications like pulseaudio should use the 'hw:CARD_INDEX' specifier for the open call in the future and snd_use_case_parse_ctl_elem_id() helper for the element control names.
This sounds good as well. Some testing with common versions of e.g. Pulseaudio is probably in order to sanity check how this works.
Yep, I will do more testing.
Do you have any progress with the pulseaudio volume UCM extension? Could you send me a link to the repository again? Thank you.
If you mean the ucm hw volume support: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/merge_requests/189 This is my fixes on top of Arun's original patch.
This is working pretty well for me, for example I have mute leds working in X1 with this. Unfortunately Pulseaudio community is pretty loaded with reviews, so no comments yet.
My problem is also that some distributions are using pulseaudio v11.1 so backporting is little bit nasty... My personal branch if for some reason someone want's to test in v11.1: https://gitlab.freedesktop.org/juimonen/pulseaudio/tree/lenovo_test (This branch has also couple of backports to support automatic routing between profiles -> ucm use cases)
br, Jaska
JaroslavBr, Kai
Dne 06. 11. 19 v 14:51 Jaska Uimonen napsal(a):
On Wed, 2019-11-06 at 14:10 +0100, Jaroslav Kysela wrote:
Dne 06. 11. 19 v 12:50 Kai Vehmanen napsal(a):
Hi Jaroslav,
On Tue, 5 Nov 2019, Jaroslav Kysela wrote:
I make some internal ucm code cleanups in alsa-lib and added three major extensions to allow more complex configurations which we require for the SOF kernel driver.
looks very good and pragmatic way to tackle some of the issues you hit with current UCM.
E.g. the If block would be also sufficient to tackle the recent HDMI codec driver change (with a single UCM file) -- i.e. use existence of the hdac-hdmi driver controls to select which enable-sequences to run. Hmm, I like this better than trying to select a whole different UCM file based on which drivers are used.
And same usage pattern can be applied to other mixer control name changes (like you already did for the HDA mic control).
That of course leads to the question do we soon need mechanisms to choose between more than two conditions (e.g. if mixer controls have changed multiple times in recent kernels, so covering for this in UCM would need a Switch, If-Else, or similar). But yeah, one can always define another UCM, so keeping-it-simple might be the right choice here.
I already implemented the nested If (so you may use another If in the True/False blocks).
Also 'String' (equal, substring) and 'RegexMatch' conditions were added.
For the substitution, I added ${CardComponents}, too. The driver might pass another component description strings to the user space for a better identification - there is snd_component_add() kernel function for this.
I added everything to keep the interface backward compatible, so the current applications should not observe any different behavior. The applications like pulseaudio should use the 'hw:CARD_INDEX' specifier for the open call in the future and snd_use_case_parse_ctl_elem_id() helper for the element control names.
This sounds good as well. Some testing with common versions of e.g. Pulseaudio is probably in order to sanity check how this works.
Yep, I will do more testing.
Do you have any progress with the pulseaudio volume UCM extension? Could you send me a link to the repository again? Thank you.
If you mean the ucm hw volume support: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/merge_requests/189 This is my fixes on top of Arun's original patch.
This is working pretty well for me, for example I have mute leds working in X1 with this. Unfortunately Pulseaudio community is pretty loaded with reviews, so no comments yet.
My problem is also that some distributions are using pulseaudio v11.1 so backporting is little bit nasty... My personal branch if for some reason someone want's to test in v11.1: https://gitlab.freedesktop.org/juimonen/pulseaudio/tree/lenovo_test (This branch has also couple of backports to support automatic routing between profiles -> ucm use cases)
Thanks, I see the misunderstanding between PlaybackVolume/PlaybackSwitch and PlaybackMixerID . The PlaybackVolume/PlaybackSwitch is for control API (snd_ctl_...) while PlaybackVolumeId is for selem API. The same is for the capture direction. It seems that PA uses the selem API, thus PlaybackVolumeId/CaptureVolumeId should be used (and defined in the ucm configuration files).
Also, the zero index for sid worries me, too:
SELEM_INIT(sid, e->alsa_name);
My machine:
$ amixer -D hw:PCH | grep Headphone Simple mixer control 'Headphone',0 Simple mixer control 'Headphone',1
We should also define and handle the dependency for mixer controls (Master -> Headphone) later in UCM and PA should handle this, too.
Jaroslav
On Tue, 05 Nov 2019 20:36:28 +0100, Jaroslav Kysela wrote:
Hi all,
I make some internal ucm code cleanups in alsa-lib and added three major extensions to allow more complex configurations which we require for the SOF kernel driver.
The first thing is the added substitution for the value strings:
https://github.com/alsa-project/alsa-lib/commit/f1e637b285e8e04e6761248a070f...
The second thing is the If block:
https://github.com/alsa-project/alsa-lib/commit/985715ce8148dc7ef62c8e3d8ce5...
The third thing is the card / hardware like specifier passed as the ucm name to snd_use_case_mgr_open() to support multiple card instances:
https://github.com/alsa-project/alsa-lib/commit/60164fc5886cdc6ca55eeed0c2e3...
All those patches (with other cleanups) are in the ucm2 branch on github for comments:
https://github.com/alsa-project/alsa-lib/commits/ucm2
The proposed SOF UCM config diff is here:
https://github.com/alsa-project/alsa-ucm-conf/commit/723b6da881721488229154e... https://github.com/alsa-project/alsa-ucm-conf/commits/ucm2
I added everything to keep the interface backward compatible, so the current applications should not observe any different behavior. The applications like pulseaudio should use the 'hw:CARD_INDEX' specifier for the open call in the future and snd_use_case_parse_ctl_elem_id() helper for the element control names.
The only concern with these extensions so far is the compatibility. Imagine that people run the new profile on the old parser, it'd break easily.
I think other scripts often installing on the versioned directory if incompatibilities are seen. Can we do that for UCM as well?
Or course, once after UCM parser is changed to be future-ready and allow some syntax for possible future extensions, we can keep that version directory in future, too.
thanks,
Takashi
Dne 07. 11. 19 v 7:48 Takashi Iwai napsal(a):
On Tue, 05 Nov 2019 20:36:28 +0100, Jaroslav Kysela wrote:
Hi all,
I make some internal ucm code cleanups in alsa-lib and added three major extensions to allow more complex configurations which we require for the SOF kernel driver.
The first thing is the added substitution for the value strings:
https://github.com/alsa-project/alsa-lib/commit/f1e637b285e8e04e6761248a070f...
The second thing is the If block:
https://github.com/alsa-project/alsa-lib/commit/985715ce8148dc7ef62c8e3d8ce5...
The third thing is the card / hardware like specifier passed as the ucm name to snd_use_case_mgr_open() to support multiple card instances:
https://github.com/alsa-project/alsa-lib/commit/60164fc5886cdc6ca55eeed0c2e3...
All those patches (with other cleanups) are in the ucm2 branch on github for comments:
https://github.com/alsa-project/alsa-lib/commits/ucm2
The proposed SOF UCM config diff is here:
https://github.com/alsa-project/alsa-ucm-conf/commit/723b6da881721488229154e... https://github.com/alsa-project/alsa-ucm-conf/commits/ucm2
I added everything to keep the interface backward compatible, so the current applications should not observe any different behavior. The applications like pulseaudio should use the 'hw:CARD_INDEX' specifier for the open call in the future and snd_use_case_parse_ctl_elem_id() helper for the element control names.
The only concern with these extensions so far is the compatibility. Imagine that people run the new profile on the old parser, it'd break easily.
I think other scripts often installing on the versioned directory if incompatibilities are seen. Can we do that for UCM as well?
Or course, once after UCM parser is changed to be future-ready and allow some syntax for possible future extensions, we can keep that version directory in future, too.
While we are going to separate UCM files from alsa-lib to alsa-ucm-conf we can define the new syntax change until the first version is released (I can put a notice to README).
Speaking for Fedora, we have dependancy 'alsa-lib package version' must be equal to 'alsa-ucm package version'. If users will do any changes on their own, they should know what they are doing.
Anyway, we should learn from this and I would propose to add add something like 'Syntax 2' to the main configuration file now. The new functions should be activated only according the version.
Unfortunately, the current parser will just show an error message like:
ALSA lib parser.c:1337:(parse_master_file) uknown master file field Syntax ALSA lib parser.c:1337:(parse_master_file) uknown master file field If
But at least, users should be notified that there is a configuration mismatch.
Another possibility is to change the suffix for the master configuration file to accept the "Syntax" check for the another future update. But honestly, I don't like ".conf2" and ".v2.conf" looks not so nice, too.
Jaroslav
On Thu, 07 Nov 2019 09:33:27 +0100, Jaroslav Kysela wrote:
Dne 07. 11. 19 v 7:48 Takashi Iwai napsal(a):
On Tue, 05 Nov 2019 20:36:28 +0100, Jaroslav Kysela wrote:
Hi all,
I make some internal ucm code cleanups in alsa-lib and added three major extensions to allow more complex configurations which we require for the SOF kernel driver.
The first thing is the added substitution for the value strings:
https://github.com/alsa-project/alsa-lib/commit/f1e637b285e8e04e6761248a070f...
The second thing is the If block:
https://github.com/alsa-project/alsa-lib/commit/985715ce8148dc7ef62c8e3d8ce5...
The third thing is the card / hardware like specifier passed as the ucm name to snd_use_case_mgr_open() to support multiple card instances:
https://github.com/alsa-project/alsa-lib/commit/60164fc5886cdc6ca55eeed0c2e3...
All those patches (with other cleanups) are in the ucm2 branch on github for comments:
https://github.com/alsa-project/alsa-lib/commits/ucm2
The proposed SOF UCM config diff is here:
https://github.com/alsa-project/alsa-ucm-conf/commit/723b6da881721488229154e... https://github.com/alsa-project/alsa-ucm-conf/commits/ucm2
I added everything to keep the interface backward compatible, so the current applications should not observe any different behavior. The applications like pulseaudio should use the 'hw:CARD_INDEX' specifier for the open call in the future and snd_use_case_parse_ctl_elem_id() helper for the element control names.
The only concern with these extensions so far is the compatibility. Imagine that people run the new profile on the old parser, it'd break easily.
I think other scripts often installing on the versioned directory if incompatibilities are seen. Can we do that for UCM as well?
Or course, once after UCM parser is changed to be future-ready and allow some syntax for possible future extensions, we can keep that version directory in future, too.
While we are going to separate UCM files from alsa-lib to alsa-ucm-conf we can define the new syntax change until the first version is released (I can put a notice to README).
Speaking for Fedora, we have dependancy 'alsa-lib package version' must be equal to 'alsa-ucm package version'. If users will do any changes on their own, they should know what they are doing.
This assumes that you have only one alsa-ucm package. If there is a downstream version of UCM profile, this won't work well always.
Anyway, we should learn from this and I would propose to add add something like 'Syntax 2' to the main configuration file now. The new functions should be activated only according the version.
Yeah, some extensibility is needed in the config space.
Unfortunately, the current parser will just show an error message like:
ALSA lib parser.c:1337:(parse_master_file) uknown master file field Syntax ALSA lib parser.c:1337:(parse_master_file) uknown master file field If
Right, that's the problem now. Even a non-existing control would lead to an error with the current version of parser.
But at least, users should be notified that there is a configuration mismatch.
I don't think this would suffice. The new UCM config is not merely a config but it's becoming rather a language, so this needs some distinction from the current v1 files.
Another possibility is to change the suffix for the master configuration file to accept the "Syntax" check for the another future update. But honestly, I don't like ".conf2" and ".v2.conf" looks not so nice, too.
My vote is for a different directory. And, with v2 extension, we should leave the room for further extensibility, and keep the same location as long as it's compatible. Keeping the location for incompatible configs would lead to a mess.
thanks,
Takashi
Dne 07. 11. 19 v 10:23 Takashi Iwai napsal(a):
On Thu, 07 Nov 2019 09:33:27 +0100, Jaroslav Kysela wrote:
Dne 07. 11. 19 v 7:48 Takashi Iwai napsal(a):
On Tue, 05 Nov 2019 20:36:28 +0100, Jaroslav Kysela wrote:
Hi all,
I make some internal ucm code cleanups in alsa-lib and added three major extensions to allow more complex configurations which we require for the SOF kernel driver.
The first thing is the added substitution for the value strings:
https://github.com/alsa-project/alsa-lib/commit/f1e637b285e8e04e6761248a070f...
The second thing is the If block:
https://github.com/alsa-project/alsa-lib/commit/985715ce8148dc7ef62c8e3d8ce5...
The third thing is the card / hardware like specifier passed as the ucm name to snd_use_case_mgr_open() to support multiple card instances:
https://github.com/alsa-project/alsa-lib/commit/60164fc5886cdc6ca55eeed0c2e3...
All those patches (with other cleanups) are in the ucm2 branch on github for comments:
https://github.com/alsa-project/alsa-lib/commits/ucm2
The proposed SOF UCM config diff is here:
https://github.com/alsa-project/alsa-ucm-conf/commit/723b6da881721488229154e... https://github.com/alsa-project/alsa-ucm-conf/commits/ucm2
I added everything to keep the interface backward compatible, so the current applications should not observe any different behavior. The applications like pulseaudio should use the 'hw:CARD_INDEX' specifier for the open call in the future and snd_use_case_parse_ctl_elem_id() helper for the element control names.
The only concern with these extensions so far is the compatibility. Imagine that people run the new profile on the old parser, it'd break easily.
I think other scripts often installing on the versioned directory if incompatibilities are seen. Can we do that for UCM as well?
Or course, once after UCM parser is changed to be future-ready and allow some syntax for possible future extensions, we can keep that version directory in future, too.
While we are going to separate UCM files from alsa-lib to alsa-ucm-conf we can define the new syntax change until the first version is released (I can put a notice to README).
Speaking for Fedora, we have dependancy 'alsa-lib package version' must be equal to 'alsa-ucm package version'. If users will do any changes on their own, they should know what they are doing.
This assumes that you have only one alsa-ucm package. If there is a downstream version of UCM profile, this won't work well always.
Anyway, we should learn from this and I would propose to add add something like 'Syntax 2' to the main configuration file now. The new functions should be activated only according the version.
Yeah, some extensibility is needed in the config space.
Unfortunately, the current parser will just show an error message like:
ALSA lib parser.c:1337:(parse_master_file) uknown master file field Syntax ALSA lib parser.c:1337:(parse_master_file) uknown master file field If
Right, that's the problem now. Even a non-existing control would lead to an error with the current version of parser.
But at least, users should be notified that there is a configuration mismatch.
I don't think this would suffice. The new UCM config is not merely a config but it's becoming rather a language, so this needs some distinction from the current v1 files.
Another possibility is to change the suffix for the master configuration file to accept the "Syntax" check for the another future update. But honestly, I don't like ".conf2" and ".v2.conf" looks not so nice, too.
My vote is for a different directory. And, with v2 extension, we should leave the room for further extensibility, and keep the same location as long as it's compatible. Keeping the location for incompatible configs would lead to a mess.
Ok, I can move the new configs to ucm2 (/usr/share/alsa/ucm2) and leave the original directory empty (as fallback), because all configs can be modified to support the right card identificator (kernel module id parameter) rather than the hard coded default value generated by the ALSA core kernel code.
But there is an issue with the environment variable "ALSA_CONFIG_UCM" which specifies directly the directory. We cannot predict the syntax for it.
Jaroslav
thanks,
Takashi
On Thu, 07 Nov 2019 12:08:26 +0100, Jaroslav Kysela wrote:
Dne 07. 11. 19 v 10:23 Takashi Iwai napsal(a):
On Thu, 07 Nov 2019 09:33:27 +0100, Jaroslav Kysela wrote:
Dne 07. 11. 19 v 7:48 Takashi Iwai napsal(a):
On Tue, 05 Nov 2019 20:36:28 +0100, Jaroslav Kysela wrote:
Hi all,
I make some internal ucm code cleanups in alsa-lib and added three major extensions to allow more complex configurations which we require for the SOF kernel driver.
The first thing is the added substitution for the value strings:
https://github.com/alsa-project/alsa-lib/commit/f1e637b285e8e04e6761248a070f...
The second thing is the If block:
https://github.com/alsa-project/alsa-lib/commit/985715ce8148dc7ef62c8e3d8ce5...
The third thing is the card / hardware like specifier passed as the ucm name to snd_use_case_mgr_open() to support multiple card instances:
https://github.com/alsa-project/alsa-lib/commit/60164fc5886cdc6ca55eeed0c2e3...
All those patches (with other cleanups) are in the ucm2 branch on github for comments:
https://github.com/alsa-project/alsa-lib/commits/ucm2
The proposed SOF UCM config diff is here:
https://github.com/alsa-project/alsa-ucm-conf/commit/723b6da881721488229154e... https://github.com/alsa-project/alsa-ucm-conf/commits/ucm2
I added everything to keep the interface backward compatible, so the current applications should not observe any different behavior. The applications like pulseaudio should use the 'hw:CARD_INDEX' specifier for the open call in the future and snd_use_case_parse_ctl_elem_id() helper for the element control names.
The only concern with these extensions so far is the compatibility. Imagine that people run the new profile on the old parser, it'd break easily.
I think other scripts often installing on the versioned directory if incompatibilities are seen. Can we do that for UCM as well?
Or course, once after UCM parser is changed to be future-ready and allow some syntax for possible future extensions, we can keep that version directory in future, too.
While we are going to separate UCM files from alsa-lib to alsa-ucm-conf we can define the new syntax change until the first version is released (I can put a notice to README).
Speaking for Fedora, we have dependancy 'alsa-lib package version' must be equal to 'alsa-ucm package version'. If users will do any changes on their own, they should know what they are doing.
This assumes that you have only one alsa-ucm package. If there is a downstream version of UCM profile, this won't work well always.
Anyway, we should learn from this and I would propose to add add something like 'Syntax 2' to the main configuration file now. The new functions should be activated only according the version.
Yeah, some extensibility is needed in the config space.
Unfortunately, the current parser will just show an error message like:
ALSA lib parser.c:1337:(parse_master_file) uknown master file field Syntax ALSA lib parser.c:1337:(parse_master_file) uknown master file field If
Right, that's the problem now. Even a non-existing control would lead to an error with the current version of parser.
But at least, users should be notified that there is a configuration mismatch.
I don't think this would suffice. The new UCM config is not merely a config but it's becoming rather a language, so this needs some distinction from the current v1 files.
Another possibility is to change the suffix for the master configuration file to accept the "Syntax" check for the another future update. But honestly, I don't like ".conf2" and ".v2.conf" looks not so nice, too.
My vote is for a different directory. And, with v2 extension, we should leave the room for further extensibility, and keep the same location as long as it's compatible. Keeping the location for incompatible configs would lead to a mess.
Ok, I can move the new configs to ucm2 (/usr/share/alsa/ucm2) and leave the original directory empty (as fallback), because all configs can be modified to support the right card identificator (kernel module id parameter) rather than the hard coded default value generated by the ALSA core kernel code.
But there is an issue with the environment variable "ALSA_CONFIG_UCM" which specifies directly the directory. We cannot predict the syntax for it.
Right, that's a bit of headache. Another idea would be to we put under /usr/share/alsa/ucm/v2/... and the parser starts looking at $ALSA_CONFIG_UCM/v$VERSION/... then falls back to the $ALSA_CONFIG_UCM/...
But I'm really open for any other options, too.
thanks,
Takashi
Dne 07. 11. 19 v 12:16 Takashi Iwai napsal(a):
On Thu, 07 Nov 2019 12:08:26 +0100, Jaroslav Kysela wrote:
Dne 07. 11. 19 v 10:23 Takashi Iwai napsal(a):
On Thu, 07 Nov 2019 09:33:27 +0100, Jaroslav Kysela wrote:
Dne 07. 11. 19 v 7:48 Takashi Iwai napsal(a):
On Tue, 05 Nov 2019 20:36:28 +0100, Jaroslav Kysela wrote:
Hi all,
I make some internal ucm code cleanups in alsa-lib and added three major extensions to allow more complex configurations which we require for the SOF kernel driver.
The first thing is the added substitution for the value strings:
https://github.com/alsa-project/alsa-lib/commit/f1e637b285e8e04e6761248a070f...
The second thing is the If block:
https://github.com/alsa-project/alsa-lib/commit/985715ce8148dc7ef62c8e3d8ce5...
The third thing is the card / hardware like specifier passed as the ucm name to snd_use_case_mgr_open() to support multiple card instances:
https://github.com/alsa-project/alsa-lib/commit/60164fc5886cdc6ca55eeed0c2e3...
All those patches (with other cleanups) are in the ucm2 branch on github for comments:
https://github.com/alsa-project/alsa-lib/commits/ucm2
The proposed SOF UCM config diff is here:
https://github.com/alsa-project/alsa-ucm-conf/commit/723b6da881721488229154e... https://github.com/alsa-project/alsa-ucm-conf/commits/ucm2
I added everything to keep the interface backward compatible, so the current applications should not observe any different behavior. The applications like pulseaudio should use the 'hw:CARD_INDEX' specifier for the open call in the future and snd_use_case_parse_ctl_elem_id() helper for the element control names.
The only concern with these extensions so far is the compatibility. Imagine that people run the new profile on the old parser, it'd break easily.
I think other scripts often installing on the versioned directory if incompatibilities are seen. Can we do that for UCM as well?
Or course, once after UCM parser is changed to be future-ready and allow some syntax for possible future extensions, we can keep that version directory in future, too.
While we are going to separate UCM files from alsa-lib to alsa-ucm-conf we can define the new syntax change until the first version is released (I can put a notice to README).
Speaking for Fedora, we have dependancy 'alsa-lib package version' must be equal to 'alsa-ucm package version'. If users will do any changes on their own, they should know what they are doing.
This assumes that you have only one alsa-ucm package. If there is a downstream version of UCM profile, this won't work well always.
Anyway, we should learn from this and I would propose to add add something like 'Syntax 2' to the main configuration file now. The new functions should be activated only according the version.
Yeah, some extensibility is needed in the config space.
Unfortunately, the current parser will just show an error message like:
ALSA lib parser.c:1337:(parse_master_file) uknown master file field Syntax ALSA lib parser.c:1337:(parse_master_file) uknown master file field If
Right, that's the problem now. Even a non-existing control would lead to an error with the current version of parser.
But at least, users should be notified that there is a configuration mismatch.
I don't think this would suffice. The new UCM config is not merely a config but it's becoming rather a language, so this needs some distinction from the current v1 files.
Another possibility is to change the suffix for the master configuration file to accept the "Syntax" check for the another future update. But honestly, I don't like ".conf2" and ".v2.conf" looks not so nice, too.
My vote is for a different directory. And, with v2 extension, we should leave the room for further extensibility, and keep the same location as long as it's compatible. Keeping the location for incompatible configs would lead to a mess.
Ok, I can move the new configs to ucm2 (/usr/share/alsa/ucm2) and leave the original directory empty (as fallback), because all configs can be modified to support the right card identificator (kernel module id parameter) rather than the hard coded default value generated by the ALSA core kernel code.
But there is an issue with the environment variable "ALSA_CONFIG_UCM" which specifies directly the directory. We cannot predict the syntax for it.
Right, that's a bit of headache. Another idea would be to we put under /usr/share/alsa/ucm/v2/... and the parser starts looking at $ALSA_CONFIG_UCM/v$VERSION/... then falls back to the $ALSA_CONFIG_UCM/...
But I'm really open for any other options, too.
I would probably vote for this:
/usr/share/alsa/ucm2 - new configs with 'Syntax' field protection /usr/share/alsa/ucm - old configs
ALSA_CONFIG_UCM2 - env path for the new configs ALSA_CONFIG_UCM - env path for the old configs
Jaroslav
On 2019-11-05 20:36, Jaroslav Kysela wrote:
Hi all,
I make some internal ucm code cleanups in alsa-lib and added three major extensions to allow more complex configurations which we require for the SOF kernel driver.
The first thing is the added substitution for the value strings:
https://github.com/alsa-project/alsa-lib/commit/f1e637b285e8e04e6761248a070f...
The second thing is the If block:
https://github.com/alsa-project/alsa-lib/commit/985715ce8148dc7ef62c8e3d8ce5...
The third thing is the card / hardware like specifier passed as the ucm name to snd_use_case_mgr_open() to support multiple card instances:
https://github.com/alsa-project/alsa-lib/commit/60164fc5886cdc6ca55eeed0c2e3...
All those patches (with other cleanups) are in the ucm2 branch on github for comments:
https://github.com/alsa-project/alsa-lib/commits/ucm2
The proposed SOF UCM config diff is here:
https://github.com/alsa-project/alsa-ucm-conf/commit/723b6da881721488229154e...
https://github.com/alsa-project/alsa-ucm-conf/commits/ucm2
I added everything to keep the interface backward compatible, so the current applications should not observe any different behavior. The applications like pulseaudio should use the 'hw:CARD_INDEX' specifier for the open call in the future and snd_use_case_parse_ctl_elem_id() helper for the element control names.
If you have another ideas to address those issues, please, let me know.
BTW, Mark: The SOF UCM configs relies on the driver name changes, so it might be worth to send "ASoC: intel - fix the card names" patch to 5.4 to make things stable more quickly:
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/commit/?h=...
Thanks, Jaroslav
Thanks for your work, Jaroslav.
However, I have some concerns here. First, could you elaborate on "we require for the SOF kernel driver"?
The substitutions and multi-instance support is probably warmly welcomed by many, but "If" blocks are what worries me. Especially the nested "Ifs". As Takashi pointed already out, UCM - which is currently is viewed as a simple configuration syntax - is becoming a language on its own. If we are to keep extending UCM on and on, we might as well switch to JSON/ XML/ YAML entirely instead of developing our own thingy.
"If" block could just be what's needed to open new pandora box, allowing for very complex and no longer easy-to-read config files. In general, if one is to enlist an "If", why not define two UCMs instead?
Moreover, I see you mentioning the card-name dependency. This sounds rather invasive. Separation of different config-versions would be required.
Czarek
Dne 07. 11. 19 v 11:18 Cezary Rojewski napsal(a):
On 2019-11-05 20:36, Jaroslav Kysela wrote:
Hi all,
I make some internal ucm code cleanups in alsa-lib and added three major extensions to allow more complex configurations which we require for the SOF kernel driver.
The first thing is the added substitution for the value strings:
https://github.com/alsa-project/alsa-lib/commit/f1e637b285e8e04e6761248a070f...
The second thing is the If block:
https://github.com/alsa-project/alsa-lib/commit/985715ce8148dc7ef62c8e3d8ce5...
The third thing is the card / hardware like specifier passed as the ucm name to snd_use_case_mgr_open() to support multiple card instances:
https://github.com/alsa-project/alsa-lib/commit/60164fc5886cdc6ca55eeed0c2e3...
All those patches (with other cleanups) are in the ucm2 branch on github for comments:
https://github.com/alsa-project/alsa-lib/commits/ucm2
The proposed SOF UCM config diff is here:
https://github.com/alsa-project/alsa-ucm-conf/commit/723b6da881721488229154e...
https://github.com/alsa-project/alsa-ucm-conf/commits/ucm2
I added everything to keep the interface backward compatible, so the current applications should not observe any different behavior. The applications like pulseaudio should use the 'hw:CARD_INDEX' specifier for the open call in the future and snd_use_case_parse_ctl_elem_id() helper for the element control names.
If you have another ideas to address those issues, please, let me know.
BTW, Mark: The SOF UCM configs relies on the driver name changes, so it might be worth to send "ASoC: intel - fix the card names" patch to 5.4 to make things stable more quickly:
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/commit/?h=...
Thanks, Jaroslav
Thanks for your work, Jaroslav.
However, I have some concerns here. First, could you elaborate on "we require for the SOF kernel driver"?
Please, look here:
https://github.com/alsa-project/alsa-ucm-conf/commit/a8253465aef2df494ccd5b1...
The HDA driver sometimes manages different JackControl names depending on the used codec and it would be the real maintenance mess to use the DMI info (long card name) for all possible configurations.
Also, if you look to the current configs, many duplications can be removed with the If evaluations.
The substitutions and multi-instance support is probably warmly welcomed by many, but "If" blocks are what worries me. Especially the nested "Ifs". As Takashi pointed already out, UCM - which is currently is viewed as a simple configuration syntax - is becoming a language on its own. If we are to keep extending UCM on and on, we might as well switch to JSON/ XML/ YAML entirely instead of developing our own thingy.
The configuration syntax itself is really close to JSON, it's just about the run-time evaluation of some blocks at the load time. The different static syntax format does not help us so much.
"If" block could just be what's needed to open new pandora box, allowing for very complex and no longer easy-to-read config files. In general, if one is to enlist an "If", why not define two UCMs instead?
For HDA, USB or and drivers with many hardware variants, the managing of thousands of files will be the real nightmare. Also, at some point, I would like to create the use case configs for all hardware, thus pulseaudio or any other server (maybe pipewire in the future) can use the use case configuration to abstract fully the hardware without their own profiles or so... The goal is to have this in the one place.
Moreover, I see you mentioning the card-name dependency. This sounds rather invasive. Separation of different config-versions would be required.
What do you mean with this?
Jaroslav
Czarek
Hi,
On Thu, 7 Nov 2019, Jaroslav Kysela wrote:
Dne 07. 11. 19 v 11:18 Cezary Rojewski napsal(a):
On 2019-11-05 20:36, Jaroslav Kysela wrote: However, I have some concerns here. First, could you elaborate on "we require for the SOF kernel driver"?
Please, look here:
https://github.com/alsa-project/alsa-ucm-conf/commit/a8253465aef2df494ccd5b1...
The HDA driver sometimes manages different JackControl names depending on the used codec and it would be the real maintenance mess to use the DMI info (long card name) for all possible configurations.
Also, if you look to the current configs, many duplications can be removed with the If evaluations.
yes, I agree with Jaroslav here. There's definitely the risk of making UCM files too complex, but pragmatically, we really need something between the current boolean choice between a generic UCM picked by card shortname and per-product tailored UCM'es picked up by card longname.
There's some variation in HDA codec controls that is more convenient to hide in if-else logic in the generic UCM. Same goes for addition of controls to the kernel. Without conditional logic, using a newer UCM file with an older kernel simply breaks -- even if the change is incremental only.
There was a discussion in the summer on the list about adding versions in kernel and using that to help pick the right UCM profile. This isn't an easy path either -- there are many components contributing to the overall card interface on kernel side, and you still need to pick (or compile at runtime) a single UCM file. It would seem with Jaroslav's current proposal, you could cover majority types of minor variations and thus significantly limit the number of UCM files that need to be maintained.
Br, Kai
participants (6)
-
Cezary Rojewski -
Jaroslav Kysela -
Jaska Uimonen -
Kai Vehmanen -
Pierre-Louis Bossart -
Takashi Iwai