sof-tplgreader: parse rates from topology file #1174
Conversation
fredoh9
force-pushed
the
fix/tplgreader_rate_support
branch
from
961605b
to
f388f07
Compare
tools/tplgtool.py
Outdated
| 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 |
fredoh9
force-pushed
the
fix/tplgreader_rate_support
branch
from
f388f07
to
a7f9990
Compare
fredoh9
changed the title
README.md
Outdated
| @@ -148,7 +148,8 @@ apl | |||
| <br> Dumps info from tplg binary file. | |||
|
|
|||
| * 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!
fredoh9
force-pushed
the
fix/tplgreader_rate_support
branch
2 times, most recently
from
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 <[email protected]>
fredoh9
force-pushed
the
fix/tplgreader_rate_support
branch
from
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 |
I was able to test with thesofproject/sof#9017