drowe67 / freedv-gui

GUI Application for FreeDV – open source digital voice for HF radio
https://freedv.org/
GNU Lesser General Public License v2.1
193 stars 49 forks source link

Voice Keyer updates #730

Closed tmiw closed 2 months ago

tmiw commented 2 months ago

Updates the Voice Keyer feature based on feedback in #728:

  1. Update application configuration to only have user selecting the path to the voice keyer file(s), not the exact file. This path also changes as needed based on user selection of the VK file (see (2)).
  2. When left-clicking the Voice Keyer button and there's no previously selected VK file available (or the previous file no longer exists), bring up a file dialog to force selection of the exact VK file to use.
  3. Update tooltip for the Voice Keyer button to display the currently selected file.
  4. Ensure that the name of the file selected in (2) continues to be persisted across sessions.
barjac commented 2 months ago

Nice job Mooneer, seems fine for me. I have directories set up as follows under ~/freedv/

└── VK_files
    ├── CQ
    │   ├── CQ_10.wav
    │   ├── CQ_12.wav
    │   ├── CQ_160_700D.wav
    │   ├── CQ_160.wav
    │   ├── CQ_17m.wav
    │   ├── CQ_20.wav
    │   ├── CQ_40.wav
    │   ├── CQ_60.wav
    │   └── CQ_80.wav
    └── Testing
        ├── Testing.wav
        ├── testnow1.wav
        └── testnow.wav

The last used directory and file are remembered OK between sessions and this is really user friendly. The tooltip looks rather cluttered but works adequately. I would still prefer just the filename without extension on the button but it's your call :) Maybe a patch to test it would allow some more evaluation?

Slightly /OT

I was going to make this next comment a new enhancement issue but since it is VK related I will add it here for now:

An option in the settings to set the relative audio level when monitoring transmitted audio.

A single value +/- (dB) relative to the current decoder output level on RX.

It is currently too loud and to be able to drop it to a comfortable level would be really good.

I never use the PTT monitor so for me one setting would do for VK and PTT however if others have a use for different levels on each then maybe that would be worthwhile.

tmiw commented 2 months ago

Maybe a patch to test it would allow some more evaluation?

I pushed a change to allow this but I'm leaning towards reverting. Here's what it looks like for me:

image

There's also a chance that this won't work well on Windows but we'll see how it goes on Linux and macOS first.

Tyrbiter commented 2 months ago

One suggestion, could the filename be in a more contrasting colour please.

tmiw commented 2 months ago

One suggestion, could the filename be in a more contrasting colour please.

Unfortunately I don't think wxWidgets allows this. All of the text needs to be one font/color.

Tyrbiter commented 2 months ago

One suggestion, could the filename be in a more contrasting colour please.

Unfortunately I don't think wxWidgets allows this. All of the text needs to be one font/color.

Fair enough, this is not easy with the limitations of the various toolkits.

barjac commented 2 months ago

Maybe a patch to test it would allow some more evaluation?

I pushed a change to allow this but I'm leaning towards reverting. Here's what it looks like for me:

image

There's also a chance that this won't work well on Windows but we'll see how it goes on Linux and macOS first.

Absolutely perfect for me in Plasma-6.1.1 using a low aspect (1280x1024) monitor.

Screenshot_20240704_192620

That is the longest file name that I currently have and I don't envisage needing longer.

There is a tiny bug though in the GUI. Sometimes the VK button width is displayed the same width as the others which crops the first and last characters. It is instantly corrected by slightly adjusting the window height, whereupon the button width expands to fit the text.

tmiw commented 2 months ago

A quick test on Windows using the default filename for the voice keyer WAV file:

image

Still not 100% sure about it. I'll see if I can get more feedback.

tmiw commented 2 months ago

How's the filename truncation (most recent commit I just pushed) working for you guys? It sounds like the feedback for having the filename in the button label is positive, so I guess it stays.

