jellyfin / jellyfin-mpv-shim

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

Arrow keys setting (while not in menu) are overrided #120

Open ahmed2m opened 4 years ago

ahmed2m commented 4 years ago

Describe the bug I have these setting in my

down add volume -1
up add volume 1

But they seem to conflict with the seek_up and other keys in config.json file. I tried setting them to null but it doesn't fall back to the input.conf.

To Reproduce Steps to reproduce the behavior:

  1. Go to '~/.config/jellyfin-mpv-shim/config.json'
  2. set arrow configs to null
    "seek_down": null,
    "seek_left": null,
    "seek_right": null,
    "seek_up": null,
  3. Go to '~/.config/jellyfin-mpv-shim/input.json'
  4. add
    
    RIGHT osd-msg-bar seek  5 exact
    LEFT  osd-msg-bar seek -5 exact

down add volume -1 up add volume 1


4. All the arrow keys are non-functioning (except in menus because of `kb_menu_*` settings)

**Expected behavior**
Expected the binding to fall back to the input.conf setting 

**Desktop (please complete the following information):**
 - OS: Archlinux
 - Kernel: 5.8.6
ahmed2m commented 4 years ago

some kind of error pops up when I click on those seek keys.

ValueError: ('Invalid value for mpv parameter', -4, (<MpvHandle object at 0x7f17fa89a540>, <mpv.c_char_p_Array_2 object at 0x7f17a80471c0>))
2020-09-24 15:33:13,094 [   ERROR] mpv: main: Command seek: required argument target not set.
Traceback (most recent call last):
  File "/usr/lib/python3.8/site-packages/mpv.py", line 526, in _event_loop
    message_handlers[target](*args)
  File "/usr/lib/python3.8/site-packages/mpv.py", line 1110, in _handle_key_binding_message
    self._key_binding_handlers[binding_name](key_state, key_name, key_char)
  File "/usr/lib/python3.8/site-packages/mpv.py", line 1046, in wrapper
    fun()
  File "/usr/lib/python3.8/site-packages/jellyfin_mpv_shim/player.py", line 245, in menu_right
    self.seek(seektime)
  File "/usr/lib/python3.8/site-packages/jellyfin_mpv_shim/utils.py", line 46, in _synchronizer
    return func(self, *args, **kwargs)
  File "/usr/lib/python3.8/site-packages/jellyfin_mpv_shim/player.py", line 489, in seek
    self._player.command("seek", offset)
  File "/usr/lib/python3.8/site-packages/mpv.py", line 719, in command
    _mpv_command(self.handle, (c_char_p*len(args))(*args))
  File "/usr/lib/python3.8/site-packages/mpv.py", line 126, in raise_for_ec
    raise ex(ec, *args)
iwalton3 commented 4 years ago

That would be because these settings:

"seek_down": null,
"seek_left": null,
"seek_right": null,
"seek_up": null,

control the amount of time that the seek function uses. They do not control the mapped key binding. To disable arrow keys, unset or change the respective kb_menu config entries:

"kb_menu_left": null,
"kb_menu_right": null,
"kb_menu_up": null,
"kb_menu_down": null,

Please note that disabling these will also disable keyboard navigation in the menu. You can either use the mouse to navigate in the menu, or you can bind the keys to something else.

Additionally, the exceptions will be fixed in the next version due to stricter data validation. It will force the type to be an integer in the configuration for seek times to handle invalid configurations.

ahmed2m commented 4 years ago

What if I don't want to seek with the up and down keys, I want to control the volume. (That is the point of this ticket) I want to make the arrow keys do whatever I set in the input.conf not be linked to something in config.json

iwalton3 commented 4 years ago

Right now the dual-purpose arrow keys are one of the deficiencies of the configuration system. You either can use them for seeking and menu navigation, or for some other purpose that you configure through MPV.

iwalton3 commented 4 years ago

Feel free to re-open if you would like to suggest some alternative way of handling this configuration.

Eisa01 commented 3 years ago

Thanks for pointing me towards this issue. However, it is actually still not working. I have tried removing the mapping from the mentioned above as well:

"kb_menu_up": null,
"kb_menu_down": null,

And for some reason the mapping is not getting removed, I have also tried to map it to different buttons, but it sticks to up and down arrow keys

ahmed2m commented 3 years ago

@Eisa01 My current setup now is

    "kb_menu": "c",
    "kb_menu_down": "",
    "kb_menu_esc": "esc",
    "kb_menu_left": "",
    "kb_menu_ok": "enter",
    "kb_menu_right": "",
    "kb_menu_up": "",

And it works with having these in my input.conf

WHEEL_DOWN    add volume -2
WHEEL_UP   add volume 2

RIGHT osd-msg-bar seek  5 exact
LEFT  osd-msg-bar seek -5 exact

This however as Izzie stated made arrows non-functioning inside the menus so I use mouse for the menu.

Eisa01 commented 3 years ago

That is very helpful, I will test your workaround once I am back. Thanks Ahmed.

Although I hope that an official fix will be released by @iwalton3

iwalton3 commented 3 years ago

I'll re-open this, but I think it'll get handled when the UI rewrite happens. The two ways of handling this are a more robust keybind mapper and yielding all events through MPV (using script binds), and I don't really like the second one.

Eisa01 commented 3 years ago

Looking forward to it. Sadly the solution provided by @ahmed2m does not seem to work in Windows 10 OS. I am not even able to cancel mapping the "c" button to not show menu.