On Fri, 26 Sep 2014 00:49:03 +0200
Luca Barbato <lu_z...@gentoo.org> wrote:

> And disable follow_mouse and draw_mouse accordingly.
> 
> Prevent spurious updates on multi screen setups.
> 
> Reported-By: Antonio Ospite <a...@ao2.it>

Only Reported-By?

> ---
> 
> Eventually I had time to reload X11 and test the setup Antonio suggested.
> I wonder if follow_mouse shouldn't follow across multiple screens now.
>

Naah, when you pass "-i :0.1" you are specifying what _screen_ to
capture, and when you say "-i :0" X11 automatically interprets this as
":0.0", so also in this case, from a logic point of view, capture is
meant to be restricted to one screen only.

BTW, I see that you rewrote the fix instead of using the one I sent.

In general I'd like people to ask me to fix and resend, especially
considering the fact that I put a _lot_ of effort and time in writing
commit messages...

If you wanted to do minor adjustments to the code yourself, fine, but my
commit message and authorship should have been preserved, I put the
brain power in this case ;P

Anyway, the _most_ important thing is that the problem gets fixed, but
please, next time, some more diplomacy wouldn't hurt.

A couple of comments inlined.

Thanks,
   Antonio

>  libavdevice/x11grab.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/libavdevice/x11grab.c b/libavdevice/x11grab.c
> index 1f91be9..0cb4414 100644
> --- a/libavdevice/x11grab.c
> +++ b/libavdevice/x11grab.c
> @@ -487,8 +487,8 @@ static int x11grab_read_packet(AVFormatContext *s1, 
> AVPacket *pkt)
>      int x_off         = s->x_off;
>      int y_off         = s->y_off;
>      int follow_mouse  = s->follow_mouse;
> -    int screen;
> -    Window root;
> +    int screen, pointer_x, pointer_y, _, same_screen = 0;
> +    Window w, root;
>      int64_t curtime, delay;
>      struct timespec ts;
> 
> @@ -516,14 +516,16 @@ static int x11grab_read_packet(AVFormatContext *s1, 
> AVPacket *pkt)
> 
>      screen = DefaultScreen(dpy);
>      root   = RootWindow(dpy, screen);
> -    if (follow_mouse) {
> +
> +    if (follow_mouse || s->draw_mouse)
> +        same_screen = XQueryPointer(dpy, root, &w, &w,
> +                                    &pointer_x, &pointer_y, &_, &_, &_);
> +

Although this logic is correct I feel it makes the code heavier on
the reader's mind.

The reader have to think about yet another condition when just calling
the function unconditionally has a negligible performance effect.
Same goes for xcbgrab.

> +    if (same_screen && follow_mouse) {
>          int screen_w, screen_h;
> -        int pointer_x, pointer_y, _;
> -        Window w;
> 
>          screen_w = DisplayWidth(dpy, screen);
>          screen_h = DisplayHeight(dpy, screen);
> -        XQueryPointer(dpy, root, &w, &w, &pointer_x, &pointer_y, &_, &_, &_);
>          if (follow_mouse == -1) {
>              // follow the mouse, put it at center of grabbing region
>              x_off += pointer_x - s->width / 2 - x_off;
> @@ -572,7 +574,7 @@ static int x11grab_read_packet(AVFormatContext *s1, 
> AVPacket *pkt)
>              av_log(s1, AV_LOG_INFO, "XGetZPixmap() failed\n");
>      }
>

What about the check for show_region?
For symmetry it makes sense to check for same_screen there too.

> -    if (s->draw_mouse)
> +    if (same_screen && s->draw_mouse)
>          paint_mouse_pointer(image, s);
> 
>      return s->frame_size;
> --
> 2.1.0
> 
> _______________________________________________
> libav-devel mailing list
> libav-devel@libav.org
> https://lists.libav.org/mailman/listinfo/libav-devel


-- 
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to