Tyrbiter commented 2 months ago

I'm fairly happy with this, and I see that some other people on various mailing lists are too. @barjac is probably quite keen too despite the GUI glitch he mentions.

On Windows I see that the button height is a bit low, is there a way to tweak that with a #define maybe?

barjac commented 2 months ago

This latest has broken it.

image

Now the VK button does not expand to take the text as it did previously, even when tweaking the main window height.

I tried dragging the window into all shapes and sizes but the VK button is stubbornly staying the same width as the others now.

Tyrbiter commented 2 months ago

This latest has broken it.

Now the VK button does not expand to take the text as it did previously, even when tweaking the main window height.

That must be the truncation part of Mooneer's comment yesterday, does the full filename appear in the tooltip for you?

Perhaps truncation above a certain length could be applied instead?

barjac commented 2 months ago

This latest has broken it. Now the VK button does not expand to take the text as it did previously, even when tweaking the main window height.

That must be the truncation part of Mooneer's comment yesterday, does the full filename appear in the tooltip for you?

Yes but I really don't like using the tooltip for this when there can be room on the button.

Perhaps truncation above a certain length could be applied instead?

Yes it must, but not sure how. With a proportional font it's not possible to specify a character count for this. It would need to be done at pixel level.

I much prefer the button width to expand to fit the text as it did before, to the maximum width that the widget allows. Only then truncate the text without ellipses, as that occupies space that could be used.

Users will soon realize that long file names will not fit and adjust them accordingly.

tmiw commented 2 months ago

This latest has broken it. Now the VK button does not expand to take the text as it did previously, even when tweaking the main window height.

That must be the truncation part of Mooneer's comment yesterday, does the full filename appear in the tooltip for you?

Perhaps truncation above a certain length could be applied instead?

Truncation was originally based on the length of the 'Voice Keyer' label. It's now based on the size of the button itself (and I've adjusted the button sizing so that it fills the entire 'Control' box too).

I much prefer the button width to expand to fit the text as it did before, to the maximum width that the widget allows. Only then truncate the text without ellipses, as that occupies space that could be used.

I actually think that the buttons should all be the same size. It looks weird having one button be significantly bigger than the others, at least IMO. The latest changes here should hopefully be sufficient.

Users will soon realize that long file names will not fit and adjust them accordingly.

Ehh....not sure that's a good scenario, either. But at least if the button width is large enough, that should provide enough letters for most to be able to differentiate between each VK file.

Tyrbiter commented 2 months ago

I actually quite like the wider buttons, they are easier to hit with the mouse and a little more obvious when active.

barjac commented 2 months ago

Yes I have no objection to those either, in fact it makes sense to use the available space if it can't be used for anything else. :)

This is looking good now however there is something strange going on with justification of the button text.

If the filename is shorter than 'Voice Keyer' then 'Voice Keyer' is centre justified on the button but the filename is left justified aligned with the the 'V' of 'Voice Keyer'.

Screenshot_20240707_151131

If the filename is longer than 'Voice Keyer' then the filename is centre justified and 'Voice Keyer' moves left and becomes left justified aligned with the start of the filename.

Screenshot_20240707_151557

Not a big deal but if it can be fixed then super, otherwise I am really happy with this now from a functionality and user friendliness point of view - nice job Mooneer!

tmiw commented 2 months ago

This is looking good now however there is something strange going on with justification of the button text.

This appears to be a Linux specific thing, unfortunately. On macOS at least the alignment looks okay (both lines being centered):

image

I'll double-check on Windows and then merge.

tmiw commented 2 months ago

Merged.

@barjac, can you go ahead and create a new GH issue for the monitor volume thing you mentioned?

barjac commented 2 months ago

Merged.

Thanks - seems odd that wxwidgets should behave differently using the same version.

@barjac, can you go ahead and create a new GH issue for the monitor volume thing you mentioned?

Done :)