[MPlayer-dev-eng] Bugreport/Patch: x11 video backend has wrong geometry assumptions

[Reimar Döffinger]

On 08.05.2013, at 01:53, Jens Stimpfle <debian at jstimpfle.de> wrote:

> I was bitten by a bug while doing some X window management.
> From my program:
> 
> static void handle_maprequest(XEvent *ev)
> {
>        ...
> 
>        /* For mplayer, these two lines must be swapped
>         * because it ignores any events up to the first
>         * MapNotify.
>         */
>        XMoveResizeWindow(dpy, w, p->x, p->y, p->w, p->h);
>        XMapWindow(dpy, w);
> }
> 
> static void handle_configurerequest(XEvent *ev)
> {
>        ...
> 
>        /* This does NOT cause a ConfigureNotify event if we already
>         * did a handle_maprequest() because the geometry doesn't change.
>         */
>        XMoveResizeWindow(dpy, w, p->x, p->y, p->w, p->h);
> }
> 
> Any events preceding the MapNotify are simply ignored by mplayer. In the
> situation above, as long as the geometry doesn't change after the first
> map, mplayer won't receive any further ConfigureNotify Events from the X
> server and thus output videos under wrong geometry assumptions.
> 
> If I swap the two lines from my handle_maprequest() function, the bug
> isn't triggered anymore (aside: it's not exactly clean to first map and
> then resize).
> 
> This patch works for me, and while not an X11 wizard I'm fairly confident it
> has no regressions:
> 
> 
> --- a/libvo/x11_common.c        2011-12-04 14:30:54.000000000 +0100
> +++ b/libvo/x11_common.c        2013-05-08 01:13:36.566182774 +0200
> @@ -1148,13 +1148,7 @@
>     if (!vo_border) vo_x11_decoration(mDisplay, vo_window, 0);
>     // map window
>     XMapWindow(mDisplay, vo_window);
> -    // wait for map
> -    do {
> -      XNextEvent(mDisplay, &xev);
> -    } while (xev.type != MapNotify || xev.xmap.event != vo_window);
>     vo_x11_clearwindow(mDisplay, vo_window);
> -    XSelectInput(mDisplay, vo_window, NoEventMask);
> -    XSync(mDisplay, False);
>     vo_x11_selectinput_witherr(mDisplay, vo_window,
>           StructureNotifyMask | KeyPressMask | KeyReleaseMask | PointerMotionMask |
>           ButtonPressMask | ButtonReleaseMask | ExposureMask);
> 
> 
> Both removed groups of lines do not seem to do anything constructive. The
> second group even starts a race condition.

Unfortunately you are wrong.
Without waiting for map, our size/position request will be ignored in many cases, causing at least -geometry to break.
It might be possible to call the check_events code to actually handle those messages though.
I'll have to research if I can find a reason for the XSelectInput.
But thanks for reporting it instead of silently accepting hacks in your code, we really should all try to avoid making X messier than necessary.