mpv-player / mpv

🎥 Command line video player
https://mpv.io
Other
28.66k stars 2.93k forks source link

command: add unload-input-conf command #15344

Open guidocella opened 1 day ago

guidocella commented 1 day ago

Add a command to remove bindings previously loaded with load-input-conf. This lets you load and then remove an input.conf without relying on deprecated input sections - at least externally, internally it still uses input sections, but this can be changed later without changing the API. This is mainly useful to load different key bindings for images and then restore the regular key bindings when switching to a video.

First, we need to assign key bindings loaded with a load-input-conf to a new input section instead of the default one, because bind_keys() permanently deallocates bindings in the same input section with the same key. This way if you bind a key in input.conf and load and unload another config file which binds the same key, the input.conf binding is restored. enable_section() is split from mp_input_enable_section() to avoid recursive locking.

The input section name is the path of the config file. It is not normalized to not break ~ expansion in parse_config_file().

unload-input-conf then deletes the input section whose name is the path argument. It is basically like the disable-section command. In theory you can also disable other input sections with unload-input-conf, but they are unlikely to clash with filenames with slashes and dots in practice, and it is not worth implementing much validation if we plan to discard input section code later.

kasper93 commented 1 day ago

I'm conflicted about this. Whole unload-input-conf seems like a convoluted way for the simple goal of having multiple input bindings layers.

guidocella commented 1 day ago

I would be fine with undeprecating input sections but it was rejected in https://github.com/mpv-player/mpv/pull/14050. So I wrote this as an alternative.

kasper93 commented 1 day ago

I would be fine with undeprecating input sections but it was rejected in https://github.com/mpv-player/mpv/pull/14050. So I wrote this as an alternative.

I feel like it is just dancing around this input sections depreciation, achieving the same goal, but with new, "non-deprecated" way.

cc @na-na-hi: Do you think this is better way than exposing input-bindings directly to scripts?

na-na-hi commented 17 hours ago

cc @na-na-hi: Do you think this is better way than exposing input-bindings directly to scripts?

I think this PR is exactly input sections in a different name, which certainly doesn't help reducing the complexity of input code.

The current keybind mechanism requiring libmpv clients to use define/enable/disable section commands just to enable and disable key bindings, and all the needed code in mpv to handle them, is cumbersome. It was designed to be used by the player and not the scripts.

Instead, in the future it's better to just forward all input events to clients and let clients handle them, so the key bindings can be controlled by the libmpv clients without needing to interact with mpv. This is how inputs of all window systems (Win32/X11/Wayland/...) work, and how mouse and touch positions work currently. This also makes handling text input much more sensible without depending on hacks like ANY_UNICODE.

guidocella commented 9 hours ago

How would clients and mpv itself know when not to react to key bindings when only another client should handle them, e.g. not to process key bindings while console is open?

na-na-hi commented 4 hours ago

How would clients and mpv itself know when not to react to key bindings when only another client should handle them, e.g. not to process key bindings while console is open?

A client can issue a command to make it the "current focused client" and capture all inputs. Once it's done it releases the capture. It would be simpler to implement than input section since mpv only needs to track which clients are requesting focus and not keybinds.

guidocella commented 4 hours ago

So e.g. stats which wants to capture only up and down arrows will disable all other key bindings instead?