WIP: Input/Output toolbars

[akleja]

Resolves: #435

Merges functionality of Meter Toolbars, ~Mixer toolbar~ and Device toolbar

• Removes device toolbar.
• Adds menu buttons next to the meters for selecting device/channel count.
• Defines new default positions for the toolbars.
• ~Adds volume sliders below the meters.~

Todo:

• [ ] Verify that I/O assignment works as it should
• [ ] Add small down arrows to buttons to indicate menu
• [ ] Sort out translatable strings / tooltips
• [ ] Improve min-sizes
• [ ] Acessiblity
  • [ ] Add device selection functionality to Extra menu
  • [ ] Test accessibility on windows for meter
  • [ ] Add Accessibility class for buttons / menus on windows?
• [ ] Reduce reliance of hardcoded numbers for sizing
• [ ] Cleanup of code / comments / commits
• [ ] Rename toolbars from Record Toolbar/Playback Toolbar to Input Toolbar/Output Toolbar
• [x] Resolve startup crash on windows

Background: At first I thought the best way to go about this was to just rely on the Prefs>Device. But for users having multiple audio devices that are switched between frequently, like say a bluetooth headset and internal soundcard it seems reasonable to make changing devices easy.

Additional notes:

• I can't get keyboard focus to work with widgets/AButton, but that seems to be an issue with AButton, and should probably be solved separately since I have the same problem with the other instances of AButton.
• ~Combined toolbar is enabled temporarily, but will be disabled again eventually.~
• This is a work in progress. Some things might behave badly.
• Some solutions aren't the best since I'm a novice. Suggestions for improvements are welcome.

Current appearance:

Checklist

• [x] I have signed off my commits using -s or Signed-off-by* (See: Contributing § DCO)
• [x] I made sure the code compiles on my machine
• [ ] I made sure there are no unnecessary changes in the code*
• [ ] I made sure the title of the PR reflects the core meaning of the issue you are solving*
• [ ] I made sure the commit message(s) contain a description and answer the question "Why do those changes fix that particular issue?" or "Why are those changes really necessary as improvements?"*

* indicates required

[akleja]

The windows build seems broken, Since I don't have a windows build environment this will be tricky to investigate further for now.

[Be-ing]

What do you mean? The Windows build passed.

[Be-ing]

I edited your todo list so it shows actual checkboxes.

[akleja]

Yeah I didn't word it properly. What I meant is that the windows version crashes on launch. Sorry for lack of clarity

[fitojb]

Since it’s a drop-down button, it should have a ▼ next to it

[akleja]

@fitojb Yeah I agree! It's on the to do list :)

[Be-ing]

This pull request is quite large. Would it be feasible to split off small independent changes? I suspect the answer is no in this case because the design only works well when considered all together.

[akleja]

I guess splitting some things off would work, and might make sense. Removal of Device/Mixer toolbars could fairly easily be split off. But the changes to meter toolbars and widget/Meter might be hard to split up in a way that makes sense.

Another thought is wether to keep the sliders for PortMixer or not. Not keeping them would simplify things a lot, not to mention reduce the size of the PR significantly. But not keeping them would kinda mean dropping PortMixer support which might be a bit hasty.

I could perhaps split the menu stuff and the sliders stuff into separate PRs. But since they wouldn't be entirely independent as to the order they would need to be applied I think just separating the commits a bit more strictly might be the better option.

Also I'm unsure about keep supporting the combined toolbar. The implementation is a bit strange and I'm not sure if it'll ever be used again. I've kept it around for now since I thought it might be useful, but dropping it might make more sense, and might be good to do while we have the chance to make possibly breaking changes to the cfg file, which I suspect is the main reason why it's still there.

[Be-ing]

I think just separating the commits a bit more strictly might be the better option.

Sure.

Also I'm unsure about keep supporting the combined toolbar.

When in doubt, I say remove code.

[Be-ing]

Another thought is wether to keep the sliders for PortMixer or not. Not keeping them would simplify things a lot, not to mention reduce the size of the PR significantly. But not keeping them would kinda mean dropping PortMixer support which might be a bit hasty.

If it is significant extra work to maintain, I am okay with removing it. As we discussed on Matrix before, that feature didn't even work reliably before and it's probably impossible to make it work reliably without the magic power to force all audio interface manufacturers and operating systems to follow a consistent standard.

