Closed juimonen closed 4 years ago
@perexg do not merge this, I need comments from other sof folks first... These are the conf files from 1.4.1 taq.
@juimonen : Thank you for this. I will wait for the feedback.
@juimonen @perexg we still need to figure out the correct names for the DAI links on i.MX8 topologies, as we already changed them 3 times until now. I do expect that this to be ready next week.
Also, the biggest problem I see is that pushing just the .conf files would be a pain. We don't really edit that files directly. The best way would be to import all .m4 files and have a script that generates the .conf file for releases.
updated:
The files should be "normalized". My proposal is already in alsa-utils (alsatpl -n option). Ideally, it should be combined with -s (sort the compound identifiers), but it should be tested. I get different output binary file sizes with this combination.
EDIT: The sort option was added to keep text sorted for the diffs.
I found the reason why the binary file sizes differ. The fix is here: https://github.com/alsa-project/alsa-lib/commit/2b50b594dcbf69f8dcc6e8cf673748f7063c3c17 . So, I would like to import only output from alsatplg -n /source-file.conf/ -s -o /normalized-file.conf/
only.
I would also recommend to add header to each file like:
# This file was generated using 'alsatplg -n /source-file.conf/ -s' from # giturl: GIT_HTTPS_URL here # gittag: branch/tag name here # sha512: SHA512 of the original .conf file
> I would also recommend to add header to each file like: > > # This file was generated using 'alsatplg -n /source-file.conf/ -s' from > # giturl: GIT_HTTPS_URL here > # gittag: branch/tag name here > # sha512: SHA512 of the original .conf file
and a pointer to the license file!
and a pointer to the license file!
Yes, if it differs from BSD 3-Clause..
@perexg @plbossart we don't have the conf files in git as they are generated from m4 macros.
If we are doing the alsatplg normalization, I think it should be added to "normal/basic" sof CMakefiles so that they get tested in our CI.
We can try also to include some form of "comment header" to all conf files in m4, so to describe where/how they are generated and possible the license.
I don't want the import to differ too much how we are compiling the topologies in sof.
@perexg so what I'm saying here, this will take me some time... I mailed our CI folks if we can try out the normalization for regressions, I can play around myself with the comment header generation, and then backport this to 1.4.1. So couple of days at least...
@perexg I don't understand how we can deal with 'normalized' files. We can't track every single option of the tools and if there is a problem with the files they should be fixed at the M4 level, not reprocessed with the errors not fixed at the source.
@perexg I don't understand how we can deal with 'normalized' files. We can't track every single option of the tools and if there is a problem with the files they should be fixed at the M4 level, not reprocessed with the errors not fixed at the source.
Make a bold warning to the header in the normalized files that fixes should go to your repository.
I did a lot of cleanups in the topology library and alsatplg tool. All is pushed to alsa-lib and alsa-utils now. If you have any issue, let me know. The normalization is now much better. Also, the output can be grouped by the index (component domains) for the verification and I have to say, that the output is much more readable than your m4 processed configuration and we can do clear diffs now.
@perexg sorry about the delay... I think we are little bit puzzled about the normalization thing. So should the topologies work exactly same way if they are normalized or not? So they should be parsed the same? And the normalization here means that widgets are grouped somehow differently that helps diffing? I kind of missed the original issue you had.
Anyway this is kind of painful to test as we need to update the CI docker images for new alsa-lib and compare the results to original ones... Just explaining why this is taking time, we can't do this kind of tests very easily in our CI. So need to be sure this is the thing to do...
@juimonen : The topology files generated from the normalized files are same (in other words - they contain identical configuration for the driver), but there are some differences:
1) I fixed DAPM widget sorting by index (domain) thus the DAPM widget blocks may be shorter with the new topology library; the -z,--dapm-nosort
option turns this sorting off (library behaves like previously)
2) normalization sorts keys; it means that the configuration blocks are shuffled in the final binary topology generated from the normalized file, but again - the information / configuration for the driver is same (I believe)
BTW: I added the binary topology decoder to the current alsatplg tool / topology library, too. The result is saved in the .conf file syntax. It might help to check the contents.
I cannot say that there are no bugs, but I tried to test the code with the current available topologies and checked the generated / decoded binary topologies.
I really completely disagree with the idea of importing .conf files that are not maintained or even version-controlled by the SOF team. It'd be a better idea to just provide a stand-alone script to convert M4 files into .tplg ones, as suggested by @lrgirdwo
@plbossart that still doesn't change the question, should this script normalize or not, so what should be the alsatplg switches and/or do we care? As I understood from @perexg, the final binary size is differing depending on these switches. I'm also just asking that the resulting tplg files are the same as we are testing in CI (normalized or not).
Again, the contents (information) does not change. If you normalize the .conf and create .tplg from this .conf file, the functionality must be identical. So you can test this in CI etc. I think that you just don't understand what I'm trying to propose. The m4 macros are not very readable anyway. Positional (instead named) arguments lose the readability etc...
I don't insist to use my solution, I'm just trying to give a tool to manage this for us (work with the text .conf files, decode the binary .tplg files back and allow the comparisons to check things with the original souce). If you have another idea, what distributions should do, I'm open for it. Happy new year to all.
The suggested path: m4 -> .conf -> normalized .conf -> .tplg -> testing/verification -> all pass -> push normalized .conf to upstream (controlled by the m4 or whatever text processor owner), ok?
alsatplg -n m4.conf -o norm.conf alsatplg -c norm.conf -o my.tplg
My alternative proposal is m4 + script > .tplg -> testing/verification -> all pass -> push m4+script to upstream
No one needs to care about .conf files that are just intermediate representations and not intended to be used.
Note that we will at some point ditch M4 and have a more structured way of generating topologies, but the .conf will remain an intermediate step, not the end-goal
Quoting my reply to ML:
"The .conf files can be very easily compared with the original source.
Basically, I really want to let things to move forward. Everything is better than to have the topology configuration in the big repo with all other things, so I can accept this, too.
I had to probably use my time to work on something different than libatopology, but at least I found several problems and did a lot of cleanups.''
Please, open new PR, if you have something ready. Maybe, I can do this import myself.
Import topology files from sof v1.4.1.
Signed-off-by: Jaska Uimonen jaska.uimonen@linux.intel.com