-
Notifications
You must be signed in to change notification settings - Fork 325
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SOF_GUI: Implement GUI and TUI for SOF demonstration #9406
Conversation
Can one of the admins verify this patch?
|
test this please |
Which systems can run this GUI, Linux or Windows? Or both? |
@@ -0,0 +1 @@ | |||
4607827,0,116,50331648,0,0,0,0,116,2,1,0,0,0,0,0,0,2,2,0,0,0,0,3316150158,2048164275,513807534,3267352229,513807534,0,16384,3867454526,1191025347,38870735,77741469,38870735,4294967292,24197, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are duplicates from tools/ctl/ip3. It would be good to align and have a single set of blobs for sof-ctl somewhere. And somehow have selection for ipc3 vs. ipc4 blobs usage. I'm also thinking to change the .txt blob headers format to be the same as .bin used by UCM2, so if duplication can be avoided there's less work to maintain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, I prefer .txt over .bin especially in git where we can see the diffs. UCM2 chose a slightly different headers format for blobs and since sof-ctl is only a small utility I'm planning to change it to align with UCM2 but still keep also the .txt blobs. Then the .bin and .txt blob content would be the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have any suggestions for where this should be? For now, I'll remove them from this directory and document where they are/how to add them to the gui.
Commit here: 01c19dd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could there be a configure widget in the GUI where the path could be set? It would also make this compatible with both IPC versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexb3103 @singalsu it should be, maybe, a text box where one can write the path to their eq_configs.
print(f"Failed to set volume: {e}") | ||
|
||
def handle_eq(eq_file_name: str): | ||
ctl_command = f"./sof-ctl -D{device_string} -n {eq_numid} -s {eq_file_name}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the -c with control name as text as shown by amixer controls is more robust than number value. E.g. -c name='DMIC0 Capture IIR Eq'
. The numbers change if topology is modified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I definitely agree with that, however one functionality of the controller engine is that it automatically detects the numid for these basic components. Check out the initialize_device() method in the controller engine, specifically lines 37-43 and 45-51.
Because of this autodetection, I think that using the explicit name leads to a higher chance of error overall.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I didn't know there's such feature. It sounds like the numids are then fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very cool @alexb3103 , thanks for the submission! Before more detailed comments, one high-level nit I have is with the name "SOF GUI". This is a bit too generic as SOF can be used in various places and purposes. I wonder if we could come up with a bit more descriptive name, perhaps "SOF Control GUI" (control-gui), or maybe something around SOF components (component-gui)?
Another is how to deal with the wave files. Could you generate these as part of the build process as these seem pretty standard files? Not completely against this, but it's just we haven't put raw audio data files to the git so far in SOF.
.wav is a very old and common format so hopefully not much of an attack vector anymore but it's still pretty bad from a security perspective to have unauditable binaries in a source repo. Also, git is really bad with binaries (hence LFS and others) and 11M per file is pretty big. |
@alexb3103 good stuff ! btw, care to share some screen shots ? |
Hi, thanks for the comment. As of now, this GUI is made for Linux and is not tested for Windows. It is definitely possible to extend support in the future if there is a lot of desire for this. |
Thoughts on "sof-demo-gui"? The overall goal here is to provide an easy way to demonstrate sof as a whole or a specific component on target hardware, so making the name too specific will not encapsulate the entire purpose. In terms of the .wav files, I removed them in this commit 01c19dd and added documentation for how to add when needed. |
![]() I just noticed you add them in the first commit and remove them in the second commit. But this project and repo do not use Github that usual way. It does not use the "squash" button either. Instead, you must rewrite your commits and force-push = the "traditional", non-Github way to use git. For more about this "traditional" way to rewrite git commits:
|
Hi, thanks for the info. I am used to working this way as that is how it is done in my professional work. However, we wait to rebase until approval so that I can provide atomic commit messages to make review much simpler. Happy to fix it now if you would prefer though. |
Here's a screenshot, I will be preparing a demonstration video in a couple days as well. |
tools/gui/README-dev.md
Outdated
@@ -0,0 +1,40 @@ | |||
## Developing sof-gui and tui | |||
|
|||
The architecture of the sof UIs is simple, and designed to make implementing new features extremely easy. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
capital "SOF" - as is it looks just like a normal word with a typo
tools/gui/README-dev.md
Outdated
|
||
The architecture of the sof UIs is simple, and designed to make implementing new features extremely easy. | ||
|
||
Controller engine "sof_controller_engine.py", that handles all interaction with linux and sof. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Linux" "SOF"
tools/gui/README-dev.md
Outdated
|
||
Add graphics and other quality of life functions to the GUI. | ||
|
||
Create a version of sof-ctl that provides direct python bindings to communicate with SOF components, rather than needing a linux command. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Python" "Linux"
tools/gui/README.md
Outdated
User input logic and display handling | ||
|
||
### sof-controller-engine | ||
Controller to abstract the gui and tui control. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's use consistent capitalisation "GUI" "TUI"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm good with "sof-demo-gui". With the other minor things addressed, I think this is ready to be merged.
Release reminder - one week to v2.11-rc1. |
@iuliana-prodan if you agree I think @AnneOnciulescu can take over this PR and fix the comments until next week? |
@alexb3103 do you have time to fix the comments/suggestions by the end of next week? |
Hi all, Just fixed the remaining comments unless I missed something. I also rebased down to the single commit. Let me know what remaining changes are necessary, thanks very much for the feedback so far. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For a POC this is really good.
Thanks @alexb3103
We can add new features/ improvements on top of this.
@alexb3103 Any demo? |
Unfortunately I have the board/monitor packed so I'm not able to make it. Also, I think it would be best for whoever takes over the project to make the demo, as that would give experience with it. I'm happy to assist in any way necessary towards that. |
3596d96
to
3241a58
Compare
CI fails at:
|
SOFCI TEST |
Looks like this is an error from older code. I wonder if I can retrigger the CI. |
The only way to retrigger everything is:
There is one shortcut for Jenkins but only for Jenkins. It will re-test but not rebuild anything if the SHA1 has not changed. |
Implements a gui and tui that can be used to easily demonstrate SOF on target HW. See README and README-dev for more information on functionality and purpose. Signed-off-by: Alexander Brown <alex.brown.3103@gmail.com>
Not tested yet by CI, failures are known and unrelated to this PR. |
Implements a gui and tui that can be used to easily demonstrate SOF on target HW. See README and README-dev for more information on functionality and purpose.
See #9223 for the initial description of the project and overall scope. Most features from that description were met here.
One important goal before this PR is ready to be merged is for this to be tested on a variety of platforms. At this point, the GUI is tested on most NXP platforms, but little other testing has been done yet. Please feel free to comment with any issues that are had on any platform, and I'll work on resolving them ASAP.
A second focus is on understandability of the docs, please let me know if any sections are unclear. A large goal of this project is to allow for new components to be easily tested and demonstrated on target HW, so it needs to be very easy for any SOF developer to pick up.