Skip to content
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

Reduce reliance on external libraries / certain upstream packages are unmaintained #85

Open
ehfd opened this issue Aug 2, 2023 · 19 comments
Labels
enhancement New feature or request help wanted External contribution is required interface OS input, display, or audio interfaces

Comments

@ehfd
Copy link
Member

ehfd commented Aug 2, 2023

11006e3

As specified here, the upstream python-uinput package is not maintained for 6-7 years.

Might need a change in the uinput interface to a project that continues to update, or maintain a selkies-project fork.

Moreover, why can't we completely replace pynput.mouse.Controller() and pynput.keyboard.Controller() with Xlib.xtest?

Also, we need to update all the dependencies for JavaScript.

@ehfd ehfd added enhancement New feature or request help wanted External contribution is required interface OS input, display, or audio interfaces labels Aug 2, 2023
@ehfd
Copy link
Member Author

ehfd commented Sep 5, 2023

Merging with #90.

@ehfd ehfd closed this as completed Sep 5, 2023
@ehfd
Copy link
Member Author

ehfd commented Oct 16, 2023

We may prefer to replace python-uinput with python-evdev, as we have gotten rid of /dev/uinput dependencies.

As far as I see, our reliance on python-uinput is limited to uinput.BTN_LEFT, uinput.BTN_MIDDLE, uinput.BTN_RIGHT, uinput.REL_X, uinput.REL_Y, uinput.REL_WHEEL, in webrtc_input.py. We aren't creating a new virtual input device with /dev/uinput, and our current usage should be covered with python-evdev.

Moreover, why can't we completely replace pynput.mouse.Controller() and pynput.keyboard.Controller(), and just use Xlib.xtest like we did in:

# NOTE: the pynput mouse.move method moves the mouse relative to the current position using its internal tracked position.
# this does not work for relative motion where the input should just be a delta value.
# instead, send the X fake input directly.
xtest.fake_input(self.xdisplay, Xlib.X.MotionNotify, detail=True, root=Xlib.X.NONE, x=x, y=y)
self.xdisplay.sync()

@ehfd ehfd reopened this Oct 16, 2023
@ehfd ehfd changed the title python-uinput upstream package is unmaintained Reduce reliance on external libraries / python-uinput upstream package is unmaintained Oct 19, 2023
@ehfd ehfd changed the title Reduce reliance on external libraries / python-uinput upstream package is unmaintained Reduce reliance on external input libraries / python-uinput upstream package is unmaintained Oct 19, 2023
@ehfd ehfd removed the help wanted External contribution is required label Oct 19, 2023
@ehfd ehfd added the help wanted External contribution is required label Nov 10, 2023
@ehfd ehfd changed the title Reduce reliance on external input libraries / python-uinput upstream package is unmaintained Reduce reliance on external input libraries / certain upstream packages are unmaintained Nov 25, 2023
@ehfd
Copy link
Member Author

ehfd commented Nov 25, 2023

#113

python-xlib is also unmaintained by the maintainer.

@ehfd
Copy link
Member Author

ehfd commented Nov 25, 2023

I am generally not too happy with dependencies: python-uinput (dependency is effectively removed but still persists, plus it is unmaintained), pynput (capabilities can be replaced by directly using python-xlib), basicauth (capabilities are trivial and replaceable with default packages), gputil (unmaintained and easily replaceable with another package such as pynvml and gpustat).

python-xlib is also used extensively and there is no viable alternative, but the repository maintainer is not that responsive.

@ehfd
Copy link
Member Author

ehfd commented Nov 25, 2023

CC @cruizba

@ehfd ehfd changed the title Reduce reliance on external input libraries / certain upstream packages are unmaintained Reduce reliance on external libraries / certain upstream packages are unmaintained Nov 25, 2023
@cruizba
Copy link
Contributor

cruizba commented Dec 3, 2023

I think I found a possible way to fix all inconsistencies and errors related to the keyboard, potentially paving the way for the removal of pyinput and, possibly in the future, python-xlib. You can find my work in this branch of my fork: GitHub - cruizba/selkies-gstreamer.

Let me share the backstory of how I arrived to this.

Upon further investigation, I realized that certain keysyms which reach this section of webrtc_input.py, particularly those representing Spanish characters on my keyboard (like ñ), lacked corresponding keycodes. I've noticed that X server loads some generic keycodes, but it doesn’t cover all of them. This might relate to the default keyboard setting, which is typically a Generic 105-key PC, but I am not sure.

In a remote desktop environment, the server is unaware of the client's keyboard. Therefore, modifying the server's keyboard configuration seemed impractical as a solution to me...

To explore how other projects handle keyboard input remotely, I looked into Neko. Neko, along with Guacamole keyboard lib in the browser, manages keyboard events from the browser. When the keysym reaches the neko server, they process input data through a set of C functions in xorg.c, which they integrate with Go.

The key function of xorg.c is XKey(). It appears that XKey dynamically assigns KeySyms to Keycodes when there is no existing association. If a keysym is sent without a corresponding keycode, it registers this new mapping in the XOrg keyboard configuration (As far as I can understand from the code). This approach is also utilized in TigerVNC's source code for keyboard keys mapping!

