-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
QML settings sound hardware #14597
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
QML settings sound hardware #14597
Conversation
d1b4984
to
73db0ed
Compare
I have pushed couple of updates, which should behave as shown bellow. I consider this the final version for now, but should like revisit it to add a tutorial/guided screen on first open. The legacy view is also TODO but I'd like to move this on a subsequent PR. Note that there is some fix-ups to be moved inside #14568 before this can go Screencast.From.2025-04-06.21-13-22-10MB.mp4(Windows demo starting at 1:22) |
2291c38
to
837911f
Compare
837911f
to
6cbc52e
Compare
The Mixer has only the Left Bus, but not Center and Right Bus. |
The Record/Broadcast has only one Input. This is by default connected to Main output of the Mixer, but can be assigned to an external soundhardware input instead. But the QML GUI allows visually double connections. |
I will had further constraints on this
Yep, I will add those too
Right, am I understanding correctly that |
There may only be one input for record/broadcast and the input must always be connected to exactly one source. No more and no less. By default a conection from the Main Output of the Mixer to this input shall be established. |
6cbc52e
to
793119f
Compare
With the latest push, I should have addressed the three point you mentioned (alternative recorder input, no direct input to output mapping missing bus outputs). Please note that the QML pre-commit hooks have yet again screwed up the code base as I was about to push so it might have broken some bits (see #14604) so I have gone ahead and disable it for now. |
There appears no visual indication of which control is selected, which makes navigation with the keyboard nearly impossible. |
0943ca6
to
9e01e38
Compare
9e01e38
to
052f86c
Compare
Test Pt1 : Clean Win 10 x64 VM (4cpus 16Gb , build installed. (1920*1080) overal 1st impression: font is too small QML takes +/- 30 of CPU, legacy +/- 15% while just playing a track (same track ALAC, no stem), even without playing decks (legacy: 8%) (overall performance is not good, spinnies stutter, waveforms stutter, music stutters QML: Audio Buffer on 85 and still not enough buffer to listen clear to the track, (SampleRate 48k) Crash on exit |
Thanks for the overall review on QML @Eve00000 ! Let's keep the focus at hand tho, and keep track of the other issues separately.
The font in general will need further work, but this will come in a later stage. Same with the combobox (not sure if you remember, but I mentioned having already 5 versions of it across the five branches, so I just need to consolidate that once merged). Overall, colours and size will need further adjustment when we get to polishing.
There is supposed to be a close button on top, but I haven't yet implemented it on the global setting dialog, SOmething to come in a future PR, alongside the "detach" mode which allow you to pop the dialog out (useful when testing thing, but also still going live)
Yes, there is still global issues with performance. Most of it likely come from the fact that I would assume you don't have any graphical acceleration in the VM (which QML heavily rely on). There is no doubt there will be some work to do on that front later!
Great! |
I really think we should not be focusing on design issues with this first pass. Let's focus on functionality -- does it work correctly? Does it cause crashes? Is the code well structured? If the foundation is ok, then we can iterate on design. |
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.
Some code structure comments
I fully aggree, lets get the technology merged, UI can be revised later. |
Thanks @ywwg and @JoergAtGithub for looking into this.
It would be really appreciated if you could elaborate a bit more what are the current issue on Windows (besides the keyboard, which is likely a "skill issue" as mentioned here - still need to wrap my head around it!) as raised there, so we can later on address this. I am mostly testing on a VM and I appreciate the the setup is likely too minimal to be realistic.
Could also clarify this? Is this only related to keyboard or do you have other accessibility issues? |
I did above. On my simple setup I need three time the dialog height to display the sounddevice inputs/outputs. Not an issue for me as expert, but a normal user would never get an overview with wires always leaving the visible screen area. Regarding accesibility: Luckily I'm not affected personal, but we have several visual impaired users. I can't imagine how our blind users can connect the wires correctly. This was never an issue with the current UI, where you can select every string individually and the braile reader reads each this selected string. |
Right - I guess there is some effort to be done on compactness for the sound boxes. Am I right in thinking this is however better that what you raised here, with same device input/output showing on the same device box instead of having these duplicates?
I'd be curious to hear feedback from impacted user. Is the current view really working this well with screen reader? Also worth to say that the legacy view with this matrix of combobox will still be available in the |
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 like this logical split a lot more, thank you. One big question about that qml_owned_ptr code :)
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.
QML to the future!!!
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.
couple thoughts, not a thorough review. Overall in decent shape I think.
75d294e
to
48a650c
Compare
Apologies @Swiftb0y I realised there was two comments from you I had forgotten to address. Hopefully everything is addressed now and it does build 🤞 |
thanks. I don't have the muse to take another look at this. @ywwg please merge if you're comfortable with your LGTM. |
@acolombier please squash the fixups and I'll merge |
0b345da
to
8511563
Compare
Done @ywwg! |
If you merge this PR, it might be worth merging #15214 |
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 don't consider the graphic setup as good user experience on Windows, due to the huge number of inputs and outputs, which do not fit in the preferences window. But lets merge this first step. We can polish the UI later, as QML is not enabled for end users yet.
Partially closes #14542
Current limitation: