[MPlayer-dev-eng] [PATCH] Remove osd menu hardcoded keybindings, support custom keybindings.

[Alban Bedel]

On Sun, 2 Dec 2007 19:54:57 +0800
Ulion <ulion2002 at gmail.com> wrote:

> 2007/12/2, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> > Hello,
> > On Sun, Dec 02, 2007 at 08:50:09AM +0100, Alban Bedel wrote:
> > > On Sat, 1 Dec 2007 14:18:11 +0800
> > > Ulion <ulion2002 at gmail.com> wrote:
> > >
> > > > Hello,
> > > >
> > > > There are keybindings in libmenu for browse osd menu, but it's
> > > > hardcoded, can not be customized.
> > > > This patch remove the hardcoded keybindings, and support user
> > > > defined keybindings from osd menu  browsering.
> > > > As we known, each input key has only one possible binding
> > > > defined input.conf, such as arrow up, down, left, and right,
> > > > but when browsing osd menu, you probably want to use these
> > > > keys, then they are hardcoded in libmenu. But for users want to
> > > > use other key to browse osd menu, there's no way to custom
> > > > browsing keybindings, so I made this patch fix the problem.
> > >
> > > Good idea, the patch seems fine. But why do you change the
> > > read_key() return type from void to int? afaict from the patch
> > > it's useless, but as it's mixed changes that's hard to say. If
> > > you *realy* need to change the prototype then do it in another
> > > patch.
> 
> Change read_key() to int is for the next step: left un-handled keys to
> input instead of steal all of them.  Currently it can be void, I will
> remove it for a clean patch.

You mean having keys not handled by the menu be handled as if there was
no menu? I don't think that's a good idea. The normal key binding is
too crazy to have that as a fallback.

> >
> > The same question applies as to why the menu_def_t typedef is
> > removed. Makes me think if maybe the patch is a bi too big for what
> > it tries to do and at least should be split.
> 
> That's needed by keybindings' code, current menu struct has no type
> info, the typedef is moved to the header file to add the type info
> into menu struct so we can get menu's type name and menu's name.
> But I can split this little part as a standlone patch and apply it
> first to simply the main patch code.
> 
> Here's the patch after cleanup. Other change: change menu cmd name
> from macro name to lower case such like 'menu up', 'menu down'.

I have been thinking about this a bit more. It can be made in a simpler
way. On a keypress, when there is a bind it should just be queued as a
normal mplayer command. If no bind is found the keypress is passed
to the menu.

+<keybindings name="cmdlist" parent="list" />

Hmm, 'name' is not good here, it should be 'menu-type' or something
like this, that's what it is.

	Albeu