Closed ranj063 closed 3 years ago
@lrgirdwo @kv2019i @plbossart FYI
I have a bit mixed feelings about the place for this upper level of the parser. I see an analogy to the C pre-processor. The original idea was to have a file (static topology) and an application (dynamic topology management) input. This upper level is used only for the static topology decription.
Perhaps, it may be nice to have this layer completely separate - the text file as input and generate the text file (original syntax) on output. The implementation can be moved to the alsatplg tool. If we see that we can use this code also in the dynamic way (applications), then we can reconsider to add it to the topology library.
@perexg fwiw, we would also want to use this code as a way to create alsa-plugin audio pipelines that reuse the SOF processing modules and would run on the host. i.e. we would have the same topology definition that could be used by host or DSP depending on use case. In this case it would be an advantage to have the API in alsa-lib (even if we have to make it a high level API ??)
I have a bit mixed feelings about the place for this upper level of the parser. I see an analogy to the C pre-processor. The original idea was to have a file (static topology) and an application (dynamic topology management) input. This upper level is used only for the static topology decription.
Perhaps, it may be nice to have this layer completely separate - the text file as input and generate the text file (original syntax) on output. The implementation can be moved to the alsatplg tool. If we see that we can use this code also in the dynamic way (applications), then we can reconsider to add it to the topology library.
Thanks for your review @perexg. The problem with having it as a separate layer and converting it to the original syntax would make the conf files unmaintainable. The main problem that we would like to address with Topology2.0 is that the original syntax is not easy to read and process when it has the full topology definition for a machine. So I see topology2.0 as an extension to the original syntax instead of a separate layer.
Thanks for your review @perexg. The problem with having it as a separate layer and converting it to the original syntax would make the conf files unmaintainable.
I don't understand your point. The layer may be independent inside alsatplg. So from the usage POV, there's no difference. The alsatplg may just read the tplg 2.0 conf file, run the class/object evaluator, generate new v1.0 configuration file (in-memory only) and pass it to the libatopology . I don't say to store / maintain the v1.0 configuration here. It may be useful to dump it only for the debugging purposes.
@perexg fwiw, we would also want to use this code as a way to create alsa-plugin audio pipelines that reuse the SOF processing modules and would run on the host. i.e. we would have the same topology definition that could be used by host or DSP depending on use case. In this case it would be an advantage to have the API in alsa-lib (even if we have to make it a high level API ??)
I'm not against to include this to libatopology on demand. But your mentioned mechanism is not prepared (dynamic topology loading to the driver etc.). And my point is, that this higher layer can be nicely isolated from the binary topology generation. If there will be another syntax in the future, we can just make this layer optional.
I don't understand your point. The layer may be independent inside alsatplg. So from the usage POV, there's no difference. The alsatplg may just read the tplg 2.0 conf file, run the class/object evaluator, generate new v1.0 configuration file (in-memory only) and pass it to the libatopology . I don't say to store / maintain the v1.0 configuration here. It may be useful to dump it only for the debugging purposes.
Ahh I see. that makes sense. I will add the class/object parser as an additional step in alsatplg. Thanks!
@perexg fwiw, we would also want to use this code as a way to create alsa-plugin audio pipelines that reuse the SOF processing modules and would run on the host. i.e. we would have the same topology definition that could be used by host or DSP depending on use case. In this case it would be an advantage to have the API in alsa-lib (even if we have to make it a high level API ??)
I'm not against to include this to libatopology on demand. But your mentioned mechanism is not prepared (dynamic topology loading to the driver etc.). And my point is, that this higher layer can be nicely isolated from the binary topology generation. If there will be another syntax in the future, we can just make this layer optional.
Fwiw, @ranj063 has a PR pending for dynamic pipelines in the driver, but this does not yet have a external interface for userspace to push the topology binaries. The plugin based pipeline would work as follows 1) Sound server opens a plugin based pipeline 2) plugin loads topology conf and parses 3) plugin creates a host (frontend) processing pipline for the audio processing parts to be performed on host. 4) (optional) plugin sends binary topology to driver to create (backend) processing pipeline in DSP.
I'm not against to include this to libatopology on demand. But your mentioned mechanism is not prepared (dynamic topology loading to the driver etc.). And my point is, that this higher layer can be nicely isolated from the binary topology generation. If there will be another syntax in the future, we can just make this layer optional.
@perexg @lgirdwood I have now updated the Topology2.0 compiler to be part of alsatplg. This PR has not been updated with the mostly function exports needed for parsing/saving topology elements.
I am still working on refactoring the alsatplg compiler but the initial version can be found here (top 31 commits) https://github.com/ranj063/alsa-utils/commits/topic/topology2.0
I'm not against to include this to libatopology on demand. But your mentioned mechanism is not prepared (dynamic topology loading to the driver etc.). And my point is, that this higher layer can be nicely isolated from the binary topology generation. If there will be another syntax in the future, we can just make this layer optional.
@perexg @lgirdwood I have now updated the Topology2.0 compiler to be part of alsatplg. This PR has not been updated with the mostly function exports needed for parsing/saving topology elements.
I am still working on refactoring the alsatplg compiler but the initial version can be found here (top 31 commits) https://github.com/ranj063/alsa-utils/commits/topic/topology2.0
Nice, but won't be better to not generate the text output internally - snd_tplg_save_printf(), but the snd_config_t tree ? You can save / dump it anytime using snd_config_save() function (including subtrees for debugging). It also means, that you may copy the configuration blocks directly without parsing / ASCII print sequences. This may help to integrate things to alsa-lib later, too.
EDIT: The in-place substitutions (remove classe / object trees and extend the others) in the snd_config_t tree are also allowed.
Nice, but won't be better to not generate the text output internally - snd_tplg_save_printf(), but the snd_config_t tree ? You can save / dump it anytime using snd_config_save() function (including subtrees for debugging). It also mean, that you may copy the configuration blocks directly without parsing / ASCII print sequences. This may help to integrate things to alsa-lib later, too.
Indeed @perexg , I think it will be better to generate the snd_config_t tree. This will reduce the number of exports I will need. Thanks for the suggestion
@perexg I've updated this PR now to generate the config tree from the 2.0 conf file. I really only need 2 exports that will be useful in the 2.0 compiler. I have the work in progress version of the compiler code here: https://github.com/ranj063/alsa-utils/tree/topic/topology2.0-v2
And the topology examples here: https://github.com/thesofproject/sof/pull/3983
Could you please let me know if you are OK with the syntax and the direction for the compiler?
@perexg I've updated this PR now to generate the config tree from the 2.0 conf file. I really only need 2 exports that will be useful in the 2.0 compiler. I have the work in progress version of the compiler code here: https://github.com/ranj063/alsa-utils/tree/topic/topology2.0-v2
And the topology examples here: thesofproject/sof#3983
Could you please let me know if you are OK with the syntax and the direction for the compiler?
Thank you for your work. Before I start a deeper review, do you think that we can simplify the code more? I mean that the current code creates own structures / lists for the already known information which is stored in the configuration tree. I think that it may be possible to create the upper level functions which do introspection in the config tree and remove the internal structures like:
/* remove struct tplg_class / struct tplg_object etc. */ /* returns the class name */ const char *tplg_class_name(snd_config_t *class_conf); /* class lookup */ snd_config_t *tplg_class_find_by_name(const char *name); /* return the attribute type */ enum tplg_class_param_type tplg_class_attribute(snd_config_t *attribute_conf); /* lookup by class and attribute name / snd_config_t *tplg_class_attribute_find_by_name(const char *class_name, const char *attribute_name); /* iterate through all classes and it's attributes */ snd_config_search(top_config, "Class", &classes); snd_config_for_each(i, next, classes) { _class = snd_config_iterator_entry(i); snd_config_search(_class, "DefineAttribute", &attributes); snd_config_search(i2, next2, attributes) { ...validate...; } } /* do objects */ snd_config_search(top_config, "Object", &objects); snd_config_for_each(i, next, objects) { _class = snd_config_iterator_entry(i); ... validate, generate ...; }
It's just an idea... It may remove the 'config -> C structure' conversion code, but I don't know if the config helper functions to extract the required information won't add more code than the current implementation. Also, if there are some internal states, they must be handled inside the config tree in my proposal, too.
It's just an idea... It may remove the 'config -> C structure' conversion code, but I don't know if the config helper functions to extract the required information won't add more code than the current implementation. Also, if there are some internal states, they must be handled inside the config tree in my proposal, too.
Thanks for your feedback @perexg. I can certainly look into replacing the c-structs with config helper functions. Let me work on it and get back with input on which version is better in terms of the number of lines of code and readability.
@perexg I was wondering if there's a way for me to use the config parser to evaluate equations? For example, for the buffer object, I'd like to compute the buffer size using its attributes.
@perexg I was wondering if there's a way for me to use the config parser to evaluate equations? For example, for the buffer object, I'd like to compute the buffer size using its attributes.
You may get any item / subtree from the config tree (snd_config_search accepts id chaining like "id1.id2.id3") and you can do own operations / extractions on top of it. But I admit, that I don't understand your question completely. An example may help.
You may get any item / subtree from the config tree (snd_config_search accepts id chaining like "id1.id2.id3") and you can do own operations / extractions on top of it. But I admit, that I don't understand your question completely. An example may help.
For ex: the buffer class has an attribute called size in my class: https://github.com/thesofproject/sof/pull/3983/commits/8d2c3750911753383015c7a4c6d111761b2ac190
But the size of the buffer depends on the other attributes periods/format/channels/rate etc. Is there a way for me to add a reference to the formula this attribute can use to set it value.
For ex: the buffer class has an attribute called size in my class: thesofproject/sof@8d2c375
But the size of the buffer depends on the other attributes periods/format/channels/rate etc. Is there a way for me to add a reference to the formula this attribute can use to set it value.
There is no such mechanism. You need to write your own. You can replace the "auto" values in the tree with the results (fixed values) when all dependencies are known.
There is no such mechanism. You need to write your own. You can replace the "auto" values in the tree with the results (fixed values) when all dependencies are known.
OK, thats what I am doing today so I will stick to it. Thank you!
There is no such mechanism. You need to write your own. You can replace the "auto" values in the tree with the results (fixed values) when all dependencies are known.
@perexg Sorry for the delay. I was out on vacation and I'm in the process of testing the new changes. I will close this PR and open a new one in alsa-utils in a couple of days.
This pull request includes the preparatory commits paving the way for introducing Topology2.0. Existing alsatplg compiler functionality is unmodified.
An example implementation of topology with Topology2.0 can be found here: https://github.com/thesofproject/sof/pull/3936
Introduction to Topology 2.0
and all topology building changes will be in the alsatplg compiler.