[MPlayer-dev-eng] [PATCH] double click to switch to full screen in GUI

Reimar Döffinger Reimar.Doeffinger at stud.uni-karlsruhe.de
Thu Oct 19 22:40:24 CEST 2006


Hello,
On Wed, Oct 18, 2006 at 09:35:20PM +0200, laurent wozniak wrote:
[...]
> And I understand you can be not motivated by a patch.

Main problem is that current code is, as you obviously noticed, a bit
confusing, partly because of inconsistencies.
I made this patch mainly for two reasons:
1) to help me understand the code and the pitfalls when implementing
this feature.
2) to help focus this discussion again to one thing. It is great that
you want to fix all these issues, but from experience discussing more
than one thing usually leads to none being applied.

> But the problem with this approach is that we keep in the code all the 
> bugs I've identified and corrected.

Point taken, but: doubleclick support is a feature that is requested
every now or then, the bugs on the other hand so far AFAICT have not
ever been reported. Some of the behaviour caused by them might be
considered a feature, so they should be checked properly and seperately.

[...]
> >Now I still have one problem with it: I think it will interact badly
> >with windows vos, which get the doubleclick events from windows
> >directly.
> >How to fix that?
> >A quick hack if this somewhat incomplete patch should be part of rc1
> >would be to put it under
> >#if !defined(__MINGW32__) && !defined(__CYGWIN__)
> >  
> I'm not sure it's a good idea to start hacking when introducing a new 
> feature.

Obviously I don't like it either. But how else to do it? Renaming
mplayer_put_key_internal to mplayer_put_key_norprocess or some such and
use that in the vos that provide double clicks would be possible, but I
don't like it much either.

[...]
> Moreover building a new feature on top of a part which has already 
> identified bugs will make correcting those bugs even more difficult.

As long as it does not rely on the not necessarily. I can even simply
things since it may provide a technical reason to fix things one way and
not another (e.g. this patch adds the requirement that every mouse button
creates both a down and an up event), thus avoiding lengthy discussions
later (yeah, "sneaking" such argumentation helpers into code is not
really too nice, but to my defense I have to say I only implemented it
this way because this is how I want doubleclicking to behave).

> I think we should wait,
> maybe let pass this release,
> and take time to discuss for a solution that sounds good to everyone.

That's a decision I'll leave to those who care about the feature itself
(i.e. not me).

Greetings,
Reimar Döffinger



More information about the MPlayer-dev-eng mailing list