libobs: Fix Wayland push to talk (sort of)

[thomasza92]

Description

Querying global key state is sometimes unreliable, so unknown states should not be treated the same as a released hotkey.

Added a pressed-state enum in order to distinguish between pressed, released, and unknown key states.

Motivation and Context

I initially ran into this issue while trying to setup OBS with Arch using Wayland. I quickly found that I could not get push to talk working correctly. I checked the open issues and saw that there were at least two I could find which I believe were describing the same problem I was experiencing (#10701 #13169).

This comment is what I based my fix on: #10701 (comment)

While I was testing the fix I also ran into this same issue on macOS when building from source at the current latest commit (085a51a). Perhaps someone can further verify that this problem exists there. I originally planned to gate the changes behind checks for linux and wayland but since it was causing problems for me on macOS as well I opted for the following changes instead.

Note that this PR touches several different files and deals with some core functionality of OBS. That being said, I tried to keep the changes as minimal as possible. Any further discussion or suggestion on edits would be greatly appreciated.

How Has This Been Tested?

I tested this on Arch Linux, Windows 11, and macOS, all of which seemed to work fine, but I think more testing should definitely be done.

Types of changes

• Bug fix (non-breaking change which fixes an issue)

This PR does NOT fix the issue of hotkeys not working on Wayland when the window is not in focus!!

I believe the only way to fix that issue would be to integrate OBS with Global Shortcuts through XDG Desktop Portal. That is a much bigger change and is probably beyond my capabilities.

Checklist:

• [ x] I have read the contributing document.
• [ x] My code has been run through clang-format.
• [ x] My code follows the project's style guidelines
• [ x] My code is not on the master branch.
• My code has been tested.
• [ x] All commit messages are properly formatted and commits squashed where appropriate.
• I have included updates to all appropriate documentation.

[suogesi]

Possibly coflict with this: #13363. It completely refactored handle_binding. Would you like to discuss this with me?

[thomasza92]

Possibly coflict with this: #13363. It completely refactored handle_binding. Would you like to discuss this with me?

Sure! After further testing I've noticed at least two issues that persist.

1. Pressing a modifier key while holding a hotkey for something like PTT will abruptly stop the hotkey. This is certainly not desired behavior. For example if streaming a game and need to press CTRL to crouch while holding another key for PTT this would abruptly cut off the mic as soon as CTRL is pressed.

2. If I press a modifier key after pressing and holding the common key of a hotkey it still activates. If I understand your description correctly then this indeed conflicts with the fix proposed in Fix: Behavior of hotkeys more like other apps now #13363.

There seems to be at least a few issues with hotkeys that could maybe be further identified / specified and then fixed in a more comprehensive PR? I am willing to do future contributions and perform testing across several different operating systems and architectures.

[suogesi]

Thank you for testing! Here I want to declare the background:

• I only have Windows 10 computers. By that means I didn't and cannot test on Linux or macOS. (I can't even compile macOS version :P)
• I only tested hotkeys like "Start Recording" which is expected to be triggered when pressing down hotkey combination. I didn't know there are hotkeys like PTT.
• In my test, this PR do fix the issue on Windows 10.

And There are my opinions about your two issues:

First

PTT behaves like this is something... as expected. Noticing there is a global variable obs->hotkeys.strict_modifiers. This variable isn't initialized utill hotkey thread is created. And it should be always true. (If you look closer, it is initialized AFTER the thread is created. It is ALMOST a racing condition.) obs->hotkeys.strict_modifiers is finally used in input_match_modifiers: if it is true, modifiers must be exactly same as hotkey required. If it is false, modifiers only need to meet hotkey's requirement.

As you said, obs->hotkeys.strict_modifiers should be false when we use hotkeys like PTT. But sadly OBS doesn't do such thing. PTT in master version will also intterupted when pressing additional modifiers but it will recover when releasing them. I don't know why.

After all: Since there is a forever true variable obs->hotkeys.strict_modifiers, it works as expected because OBS never try to keep recording when holding PTT hotkey and pressing another modifier.

Secound

I believe different platforms have different keyboard event mechanicals. And OBS didn't do well about such thing for a long time. This PR do fix the issue on my machine. (Classic speaking.) We need to debug it more.

And yes, since there are so many problems about hotkeys. I assume this part just happend to be usable. I mean, do you even understand how handle_binding work? I don't anyway and I'm not surprise about it is borken. We need to go deeper.

[suogesi]

I'm free these days. If you wish, we can discuss this further. By the way, I don't know how do Linux and macOS deal with this. If you can help me test it, I'll be appreciate.

P.S. I'm using UTC+8.

[suogesi]

Just when I'm typing here, I find PTT doesn't work when I'm playing Delta Force but work when I'm playing CS2. There are brunch of issues about hotkeys.

[suogesi]

I think we are tricked. obs-hotkey.c is just for OBS not in focus. Even if I commented query_hotkeys at obs-hotkey.c, hotkey still can be triggered... for only one time.

Actually Qt will handle key events with QObject *CreateShortcutFilter() from frontend/OBSApp.cpp. Frontend finally calls obs_hotkey_inject_event to trigger a hotkey. They won't conflict because there is a mutex (what a software engineering!). If one of them cannot get this mutex, it will give up triggering a hotkey.

My suggestion is that we must merge their hotkeys fuction someday. However it is so complicated - we cannot just remove Qt part because it is needed for some other shotcuts. (e.g. Alt + C to cancel. This may cause other issues.) Of course, there are many problems about cross-platform.

[thomasza92]

I think we are tricked. obs-hotkey.c is just for OBS not in focus. Even if I commented query_hotkeys at obs-hotkey.c, hotkey still can be triggered... for only one time.

Actually Qt will handle key events with QObject *CreateShortcutFilter() from frontend/OBSApp.cpp. Frontend finally calls obs_hotkey_inject_event to trigger a hotkey. They won't conflict because there is a mutex (what a software engineering!). If one of them cannot get this mutex, it will give up triggering a hotkey.

My suggestion is that we must merge their hotkeys fuction someday. However it is so complicated - we cannot just remove Qt part because it is needed for some other shotcuts. (e.g. Alt + C to cancel. This may cause other issues.) Of course, there are many problems about cross-platform.

Interesting... Thanks for all the comments and insights. I will have time to take a look some more this upcoming weekend. If you would like, feel free to send me an email and perhaps we could organize a time to work on this in real time.

[suogesi]

Note: OBS processes hotkeys in different way when the windows is in focus. In CreateShortcutFilter from OBSApp.cpp, OBS watches QEvent::KeyPress and QEvent::KeyRelease. And (assume hotkeys are enabled in focus):

1. OBS tries to figure out if that event is for a dialog by checking:

• There is a dialog.
• This key event is a press event.
• The Key is Enter, ESC or Backspace.

2. Otherwise, save this key in a obs_key_combination_t. NOT to save modifiers.
3. Also save true in var pressed if this event is a press event.
4. Get and save all current modifiers in the obs_key_combination_t.
5. Send to obs_hotkey_inject_event!

Looks bad. Because obs_hotkey_inject_event might not expecting such params: pressed here stands for "We're triggering this hotkey." but frontend just send a param indicating whether the last event is a press event.