Consequently, I've created xorg.py to integrate the build of xorg.c and changed the keyboard part of webrtc_input.py to utilize the new xorg class. This solution works exceptionally well! It detects all keyboard keys correctly without altering any remote keyboard settings and while using my own keyboard configuration.

The challenge was creating a new addon and compiling the xorg.so file to implement this functionality. The project now requires additional headers and libraries. However, the good news is that this addon could potentially replace python-xlib for mouse interactions and other X interactions.

If you're interested in testing it, my branch is fully functional. Simply run vscode with the devcontainer as usual and execute the task [run] re-build, re-install and run selkies-gstreamer. It's configured to compile and install the library.

@ehfd
Copy link
Member Author

ehfd commented Dec 4, 2023

This is insanely good. @cruizba
I am open to compiling libraries.
I had an issue where the '>' key was pushed as '<', and it was indeed an issue with the keyboard layout.

I would like to merge this ASAP. Could you help with the PR?

@ehfd
Copy link
Member Author

ehfd commented Dec 4, 2023

@cruizba
One other possible option is to start using cython. Then, xorg.c and xorg.h can be seamlessly integrated into the Python package.

@cruizba
Copy link
Contributor

cruizba commented Dec 4, 2023

I would like to merge this ASAP. Could you help with the PR?

Sure, I'll try to continue this weekend, I am doing this in my spare time.

I had an issue where the '>' key was pushed as '<', and it was indeed an issue with the keyboard layout.

Is this specific issue fixed? Did you test it?

cython

I will take a look. I am not a python expert, but I've written some things in the past. But I suppose that this let you write C code with python syntax, am I right?

Also, I want to optimize the xorg.c implemenation a little bit.

I see that the table of keycodes has a max of 255 keycodes: TigerVNC/tigervnc#93

I think if you change keyboard layouts frequently, the table fills entirely. So I was thinking about having an auxiliary map to save associations and then removing Key Codes just when a key up event arrives, so the table never fills.

@ehfd
Copy link
Member Author

ehfd commented Dec 4, 2023

@totaam

I would like to consult your opinion.

https://github.com/cruizba/selkies-gstreamer/tree/fix-inputs/addons/xorg-iface
https://github.com/cruizba/selkies-gstreamer/blob/fix-inputs/src/selkies_gstreamer/xorg.py

Would it be plausible to integrate this code into a wheel file? Asking because Xpra actively uses Cython.

@cruizba
Copy link
Contributor

cruizba commented Dec 4, 2023

I will try to rewrite the essential parts of the keyboard in Cython. I'm pretty sure it is possible to do.

@totaam
Copy link

totaam commented Dec 4, 2023

@ehfd
Seeing how few method calls you actually need, porting to Cython would be trivial.
However, wheels are portable, Cython modules are not.
Using python-xlib or a ctypes wrapper would be a portable solution.

@ehfd
Copy link
Member Author

ehfd commented Dec 4, 2023

Does XKey() exist in python-xlib?

Using an external .so file looks like the current approach. I would accept that with the Joystick Interposer because it needs to set LD_PRELOAD systemwide, but this is slightly outside my comfort zone for this purpose.

@ehfd
Copy link
Member Author

ehfd commented Dec 4, 2023

I had an issue where the '>' key was pushed as '<', and it was indeed an issue with the keyboard layout.

@cruizba

# With the Generic 105-key PC layout (default in Linux without a real keyboard), the key '<' is redirected to keycode 94
# Because keycode 94 with Shift pressed is instead the key '>', the keysym for '<' should instead be redirected to ','
# Although prevented in most cases, this fix may present issues in some keyboard layouts
if keysym == 60 and self.keyboard._display.keysym_to_keycode(keysym) == 94:
keysym = 44

@totaam
Copy link

totaam commented Dec 4, 2023

Does XKey() exist in python-xlib?

XKey is not an xlib function, so no.

.. this fix may present issues in some keyboard layouts

Hmm. Been there, got scars to prove it.
These kinds of hacks don't last long.

@cruizba
Copy link
Contributor

cruizba commented Dec 4, 2023

@ehfd

Does XKey() exist in python-xlib?
A ctypes wrapper is also totally fine.

No, files present at https://github.com/cruizba/selkies-gstreamer/tree/fix-inputs/addons/xorg-iface are just a simple module using x11 libs, it is not part of the X library.

But I can try to rewrite the xorg.c using Ctypes, instead of calling the xorg.so

@ehfd
Copy link
Member Author

ehfd commented Dec 4, 2023

Understood. Thank you all.

I will give you the collaborator role in Discord if you tag me. @cruizba

@ehfd
Copy link
Member Author

ehfd commented Jan 5, 2024

python-uinput was removed from the dependencies.

@ehfd
Copy link
Member Author

ehfd commented Jan 6, 2024

I could really receive help from someone who could update the breaking web interface. Old Vue and Vuetify are breaking a lot of capabilities. Basically a 2019-era antique.

Follow web dependencies up in #108.

This issue will be focused on backend dependencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted External contribution is required interface OS input, display, or audio interfaces
Projects
None yet
Development

No branches or pull requests

3 participants