[MPlayer-dev-eng] Re: [PATCH] Cleanup the libvo/input hacks

[Reimar Döffinger]

Hi,
On Sun, Oct 30, 2005 at 02:09:57AM +0200, Alban Bedel wrote:
> On Sat, 29 Oct 2005 22:00:10 +0200
> Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de> wrote:
> > By making the hack of mapping 'O' and 'Q' that could be used free
> > before, too? This seems wrong to me. Why is this needed anyway?
> 
> Read more the code more carfully. Currently if you map a command to O
> and another to o, and type O you will have different behaviour if the
> key enter mplayer via the term or via the movie window :( Their is no
> reason for the vo to do such non sense hacks.

I agree with that. But I can't see why just moving the hack to the input
files is any better. And it seems it is only the vos using X11 that are broken,
so why can't we fix the real problem?

> > IMHO using
> > mplayer_put_key(KEY_CLOSE_WIN);
> > instead of
> > mp_input_queue_cmd(mp_input_parse_cmd("quit"));
> > is a giant hack.
> 
> Hmm, we probably don't have the same definition ;) imho the various
> parts of mplayer should be kept cleanly separated as far as it's possible.
> libvo already expect the "core" to provide mplayer_put_key(),
> why should it also need the whole libinput stuff ?

Because my understanding of mplayer_put_key is that it is there to pass
keypress events from the vo to the core, not just "any data" the vo
would like the core to know about.

> > This just is not a key event, it is some completely
> > different thing that should make MPlayer quit, and not maybe something
> > else the user happened to assign to that key.
> 
> That "key" only exist if one input (like a vo) return it. If you really
> want to prevent users from remapping that just remove the key name from
> the key name list, but i don't really see the point.

The point is, it munges two unrelated things: a real keypress, and some
other event, that might have all kinds of reason.

> > If you really think a input dependency is a problem, we can still introduce
> > a mplayer_put_command function.
> 
> Why that ? vo should be purely "passive", they have no reason to start
> telling the "core" what it should do. They should just pass events down,
> then the "core" decide what to do with them.

Well, even after the patch they are still "telling the core what it
should do", just that it is now obfuscated.

> Sure a better interface than a single int could be used, but currently
> it's enouth.

Well, I think it's a hack. My minimum requirement would be a bit of
documentation for mplayer_put_key and how it is used.

> > Also, how sure is it that the KEY_INTERN definition is colission-free?
> > Maybe I'm missing something, but currently this has a very bad smell for
> > me.
> 
> I think it's colission free, but yeah it's a bit messy. Perhaps we should
> merge keycodes.h, mouse.h, joystick.h and put some order in these defines.

I was thinking about colissions with system keycodes, but looks like
that's no problem.

Greetings,
Reimar Döffinger