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

[Alban Bedel]

On Sun, 30 Oct 2005 09:30:17 +0100
Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de> wrote:

> 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?

Huh ??? What do you mean with real problem ? For me the real problem is
that some vo muck up with the key in random, illogical and undocumented ways.
This lead to inconsistent behaviour which can only be traced by rtfs.
However i like to keep backwared compatibility, hence i think it's a good
idea to still have O, M and P act like o, m and p by default.

> > > 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.

Sure originaly it's only there for keys. But i really fail to see what's
your pb with that. Just bcs of the function name ?
Try to think about reusing libvo outside of mplayer. Having to provide
an api compatible with libinput just to use libvo is insane.

> > > 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.

i don't see the pb here. Both are simple events and imho an int is really
enouth to pass that information. If you really want, rewrite all vo to use
a mplayer_put_event() function, use a nice struct/union were key, mouse and
wm events can be "splited". Nice, but imho it would be only cosmetics.
As in the end you would have to bind these "non-key events" with commands
anyway. So instead of adding useless complexity why not just use the system
already in place for keys ?
 
> > > 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.

I think you are missing the subtle difference: the difference between telling
what happened and what should be done about it. That's the whole point
of libinput, putting the user in control of the event/action translation.
In the case of CLOSE_WIN i can imagine that some might want to bind
"pt_step 1 1" instead of "quit".

> > 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.

Yep the doc could be updated to mention that mplayer_put_key() is used to pass
back a few other events than just keys.

	Albeu