[akleja]

When in doubt, I say remove code.

I've begun removed the combined meter. It's a relief to get rid of it tbh.

If it is significant extra work to maintain, I am okay with removing it. As we discussed on Matrix before, that feature didn't even work reliably before and it's probably impossible to make it work reliably without the magic power to force all audio interface manufacturers and operating systems to follow a consistent standard.

Yeah I mean right now it's fine but for the future it'll surely be quite the hassle maintaining an old conan branch just for making sure the PortMixer stuff works. At least on my system PortMixer never worked reliably in the first place. And apart from maintainability, dropping in would make this PR so much smaller and simpler. But if that is the way forward I think the rest of the PortMixer stuff should be stripped out in it's own PR along with the removal of the mixer toolbar. That I can easily break of from this PR.

[Be-ing]

Maybe we should put this PR on hold and remove all the PortMixer stuff first.

[akleja]

Yeah I could split of the commit that removes the mixer toolbar and then go through the rest of the PortMixer stuff and make a separate PR. Then strip out the slider implementation in this PR and move forward. I'll start looking into it tomorrow. At a first glance it doesn't seem too troublesome.

[akleja]

There's also EXPERIMENTAL_AUTOMATED_INPUT_LEVEL_ADJUSTMENT that should be scrapped as well. It uses PortMixer to try to adjust the input level automatically, which is just silly imo.

[n0toose]

There's also EXPERIMENTAL_AUTOMATED_INPUT_LEVEL_ADJUSTMENT that should be scrapped as well. It uses PortMixer to try to adjust the input level automatically, which is just silly imo.

On it.

[n0toose]

I have good news and bad news.

The good news is that I fixed my compiler setup because I haven't been at it for a bit. The bad news is (for me, this is actually good news lol) that the issue @akleja brought up already being dealt with here: https://github.com/tenacityteam/tenacity/pull/559/

Just leaving it here for posterity.

[akleja]

@panos Oh sorry, I should've posted here when I opened the PR, my bad

[n0toose]

Not sure if this is to be reviewed.

[akleja]

Sorry for the lack of activity here. I'm not sure how to move forward with this PR since there's a problem on windows which I don't know how to debug. I suspect it is linked to the accessibility features in widgets/Meter.cpp, but this is purely speculative. Any help on this would be much appreciated.

[n0toose]

Sorry, kind of missed something here. What kind of problem are we talking about?

CC: @emabrey

[akleja]

It just segfaults on launch.

The first step in this PR was to remove the old buttons from Meter.cpp and I think I messed up MeterAx when I did so. But it could be caused by something completely different as well, I'm mostly guessing here.

I don't have the possibility of setting up a win development VM right now due to having an old machine and very limited hard drive space, as well as a lack of familiarity with development on windows. Which leaves me with the option of pushing possible fixes and check the artifacts, which seems like a tedious approach and seems like it would clog the build queue. If anybody can provide some insight, or at least a backtrace I'd be really thankful.

[n0toose]

@akleja Would it help if I provided you with a Windows VM? It'd be very clunky and running on my laptop because I was planning to install Windows on that thing for development purposes myself, but I guess that's one way to go around this.

[akleja]

@panos Thanks, but I'm not sure, it sounds like a last resort type of solution, but if everything else falls through it might be an option. Just realized I do have an ancient laptop, so I'll try using it.

[akleja]

Just a small update. I have a windows build environment set up and I'm making some progress. Startup crash on windows is resolved. Crash on windows when changing host by accessing device settings from Input/Output menus detected. Investigating.

[peepo5]

If it is causing crashing, I suggest we have quite a few testers to mess around with this version.

[akleja]

@peepopoggers Absolutely, especially when it's finished for review it'll need extensive testing since it's quite a big change. There's still a lot to do though. I have at least resolved the crashing issues for now it seems.

[n0toose]

Is this ready to be tested? (Should I change the name and then share this as much as possible?)

[akleja]

No not yet, I still have to figure out some of the accessibility issues, like a good way to include the dropdowns in the extra menu, and improve screen reader behaviour. Also I'll have to add arrows to the buttons. And tidy up code. I'm working on it, but I'm having a busy week. Should be able to make more progress during the weekend

[n0toose]

Okay, got it, take your time :)