mpv-player / mpv

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

Keyboard shortcuts work correctly only when using English layout #351

Closed MadFishTheOne closed 9 years ago

MadFishTheOne commented 10 years ago

Shortcuts should not depend on current layout.

Layout-independent key codes can be obtained from:

  1. event.xkey.keycode on X11.
  2. [event keyCode] on Cocoa.
  3. wParam on Windows.
pigoz commented 10 years ago

Wouldn't that be confusing? Also, I generally use US layout even on i.e. Italian keyboards at work (because companies buy PCs for the masses), wouldn't this feature troll me in that scenario?

Nikoli commented 10 years ago

In most GUI software keyboard shortcuts do not depend on layout, switching layout for using player is annoying. Please add this feature.

ghost commented 10 years ago

This was already discussed here: https://github.com/mpv-player/mpv/issues/45

MadFishTheOne commented 10 years ago

This is mostly for non-latin layouts. Specifically Russian in my case. With such layouts, necessary letters are nowhere to be found. This is unlike Italian, that uses the same letters (maybe just in different places). Since guessing whether layout is latin or not is not practical, I would suggest a user option for this.

Also on Cocoa layout switching for some reason does not work when keyboard focus is in mpv. So it is triple-pain: switch away, switch layout, switch back.

MadFishTheOne commented 10 years ago

This was already discussed here: #45

I've read through discussion, but I don't agree with the conclusion:

it's not generally possible to be independent of keyboard layout, no matter what you do it will always cuse inconveniences in one way or another.

It is definitely possible, as SDL (or any other similar library) does that exact thing for games. In fact it provides both layout-independent code and layout-dependent one AFAIK. I would recommend checking the code there for specifics.

For mpv, I would recommend a user option switch. So that both latin keyboard users with non-English-weird-key-positions, and non-latin users (I am only aware of Cyrillic) can be happy.

Zehkul commented 10 years ago

As an obviously highly biased Neo user I agree. I’m always pleased when games and programs manage to cope with my layout.

But: In (well written) games and other applications you generally assign actions to positions on the keyboard, what would be a non confusing way to do this in a config file? Letter → position of it on the US layout → letter on the corresponding position of your layout is kinda dumb. And really confusing. Not nearly as confusing as having to type scancodes though. Some kind of layout switching script could work though, convert all hotkeys to the active layout or something. I’m pretty sure some program (a tiling WM?) does this, I just can’t remember which one…

ghost commented 10 years ago

For mpv, I would recommend a user option switch. So that both latin keyboard users with non-English-weird-key-positions, and non-latin users (I am only aware of Cyrillic) can be happy.

It already does: --input-conf=yourcyrillicinput.conf

ghost commented 10 years ago

Some kind of layout switching script could work though, convert all hotkeys to the active layout or something. I’m pretty sure some program (a tiling WM?) does this, I just can’t remember which one…

There are programs that can switch your keyboard layout depending on what application's window is focused.

MadFishTheOne commented 10 years ago

It already does: --input-conf=yourcyrillicinput.conf

You miss (entirely) the point of this issue. Imagine the situation: two keyboard layouts on the system - English and Russian, and user switches between them. In current situation no matter what bindings you specify, one of the layouts won't work. And that is frustrating.

ghost commented 10 years ago

Then use a keyboard layout switcher.

These scancodes are not portable and you'd need huge lookup tables. And in the end you'd bother everyone with weird crap like the 'Z' binding actually mapping to the 'Y' key on German keyboard. Everyone would have trouble finding the correct symbol to bind. I don't see how that's an advantage.

MadFishTheOne commented 10 years ago

These scancodes are not portable and you'd need huge lookup tables.

They are portable to the extent we should care about (they will work on any PC with a keyboard). For implementation specifics, check SDL (or others) - they should have figured portability already, and SDL is widely used.

And in the end you'd bother everyone with weird crap like the 'Z' binding actually mapping to the 'Y' key on German keyboard. Everyone would have trouble finding the correct symbol to bind.

That's why I recommended it to be a user option. To not bother people with weird crap.

I don't see how that's an advantage.

Advantage is for multi-layout users, that would like the same key to trigger shortcut regardless of what layout was current at the moment.

