jellyfin / jellyfin-mpv-shim

MPV Cast Client for Jellyfin
Other
1.59k stars 93 forks source link

Hold Arrow Keys for Seeking #176

Open Eisa01 opened 3 years ago

Eisa01 commented 3 years ago

Describe the bug The currently implemented seeking does not have the ability to hold the arrow < > keys for seeking to keep on going forward or backward. The action is not repeatable and the keys have to be pressed multiple times in order to seek. mpv player default seeking behaves differently and offers holding down arrow keys.

To Reproduce Simply hold down the arrow < > keys and it will only execute once. Expected behavior Holding down the arrow < > keys will keep on seeking until the keys are released.

Desktop (please complete the following information):

iwalton3 commented 3 years ago

As stated elsewhere the MPV keybind API doesn't support this outside of lua. SyncPlay also means it is still a good idea to get between these keystrokes too, so it would likely need to be supported with a lua script.

Eisa01 commented 3 years ago

@iwalton3 Perhaps we can move away from python keybinding where possible, and apply script keybinding by sending script-messages. I used the capture_mouse event instead of making a new event. Hence the rename in PR. This script solves this issue, as well as #120.

I was confused into which method I should use to allow keybinds customization. I chose input.conf and chose unfamilar keybinds that links to the script. (You can simply change the function of keys inside input.conf) inside input.conf

#mapping of arrow keys for jellyfin mpv player
ctrl+shift+alt+UP add volume 2
ctrl+shift+alt+DOWN add volume -2
ctrl+shift+alt+RIGHT osd-msg-bar seek 5
ctrl+shift+alt+LEFT osd-msg-bar seek -5

In the mpv.conf added a link for the script

--script="/home/username/.config/jellyfin-mpv-shim/scripts/shim-companion.lua"

shim-companion.lua script

function up_key()
    mp.command('keypress ctrl+shift+alt+UP')
end

function down_key()
    mp.command('keypress ctrl+shift+alt+DOWN')
end

function right_key()
    mp.command('keypress ctrl+shift+alt+RIGHT')
end

function left_key()
    mp.command('keypress ctrl+shift+alt+LEFT')
end

function menu_disabled_arrow_keys()
    mp.add_forced_key_binding('UP', 'up_key', up_key, "repeatable")
    mp.add_forced_key_binding('DOWN', 'down_key', down_key, "repeatable")
    mp.add_forced_key_binding('RIGHT', 'right_key', right_key, "repeatable")
    mp.add_forced_key_binding('LEFT', 'left_key', left_key, "repeatable")
end

function menu_enabled_arrow_keys()
    mp.remove_key_binding('up_key')
    mp.remove_key_binding('down_key')
    mp.remove_key_binding('right_key')
    mp.remove_key_binding('left_key')
end

function shim_companion_handler(event)
    if event["args"][1] == "shim-menu-enable"
    then
        if event["args"][2] == "True"
        then
            mp.log("info", "Enabled shim menu events.")
            menu_enabled_arrow_keys()
        else
            mp.log("info", "Disabled shim menu events.")
            menu_disabled_arrow_keys()
        end
    end
end

mp.register_event("client-message", shim_companion_handler)
mp.register_event('file-loaded', menu_disabled_arrow_keys)

I believe shipping mpv.conf and input.conf with pre-configurations could be a good solution.

iwalton3 commented 3 years ago

I believe shipping mpv.conf and input.conf with pre-configurations could be a good solution.

The problem with doing this is it breaks it for all existing users and you cannot add the ability to configure it easily in the UI in the future without writing a parser/writer for MPV's config. I'm unsure of an easy solution to this, although I will say it is likely in the future that the QT UI will handle keybinds outside of MPV and likely forward in unknown bindings into MPV. But that's a long way off.

Eisa01 commented 3 years ago

@iwalton3 Sadly I knew that input.conf and mpv.conf approach will force the changes on existing users and will discard arrow key changes made inside conf.json. But I still thought input.conf would be the best approach since it works beautifully, and it is the native keybind handler of mpv. And that it will trigger an idea so you do something about it once I paste it 💯

Side note I also made changes to the default OSC so that it does not break when closing when border=no. So hovering shows, close button, minimize button, etc, in fullscreen. This makes the UI of jellyfin-mpv player (feels and looks good when launched from Jellyfin website and Jellyfin Desktop).

Yet this requires binding the OSC script to mpv.conf and disabling osc from config.json (to avoid UI conflict). The only problem again, it will break the option of enable_osc from config.json file.

The same challenge that is confusing me keeps on appearing on any changes I make :/

iwalton3 commented 3 years ago

I think sending a serialized config object from python to the script could be a solution.

iwalton3 commented 3 years ago

But whatever solution it is it absolutely cannot break backwards compatibility. Only a fraction of users touch the MPV config and many would probably be confused by it if I said in release notes that they had to change it.

This of course means looking for an empty config is a valid solution, but dealing with changes is still an issue. Hence why I would like to keep defaults in the conf.json that can be updated.

Eisa01 commented 3 years ago

@iwalton3 I like the idea of serialized, seems an approach that could work

Also, what do you think of a temporary option to redirect config to input.conf? redirect_arrow_keys: true, false

When enabled, it would show that the arrow keys is disabled from the menu (and it can grey it out from gui).

And in config.json the value will be "disabled" for arrow keys. Something like that

This idea is similar to advanced options for experts, etc.

iwalton3 commented 3 years ago

Of course that is fine. One other consideration I thought of is not all mpv builds have Lua so a fallback is needed.

user18081972 commented 1 year ago

@iwalton3 Perhaps we can move away from python keybinding where possible, and apply script keybinding by sending script-messages. I used the capture_mouse event instead of making a new event. Hence the rename in PR. This script solves this issue, as well as #120. ( ... ) I believe shipping mpv.conf and input.conf with pre-configurations could be a good solution.

thanks for the script, works great for me. i dont use syncplay so theres no issue for me there.