[MPlayer-dev-eng] [PATCH] Fix for broken vo_xmga support in GUI mode

[Dirk Porezag]

Reimar, I completely understand your problem with people like me who are trying 
to fix issues in code they don't completely understand, for this reason I'm 
grateful for your explanations. I've been mostly comparing older versions that 
worked for me with the current code and trying to find the places where I can 
tweak the new code to provide the old behavior. But this is not necessarily the 
right strategy if a lot of cleanup and restructuring action has been going on. 
Since I know this now I'll rather try to fix the individual vo_xmga instead of 
changing code that is shared with many other VOs.

As for your question: with "-fs" the current version doesn't even initialize in 
fullscreen mode with vo_xmga. But is also doesn't with vo_x11 or vo_xv! At least 
for me. Can you confirm this? The SVN version as of 2010-05-16 had it working 
for vo_xv and vo_xmga but not for vo_x11.

At first sight, vo_dwidth etc. is initialized in libvo/vo_xmga.c:config(), also 
I'd expect that the first call to libvo/x11_common.c:vo_x11_update_geometry() 
should take care of setting these variables if they are not initialized 
directly. Looks like this needs some further digging.

Maybe I'll stop using vo_*mga completely since the vo_xv which should be used 
much more widely gives me equally fine results with just slightly more CPU load. 
It's much more desirable to have a general standard like the XVideo extension 
implemented in the X11 driver rather that fiddling around with specific hardware 
on the application layer.

Dirk


----- Ursprüngliche Mail ----
> Von: Reimar Döffinger <Reimar.Doeffinger at gmx.de>
> An: mplayer-dev-eng at mplayerhq.hu
> Gesendet: Sonntag, den 26. September 2010, 12:43:28 Uhr
> Betreff: Re: [MPlayer-dev-eng] [PATCH] Fix for broken vo_xmga support in GUI 
>mode
> 
> On Sun, Sep 26, 2010 at 03:08:11AM -0700, Dirk Porezag wrote:
> > > The  x11_common.c patch unfortunately is not ok, setting resize for a  
>movement
> > > is a bad idea and causes flickering and other issues with  other VOs.
> > > Maybe add a VO_EVENT_MOVE or so...
> > 
> > I  have a hard time verifying that the submitted patch actually causes the 
> >  flickering. While I agree that VO_EVENT_RESIZE may be a bit too much for a 
>move, 
>
> > the SVN version as of May 16th 2010 (last working version I used) did  
>exactly 
>
> > this. In fact, the patch just reinstates the behavior that was  present until 
>a 
>
> > few months ago! I don't see how flickering would be  caused by this patch 
>without 
>
> > having an actual change in geometry just  before 
> > libvo/x11_common.c:vo_x11_check_events() is  called
> 
> Because several vos (e.g. gl) need to reinitialize if the size is  changed.
> Using VO_EVENT_RESIZE would make every window movement cause a  completely
> useless reinitializtion.
> 
> > I have modified my patch to  only generate a VO_EVENT_EXPOSE which is 
>sufficient 
>
> > to get the correct  bevavior, at least for vo_xmga.
> 
> Great, now you are adding a bug to use  another bug to hide a third bug.
> Maintainability byebye.
> A window movement  event is _not_ an expose event (this is the bug you add).
> Treating it as such  will cause e.g. -vo gl to needless redraw the window on
> move in a composited  environment.
> vo_xmga has no need to and thus should not do anything at all on  a
> VO_EVENT_EXPOSE (this is the bug you use).
> vo_xmga needs to know when  the window was moved, which currently is not
> exposed (which is the bug you  hide).
> 
> > But IMHO the critical point 
> > here is not my change in  libvo/x11_common.c:check_resize(). Even without my 

> > patch I have  flickering in full screen mode. After digging a little deeper, 
>I 
>
> > have  found that my flickering problem is caused by calling check_resize 
>pretty 
>
> > much always before the main event processing loop in 
> >  libvo/x11_common.c:vo_x11_check_events():
> >  #-----------------------------
> > if (WinID > 0)
> >          ret |= check_resize();
> >  #-----------------------------
> > This line was introduced after May 16th  2010, I haven't yet checked when and 
>why 
>
> > but when I deactivate it, all  my flickering problems go away (but - as 
>already 
>
> > said - they are  already present if I use the current unpatched HEAD version 
>of 
>
> >  libvo/x11_common.c).
> 
> Possibly vo_xmga does not set vo_dwidth correctly in  fullscreen, in which case 
>it would
> get a resize event all the time,  reinitializing constantly.
> Does this happen when you start MPlayer with -fs,  when you switch to 
>fullscreen
> at runtime or in both cases.
> 
> >  Enclosed is my modified patch, I'm aware of the fact that this will probably 
>
> > need further discussion. I'll try to find the revision where the  changes 
> > discussed above were introduced, maybe this will shed some  light on the 
>whole 
>
> > issue.
> 
> It will only shed light on which  issues they fixed, but I doubt it will
> help much with fixing vo_xmga.
> A  lot of stuff in libvo/ was cleaned up, code deduplicated, behaviour
> made  consistent etc.
> Some of these changes need changes in the individual vos to  work 100%
> right, and vo_xmga was never fully updated because of the lack  of
> hardware to test any changes  properly.
> _______________________________________________
> MPlayer-dev-eng  mailing list
> MPlayer-dev-eng at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/mplayer-dev-eng
>