ghost commented 10 years ago

For implementation specifics, check SDL (or others) - they should have figured portability already

Yes, and I've seen a very big bunch of dirty hacks.

That's why I recommended it to be a user option.

Making it an option would require even more code. (You'd have to keep the code for old method and the new one.)

Advantage is for multi-layout users, that would like the same key to trigger shortcut regardless of what layout was current at the moment.

Just use a program that switches the keyboard layout based on the currently focused application window. This is much simpler, saves us a lot of trouble, and avoids bloat in mpv.

MadFishTheOne commented 10 years ago

Few more points for completeness:

  1. Getting layout-independent mappings is only a potential problem on X11 ("mechanism, not policy" yay! where the most common case is handled as the special case). Both OS X and Windows are more defined in that regard and getting it to work there is rather trivial.
  2. Most other software has layout-independent bindings. Ctrl+C Ctrl+V works everywhere regardless of layout.
  3. Useful (for some people) feature is not necessarily bloat.

Besides, this:

Just use a program that switches the keyboard layout based on the currently focused application window.

forces user to a specific workflow (per-window layout) which might not necessarily be what the user wants (in fact I prefer global layout).

ghost commented 10 years ago

If someone wants to implement it (in a non-intrusive enough way) I wouldn't be opposed to it. But personally I don't want to work on this. Probably nothing will happen into this direction, so I'm closing this for now.

forces user to a specific workflow (per-window layout) which might not necessarily be what the user wants (in fact I prefer global layout).

You could configure it to switch the layout for mpv, but not any other application.

otommod commented 10 years ago

Checkout xkb-switch !

MadFishTheOne commented 10 years ago

I would suggest leaving the issue open, so that people can find it. Closing an issue means "this user story is resolved" (fixed, or would-not-fixed). However, that is not really the case here.

ghost commented 10 years ago

Having too many issues open is a problem for me, because I can't see which things need work and for which are resolved.

ghost commented 10 years ago

I can reopen it, if you take offense.

I won't work on this, though.

divVerent commented 10 years ago

There are three issues if we made the controls scancode-aware:

  1. mpv also runs on a terminal, e.g. when playing music. And we cannot get scan codes there.
  2. Documentation - we simply cannot describe such a mapping with text. A picture of a keyboard with is assignments would work, but can not be put on a manpage.
  3. input.conf becomes a mess.
Nikoli commented 10 years ago

2 and 3 do not exist: input.conf will still use english letters.

divVerent commented 10 years ago

That IS the issue. When the doc says press Y but you have to press Z, that is bad. In the conf it may be acceptable, in the documentation sure not. Am 22.11.2013 21:14 schrieb "Nikoli" notifications@github.com:

2 and 3 do not exist: input.conf will still use english letters.

— Reply to this email directly or view it on GitHubhttps://github.com/mpv-player/mpv/issues/351#issuecomment-29105441 .

Nikoli commented 10 years ago

As user i press button with both 'm' and 'ь', but software may interpret it as 'm', 'ь' or 'cycle mute', etc depending on circumstances. The idea is to make mpv layout agnostic.

divVerent commented 10 years ago

As end user, if the docs say press Y - why do you expect me to know that is Z on my keyboard? Not everyone knows US layout. Am 22.11.2013 22:04 schrieb "Nikoli" notifications@github.com:

As user i press button with both 'm' and 'Ø', but software may interpret it as 'm', 'Ø' or 'cycle mute', etc depending on circumstances. The idea is to make mpv layout agnostic.

Reply to this email directly or view it on GitHubhttps://github.com/mpv-player/mpv/issues/351#issuecomment-29109182 .

divVerent commented 10 years ago

Adding: the only fix I can see is a layout picture, instead of saying "the key Y does ...".

But how to fit that in the manpage?

ghost commented 10 years ago

Yep, it would be very strange if the manpage says "Key Y does action A" and "Z does action B", but then the actions are apparently swapped on the keyboard. And then the user tries the terminal, and it's acting as the manpage documents. That must be pretty confusing. Maybe we'd then get feature requests demanding to reverse-map terminal input as if it used the other layout...

Anyway, I think MadFishTheOne suggested that the "layout independent" way shouldn't be default, just that there should be a switch to enable it.

And again, I don't see how that's better than creating a Russian input.conf, which had to be there additionally to the English bindings to make sure it works with both keyboard layouts.

divVerent commented 10 years ago

As for Russian: why not just add Cyrillic bindings to our default config? And Greek too, maybe.

Only issue: there are two common Cyrillic layouts. My wife uses the non phonetic one; however, does it make more sense to map keys to same location or phonetic equivalence to Russian end users?

As for reverse mapping on the terminal: forget it. Too complex due to handling shift-digit, Ctrl and such stuff.

So I suggest: for now let's make it an option, handled by each vo event handler back end separately.

In the future: make audio use a vo window too (then only non-window vo like lavc, null will trigger the terminal handling). And in these operation modes, the only key you really need is Q.

As for the manpage: one idea might be to include ascii art of US layout, and referring to keys by the US layout names. As we would only include one keymap diagram, we then have to be careful to e.g. write Shift-2 and not @.

BTW: on windows, editing shortcuts are NOT layout independent, as opposed to what someone here wrote. Undo key (Ctrl-Z) e.g. moves between German and US layout. I would rather think Windows Cyrillic layouts might send Latin letter keys when combined with Ctrl/Alt - either that, or Windows applications/controls define shortcuts per language (MS Word e.g. does, German bold is Ctrl-F and not Ctrl-B despite these keys being identical between US and German layout). Any windows developer here?

ghost commented 10 years ago

In the future: make audio use a vo window too (then only non-window vo like lavc, null will trigger the terminal handling). And in these operation modes, the only key you really need is Q.

Like with --force-window? I disagree that terminal handling doesn't matter, though. I often use the terminal controls myself, even for video.

MadFishTheOne commented 10 years ago

RFC patch: MadFishTheOne/mpv@fb930105e4e52fd6be7c592533180d01f6c605c1

Adds --keyboard-layout-independent option and implements it for Cocoa. Not sure really about that whole options passing thing. Summoning pigoz to review that part.

pigoz commented 10 years ago

Maybe make it an input option instead of a vo option? Like media_keys for instance. From the vo you can access the input context anyway and you could have opaque accessors for this config. Not sure what @wm4 thinks.

MadFishTheOne commented 10 years ago

So you mean storing the flag in input_ctx, and providing an accessor function like

bool mp_input_is_keyboard_layout_independent(const struct input_ctx *ictx);

?

Seems like a valid approach to me (less intrusive than what I did).

pigoz commented 10 years ago

yes, that's what I had in mind.

MadFishTheOne commented 10 years ago

Updated version available at: MadFishTheOne/mpv@db52878df74486b35793be3ca30b554ae41ec525

This should be of mergeable quality now, hopefully.

ghost commented 10 years ago

IMO looks really ugly. The option name isn't good. This is not keyboard layout indepdendent, it's layout dependent, because now it depends on the physical layout of the keyboard what you get. Also, apparently it uses the US layout, so there's nothing "independent" about it.

And I really wonder why this should be in a video player. We're not even a game engine.

What's with the shift table?

MadFishTheOne commented 10 years ago

Basically the option means "interpret keyboard events as if they happened with English US layout, regardless of what layout you actually use". Which in turn means that your input.conf works with whatever layout without changes (as if it was specified for English US). English US is picked as a sensible default one. That is the layout most people with this issue would use to input English letters.

I don't care much about how the option is named, so why not suggest a better name?

Shift table is needed, because, for example, keybindings for 3 and # are different things.

ghost commented 10 years ago

So I heard OSX has native per-application keyboard layouts or similar. This option should be explored first. E.g. is there a mechanism to make OSX always use US keyboard layout for mpv? Or if not, can mpv request this layout? This would be much better than stupid hardcoded tables.

I don't care much about how the option is named, so why not suggest a better name?

Maybe force-us-keyboard-layout or something.

Shift table is needed, because, for example, keybindings for 3 and # are different things.

What about bindings with Alt or Ctrl?

pigoz commented 10 years ago

So I heard OSX has native per-application keyboard layouts or similar. This option should be explored first. E.g. is there a mechanism to make OSX always use US keyboard layout for mpv? Or if not, can mpv request this layout? This would be much better than stupid hardcoded tables.

Apparently it is per-document. mpv is not document based though. Maybe there is some way to get this to work correctly in mpv though.

MadFishTheOne commented 10 years ago

What about bindings with Alt or Ctrl?

Keyboard modifiers are still passed through, just as before (did you even look at the code? sigh). Shift is handled specially only because we actually specify bindings like #, and not Shift+3. That's why there is a translation table to know what symbol is Shift+3.

Maybe force-us-keyboard-layout or something.

keyboard-layout-indepedent was stating the intention, and force-us-keyboard-layout is stating the implementation detail. I would say that intention is more important, but this is all bikeshedding, so whatever. As I said before, I don't care much.

So I heard OSX has native per-application keyboard layouts or similar.

Unless there is an API for an application to declare desired keyboard layout (which I doubt there is), this is still a workaround territory, and does not truly solve the issue.

divVerent commented 10 years ago

I think the point is rather that we see the current behavior as layout independent and the suggested one as layout dependent.

Because: if the manpage says Y and the user has to first map that to US layout to know which key is actually meant, this mapping depends on the user's layout - e.g. a German user has to press Z when the manpage would say Y. It makes sense to call THAT layout dependent.

That is why I oppose both namings "layout dependent" and "layout independent" - both interpretations make sense and thus it's a bad option name if it means one thing to some users and the exact opposite to others.

ghost commented 10 years ago

What's the status on this? Not that I want it in.

MadFishTheOne commented 10 years ago

I was under impression that the status was precisely that "you don't want it in". Hence technical discussion ceased as pointless.

If you've changed your mind, patch is still there. And you can change option name to whatever you like (as that seems to be the only actual objection).

ghost commented 10 years ago

I was under impression that the status was precisely that "you don't want it in".

I'm against it, but if there are enough people who want it, you can get it in.

Jessidhia commented 10 years ago

A possible workaround I was considering is to have a script in TOOLS that a user can run to translate the input.conf between different keyboard layouts; that way, you could have an input.conf that had the keys in their intended positions (such as 'z' and 'x' for subtitle delay actually being together in the lower left of the keyboard), or an input.conf that actually works at all on non-latin keyboards.

This, of course, doesn't solve the issue that would be fixed by using hardware scancodes, but that is a different can of worms (such as what "layout-dependent" means as pointed out by @divVerent).

ghost commented 10 years ago

Here's another idea I just had: add a translation table of unicode characters, so if you press a key, and it produces a character mapped by it, then use the translated character to look up input.conf.

Should make the russians happy and doesn't require anything special in the backend input code.

Jessidhia commented 10 years ago

The problem with that is that there are 2 commonly used russian layouts (phonetic and standard) and they're completely different between themselves...

Nikoli commented 10 years ago

The problem with that is that there are 2 commonly used russian layouts

What do you mean? Only qwerty/йцукен is commonly used, it is very hard aka almost impossible to buy device with non йцукен layout.

Jessidhia commented 10 years ago

I was thinking of QWERTY layouts that wrote the "phonetic equivalent" cyrillic letter (e.g. "T" in QWERTY would write "Ñ‚" in cyrillic), but I guess that such layouts are more used by non-native speakers than actual russians.

ghost commented 10 years ago

So if there's only 1 common layout, would my suggestion work or not?

Here's another idea I just had: add a translation table of unicode characters, so if you press a key, and it produces a character mapped by it, then use the translated character to look up input.conf.

Nikoli commented 10 years ago

It should work. I did not check, but isn't such table already provided by X11, input drivers or some other low level software?

ghost commented 10 years ago

So, can anyone make such a table? Then I can implement it. Something like a text file with the cyrillic character in the first column, and the ASCII one in the second column (according to the keyboard layout).

but isn't such table already provided by X11, input drivers or some other low level software?

Probably, but I expect it to be nearly inaccessible, or only accessible with a lot of pain.

Nikoli commented 10 years ago

May be this will help: https://qt.gitorious.org/qt/qt/source/0c03af0d4d928bdbb32b09eedb1dba3ce59e5278:src/gui/kernel/qkeymapper_x11.cpp

https://bugreports.qt-project.org/browse/QTBUG-32908