Repository navigation
sof-tplgreader: parse rates from topology file - #1174
Conversation
961605b to
f388f07
Compare
| if rate & (1 << 8) != 0: | ||
| rates.append("64000") | ||
| if rate & (1 << 9) != 0: | ||
| rates.append("96000") |
There was a problem hiding this comment.
This is a lot of repetition; I think you don't need to "unroll" that loop:
bit_rates_dict = { 1 : 8000, 3 : 16000, ...}
for bit in rates_dict:
if rate & (1 << bit) != 0:
rates.append(bit_rates_dict[bit])Or even shorter
rates = [ bit_rates_dict[bit] for bit in bit_rates_dict
if rate & (1 << bit) != 0 ]There was a problem hiding this comment.
This is really good comment, but I can't get it to work. I put a TODO above. There are a couple of places where the same thing is applicable. Can I make this improvement as a separate PR?
There was a problem hiding this comment.
What is the error? Later = never.
There was a problem hiding this comment.
I tried to test this to help but the new functions are never called. Did you forget a part?
There was a problem hiding this comment.
I tried to test this to help but the new functions are never called. Did you forget a part?
OK I figured it out: the "interactive" output of the parser is incomplete.
I just tested this below together with https://github.com/thesofproject/sof/pull/9017/files and it works perfectly fine:
@staticmethod
def _to_rate_string(rate):
# Note: intentionally put 48000 first to make first in the list
bit_rates_dict = { 7 : 48000, 6 : 44100, }
return [ str(bit_rates_dict[bit]) for
bit in bit_rates_dict if rate & (1 << bit) != 0 ]I forgot the str() earlier, is that why it was failing for you?
There was a problem hiding this comment.
wow, it works nicely. I just updated again. Let me check the test results before remove draft
|
I have further changes and currently testing with devices now. Found some errors, will try to update the PR in this afternoon |
f388f07 to
a7f9990
Compare
fredoh9
changed the title
| * tplgtool.py | ||
| <br> Dumps info from tplg binary file. (deprecated) | ||
| <br> Dumps info from tplg binary file. sof-tplgreader.py still use this but new features | ||
| will go to tplgtool2.py. Will be deprecated. |
There was a problem hiding this comment.
Can you please explain a little bit more which script uses which script and when?
There was a problem hiding this comment.
The way these 3 scripts interact with each other is really not intuitive and it took me an unreasonable amount time to figure out. Since you're adding a new feature to the deprecated script (while leaving the newer script behind?), please add a couple more sentences describing the situation better. Right now this part of the README.md is worse than nothing.
There was a problem hiding this comment.
The way this 3 scripts interact with each other is really not intuitive.
I agree this part.
I don't understand rest of your comment. Why do you think tplgtool.py is deprecated? README was misleading. When tplgtool2.py was introduced, (if it wants to replace tplgtool.py) it has to take care of all the functions in tplgtool.py. But it didn't. More likely it added a new file and put a deprecated notice on the currently being used script.
There was a problem hiding this comment.
Why do you think tplgtool.py is deprecated?
Because it says so:
$ ./tools/tplgtool.py
warnings.warn("tplgtool.py is deprecated, use tplgtool2.py instead.", DeprecationWarning)
There was a problem hiding this comment.
Glad I'm not the only one confused!
3d8eeef to
9179e7e
Compare
Related to sampling rate in caps, there are rates, rate_min and rate_max. Added support for the rates list from the topology file and use first in the list as supported rate. The sampling rate 48000 is intentionally placed first in the dictionary to ensure it appears first in the list. Signed-off-by: Fred Oh <fred.oh@linux.intel.com>
9179e7e to
840d3cc
Compare
|
This has some side-effects to tests with PCMs that have lower than 48000 rates: |
|
@fredoh9 should we revert temporarily while you design a fix? |
No, it will break every case. I will take a look shortly |
4 participants
I was able to test with thesofproject/sof#9017