diff mbox series

[FFmpeg-devel,v2] x11grab: capture a window instead of the whole screen

Message ID 4ef7cc3a-1d14-49e6-291d-c58ed84da898@aol.com
State Superseded
Headers show
Series [FFmpeg-devel,v2] x11grab: capture a window instead of the whole screen | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

sgerwk-at-aol.com@ffmpeg.org Feb. 10, 2021, 5:15 p.m. UTC 
On Sat, 6 Feb 2021, Paul B Mahol wrote:

> What happens if you use non-existing window?

I added an error message for this case, but the output video is still created
(empty). Is there a value I can return to avoid this?

On Sun, 7 Feb 2021, Andriy Gelman wrote:

> Also the commit title should start with avdevice/xcbgrab: or lavd/xcbgrab:
>

done

>
> If you try with -show_region 1, the border gets drawn in the wrong place.
>

fixed

I also fixed the misalignement of draw_mouse I didn't notice last time.

>> 
>> +@item window_id
>> +Grab this window, instead of the whole screen.
>> +
>
> I think it would be useful to say how to get a window id.
> Probably worth adding what happens if the window size changes or it
> disappears.
>

Done. Recording also ends when the window is iconified or reduced in size beyond
the video size.

For now I disabled follow_mouse and select_region when window_id is given.

> xcb_window_t is uint32_t, so max should be UINT32_MAX?

Indeed.

>> @@ -825,6 +827,9 @@ static av_cold int xcbgrab_read_header(AVFormatContext
>> *s)
>>          return AVERROR(EIO);
>>      }
>> 
>> +    if (c->window_id == 0)
>> +        c->window_id = c->screen->root;
>> +
>
> Can 0 be a valid window_id? Maybe it's better to use -1 for default. I saw the
> function xcb_generate_id() returns -1 on error. 
>

0 is not a valid x11 identifier, but it's better to use the XCB_NONE macro for
it (in Xlib and higher-level libraries, it's "None")

Thanks!

> Thanks,
> -- 
> Andriy
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
From e13c1be7abd6989b3ad80fd8086fe6a0819fd810 Mon Sep 17 00:00:00 2001
From: sgerwk <sgerwk@aol.com>
Date: Wed, 10 Feb 2021 17:36:00 +0100
Subject: [PATCH] libavdevice/xcbgrab: option for grabbing a window instead of
 desktop

---
 doc/indevs.texi       | 14 +++++++-
 libavdevice/xcbgrab.c | 79 ++++++++++++++++++++++++++++++++-----------
 2 files changed, 72 insertions(+), 21 deletions(-)

Comments

Andriy Gelman Feb. 21, 2021, 7:30 a.m. UTC | #1 
On Wed, 10. Feb 18:15, sgerwk-at-aol.com@ffmpeg.org wrote:
> On Sat, 6 Feb 2021, Paul B Mahol wrote:
> 
> > What happens if you use non-existing window?
> 
> I added an error message for this case, but the output video is still created
> (empty). Is there a value I can return to avoid this?
> 
> On Sun, 7 Feb 2021, Andriy Gelman wrote:
> 
> > Also the commit title should start with avdevice/xcbgrab: or lavd/xcbgrab:
> > 
> 
> done
> 
> > 
> > If you try with -show_region 1, the border gets drawn in the wrong place.
> > 
> 
> fixed
> 
> I also fixed the misalignement of draw_mouse I didn't notice last time.
> 
> > > 
> > > +@item window_id
> > > +Grab this window, instead of the whole screen.
> > > +
> > 
> > I think it would be useful to say how to get a window id.
> > Probably worth adding what happens if the window size changes or it
> > disappears.
> > 
> 
> Done. Recording also ends when the window is iconified or reduced in size beyond
> the video size.
> 
> For now I disabled follow_mouse and select_region when window_id is given.
> 
> > xcb_window_t is uint32_t, so max should be UINT32_MAX?
> 
> Indeed.
> 
> > > @@ -825,6 +827,9 @@ static av_cold int xcbgrab_read_header(AVFormatContext
> > > *s)
> > >          return AVERROR(EIO);
> > >      }
> > > 
> > > +    if (c->window_id == 0)
> > > +        c->window_id = c->screen->root;
> > > +
> > 
> > Can 0 be a valid window_id? Maybe it's better to use -1 for default. I saw the
> > function xcb_generate_id() returns -1 on error.
> > 
> 
> 0 is not a valid x11 identifier, but it's better to use the XCB_NONE macro for
> it (in Xlib and higher-level libraries, it's "None")
> 
> Thanks!
> 
> > Thanks,
> > -- 
> > Andriy
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > 
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Hi, 

Thanks for updating the patch. Sorry for the delay in getting you some feedback..

When I tested with -show_mouse 1 -show_region 1 -window_id xx, the mouse and
border get drawn in the wrong place. There is around (0.6cm error). Can you
reproduce?

> From e13c1be7abd6989b3ad80fd8086fe6a0819fd810 Mon Sep 17 00:00:00 2001
> From: sgerwk <sgerwk@aol.com>
> Date: Wed, 10 Feb 2021 17:36:00 +0100
> Subject: [PATCH] libavdevice/xcbgrab: option for grabbing a window instead of
>  desktop
> 
> ---
>  doc/indevs.texi       | 14 +++++++-
>  libavdevice/xcbgrab.c | 79 ++++++++++++++++++++++++++++++++-----------
>  2 files changed, 72 insertions(+), 21 deletions(-)
> 
> diff --git a/doc/indevs.texi b/doc/indevs.texi
> index 3924d03..48fd2b1 100644
> --- a/doc/indevs.texi
> +++ b/doc/indevs.texi
> @@ -1564,8 +1564,20 @@ With @var{follow_mouse}:
>  ffmpeg -f x11grab -follow_mouse centered -show_region 1 -framerate 25 -video_size cif -i :0.0 out.mpg
>  @end example
>  
> +@item window_id
> +Grab this window, instead of the whole screen.
> +
> +The id of a window can be found by xwininfo(1), possibly with options -tree and
> +-root.
> +
> +If the window is later enlarged, the new area is not recorded. Video ends when
> +the window is closed, unmapped (i.e., iconified) or shrunk beyond the video
> +size (which defaults to the initial window size).
> +
> +This option disables options @option{follow_mouse} and @option{select_region}.
> +
>  @item video_size
> -Set the video frame size. Default is the full desktop.
> +Set the video frame size. Default is the full desktop or window.
>  
>  @item grab_x
>  @item grab_y
> diff --git a/libavdevice/xcbgrab.c b/libavdevice/xcbgrab.c
> index be5d5ea..7697090 100644
> --- a/libavdevice/xcbgrab.c
> +++ b/libavdevice/xcbgrab.c
> @@ -60,6 +60,8 @@ typedef struct XCBGrabContext {
>      AVRational time_base;
>      int64_t frame_duration;
>  
> +    xcb_window_t window_id;

> +    int win_x, win_y;

The position of the window is always recalculated so I don't think win_x and
win_y should be part of the of XCBGrabContext.

>      int x, y;
>      int width, height;
>      int frame_size;
> @@ -82,6 +84,7 @@ typedef struct XCBGrabContext {
>  #define OFFSET(x) offsetof(XCBGrabContext, x)
>  #define D AV_OPT_FLAG_DECODING_PARAM
>  static const AVOption options[] = {
> +    { "window_id", "Window to capture", OFFSET(window_id), AV_OPT_TYPE_INT, { .i64 = XCB_NONE }, 0, UINT32_MAX, D },
>      { "x", "Initial x coordinate.", OFFSET(x), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, D },
>      { "y", "Initial y coordinate.", OFFSET(y), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, D },
>      { "grab_x", "Initial x coordinate.", OFFSET(x), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, D },
> @@ -157,7 +160,7 @@ static int xcbgrab_frame(AVFormatContext *s, AVPacket *pkt)
>      XCBGrabContext *c = s->priv_data;
>      xcb_get_image_cookie_t iq;
>      xcb_get_image_reply_t *img;
> -    xcb_drawable_t drawable = c->screen->root;
> +    xcb_drawable_t drawable = c->window_id;
>      xcb_generic_error_t *e = NULL;
>      uint8_t *data;
>      int length;
> @@ -267,7 +270,7 @@ static int xcbgrab_frame_shm(AVFormatContext *s, AVPacket *pkt)
>      XCBGrabContext *c = s->priv_data;
>      xcb_shm_get_image_cookie_t iq;
>      xcb_shm_get_image_reply_t *img;
> -    xcb_drawable_t drawable = c->screen->root;
> +    xcb_drawable_t drawable = c->window_id;
>      xcb_generic_error_t *e = NULL;
>      AVBufferRef *buf;
>      xcb_shm_seg_t segment;
> @@ -355,17 +358,17 @@ static void xcbgrab_draw_mouse(AVFormatContext *s, AVPacket *pkt,
>      cx = ci->x - ci->xhot;
>      cy = ci->y - ci->yhot;
>  
> -    x = FFMAX(cx, gr->x);
> -    y = FFMAX(cy, gr->y);
> +    x = FFMAX(cx, gr->win_x + gr->x);
> +    y = FFMAX(cy, gr->win_y + gr->y);
>  
> -    w = FFMIN(cx + ci->width,  gr->x + gr->width)  - x;
> -    h = FFMIN(cy + ci->height, gr->y + gr->height) - y;
> +    w = FFMIN(cx + ci->width,  gr->win_x + gr->x + gr->width)  - x;
> +    h = FFMIN(cy + ci->height, gr->win_y + gr->y + gr->height) - y;
>  
>      c_off = x - cx;
> -    i_off = x - gr->x;
> +    i_off = x - gr->x - gr->win_x;
>  
>      cursor += (y - cy) * ci->width;
> -    image  += (y - gr->y) * gr->width * stride;
> +    image  += (y - gr->y - gr->win_y) * gr->width * stride;
>  
>      for (y = 0; y < h; y++) {
>          cursor += c_off;
> @@ -403,8 +406,8 @@ static void xcbgrab_draw_mouse(AVFormatContext *s, AVPacket *pkt,
>  static void xcbgrab_update_region(AVFormatContext *s)
>  {
>      XCBGrabContext *c     = s->priv_data;
> -    const uint32_t args[] = { c->x - c->region_border,
> -                              c->y - c->region_border };
> +    const uint32_t args[] = { c->win_x + c->x - c->region_border,
> +                              c->win_y + c->y - c->region_border };
>  
>      xcb_configure_window(c->conn,
>                           c->window,
> @@ -417,16 +420,22 @@ static int xcbgrab_read_packet(AVFormatContext *s, AVPacket *pkt)
>      XCBGrabContext *c = s->priv_data;
>      xcb_query_pointer_cookie_t pc;
>      xcb_get_geometry_cookie_t gc;
> +    xcb_translate_coordinates_cookie_t tc;
>      xcb_query_pointer_reply_t *p  = NULL;
>      xcb_get_geometry_reply_t *geo = NULL;
> +    xcb_translate_coordinates_reply_t *translate = NULL;
>      int ret = 0;
>      int64_t pts;
>  
>      pts = wait_frame(s, pkt);
>  
> -    if (c->follow_mouse || c->draw_mouse) {
> -        pc  = xcb_query_pointer(c->conn, c->screen->root);
> -        gc  = xcb_get_geometry(c->conn, c->screen->root);
> +    if (c->window_id == c->screen->root) {
> +        c->win_x = 0;
> +	c->win_y = 0;
> +    }
> +    if (c->follow_mouse || c->draw_mouse || c->window_id != c->screen->root) {
> +        pc  = xcb_query_pointer(c->conn, c->window_id);
> +        gc  = xcb_get_geometry(c->conn, c->window_id);
>          p   = xcb_query_pointer_reply(c->conn, pc, NULL);
>          if (!p) {
>              av_log(s, AV_LOG_ERROR, "Failed to query xcb pointer\n");
> @@ -438,6 +447,12 @@ static int xcbgrab_read_packet(AVFormatContext *s, AVPacket *pkt)
>              free(p);
>              return AVERROR_EXTERNAL;
>          }

> +        tc = xcb_translate_coordinates(c->conn, c->window_id, c->screen->root, geo->x, geo->y);

It would cleaner if you move win_x,win_y calculation to a separate if statement.
(I don't think you are going to end up reusing geo->x, geo->y anyway because probably
that's what causing the mouse/border offset error).

> +        translate = xcb_translate_coordinates_reply(c->conn, tc, NULL);

> +        if (!translate)
> +            return AVERROR_EXTERNAL;

p and geo need to be freed on this error.
You also need free translate when cleaning up at the end of this function. 

> +        c->win_x = translate->dst_x;
> +        c->win_y = translate->dst_y;
>      }
>  
>      if (c->follow_mouse && p->same_screen)
> @@ -447,7 +462,7 @@ static int xcbgrab_read_packet(AVFormatContext *s, AVPacket *pkt)
>          xcbgrab_update_region(s);
>  
>  #if CONFIG_LIBXCB_SHM

> -    if (c->has_shm && xcbgrab_frame_shm(s, pkt) < 0) {
> +    if (c->has_shm && (ret = xcbgrab_frame_shm(s, pkt)) == AVERROR(ENOMEM)) {
>          av_log(s, AV_LOG_WARNING, "Continuing without shared memory.\n");
>          c->has_shm = 0;
>      }

IMO this should be a separate commit as it changes how EACCESS is interpreted 
even when the new option is not used.

> @@ -558,7 +573,9 @@ static int create_stream(AVFormatContext *s)
>      XCBGrabContext *c = s->priv_data;
>      AVStream *st      = avformat_new_stream(s, NULL);
>      xcb_get_geometry_cookie_t gc;
> +    xcb_translate_coordinates_cookie_t tc;
>      xcb_get_geometry_reply_t *geo;
> +    xcb_translate_coordinates_reply_t *translate;
>      int64_t frame_size_bits;
>      int ret;
>  
> @@ -571,11 +588,20 @@ static int create_stream(AVFormatContext *s)
>  
>      avpriv_set_pts_info(st, 64, 1, 1000000);
>  
> -    gc  = xcb_get_geometry(c->conn, c->screen->root);
> +    gc  = xcb_get_geometry(c->conn, c->window_id);
>      geo = xcb_get_geometry_reply(c->conn, gc, NULL);
> -    if (!geo)
> +    if (!geo) {
> +        av_log(s, AV_LOG_ERROR, "Can't find window '0x%x', aborting.\n", c->window_id);
> +        return AVERROR_EXTERNAL;
> +    }
> +
> +    tc = xcb_translate_coordinates(c->conn, c->window_id, c->screen->root, geo->x, geo->y);
> +    translate = xcb_translate_coordinates_reply(c->conn, tc, NULL);

> +    if (!translate)
>          return AVERROR_EXTERNAL;

error would leak geo.
and translate will need to be freed somewhere if there's no error. 

>  
> +    c->win_x = translate->dst_x;
> +    c->win_y = translate->dst_y;
>      if (!c->width || !c->height) {
>          c->width = geo->width;
>          c->height = geo->height;

You calculate win_x/win_y, but it actually doesn't end up being used anywhere in 
xcbgrab_read_header(), and will get recalculated in xcbgrab_read_packet().
You probably meant to also update setup_window()?

(btw, currently the border doesn't end being displayed until the first
xcbgrab_read_packet() call because there is no xcb_flush(c->conn) after
setup_window(s). But it could be added to display the initial border.)


> @@ -750,7 +776,7 @@ static int select_region(AVFormatContext *s)
>              press_position = (xcb_point_t){ press->event_x, press->event_y };
>              rectangle.x = press_position.x;
>              rectangle.y = press_position.y;
> -            xcb_poly_rectangle(conn, root_window, gc, 1, &rectangle);
> +            xcb_poly_rectangle(conn, c->window_id, gc, 1, &rectangle);
>              was_pressed = 1;
>              break;
>          }
> @@ -759,14 +785,14 @@ static int select_region(AVFormatContext *s)
>                  xcb_motion_notify_event_t *motion =
>                      (xcb_motion_notify_event_t *)event;
>                  xcb_point_t cursor_position = { motion->event_x, motion->event_y };
> -                xcb_poly_rectangle(conn, root_window, gc, 1, &rectangle);
> +                xcb_poly_rectangle(conn, c->window_id, gc, 1, &rectangle);
>                  rectangle = rectangle_from_corners(&press_position, &cursor_position);
> -                xcb_poly_rectangle(conn, root_window, gc, 1, &rectangle);
> +                xcb_poly_rectangle(conn, c->window_id, gc, 1, &rectangle);
>              }
>              break;
>          }
>          case XCB_BUTTON_RELEASE: {
> -            xcb_poly_rectangle(conn, root_window, gc, 1, &rectangle);
> +            xcb_poly_rectangle(conn, c->window_id, gc, 1, &rectangle);
>              done = 1;
>              break;
>          }
> @@ -830,6 +856,19 @@ static av_cold int xcbgrab_read_header(AVFormatContext *s)
>          return AVERROR(EIO);
>      }

You disabled select_region() earlier. These changes can be skipped then.

>  
> +    if (c->window_id == XCB_NONE)
> +        c->window_id = c->screen->root;
> +    else {
> +        if (c->select_region) {
> +            av_log(s, AV_LOG_WARNING, "select_region ignored with window_id.\n");
> +            c->select_region = 0;
> +        }
> +        if (c->follow_mouse) {
> +            av_log(s, AV_LOG_WARNING, "follow_mouse ignored with window_id.\n");
> +            c->follow_mouse = 0;
> +        }
> +    }
> +
>      if (c->select_region) {
>          ret = select_region(s);

Thanks,
sgerwk-at-aol.com@ffmpeg.org Feb. 22, 2021, 4:53 p.m. UTC | #2 
Hi,

On Sun, 21 Feb 2021, Andriy Gelman wrote:
> Hi, 
>
> Thanks for updating the patch. Sorry for the delay in getting you some feedback..
>
> When I tested with -show_mouse 1 -show_region 1 -window_id xx, the mouse and
> border get drawn in the wrong place. There is around (0.6cm error). Can you
> reproduce?
>

I didn't notice the problem because the wm I use places the top-level
windows in a window of the same size - at position 0,0. Still, I could
reproduce it by capturing a subwindow.

The problem was indeed what you suspected: geo->x,geo->y is the position 
inside the parent, but translate includes it already as it is the position
within the root window.

>> From e13c1be7abd6989b3ad80fd8086fe6a0819fd810 Mon Sep 17 00:00:00 2001
>> From: sgerwk <sgerwk@aol.com>
>> Date: Wed, 10 Feb 2021 17:36:00 +0100
>> Subject: [PATCH] libavdevice/xcbgrab: option for grabbing a window instead of
>>  desktop
>> 
>> ---
>>  doc/indevs.texi       | 14 +++++++-
>>  libavdevice/xcbgrab.c | 79 ++++++++++++++++++++++++++++++++-----------
>>  2 files changed, 72 insertions(+), 21 deletions(-)
>> 
>> diff --git a/doc/indevs.texi b/doc/indevs.texi
>> index 3924d03..48fd2b1 100644
>> --- a/doc/indevs.texi
>> +++ b/doc/indevs.texi
>> @@ -1564,8 +1564,20 @@ With @var{follow_mouse}:
>>  ffmpeg -f x11grab -follow_mouse centered -show_region 1 -framerate 25 -video_size cif -i :0.0 out.mpg
>>  @end example
>> 
>> +@item window_id
>> +Grab this window, instead of the whole screen.
>> +
>> +The id of a window can be found by xwininfo(1), possibly with options -tree and
>> +-root.
>> +
>> +If the window is later enlarged, the new area is not recorded. Video ends when
>> +the window is closed, unmapped (i.e., iconified) or shrunk beyond the video
>> +size (which defaults to the initial window size).
>> +
>> +This option disables options @option{follow_mouse} and @option{select_region}.
>> +
>>  @item video_size
>> -Set the video frame size. Default is the full desktop.
>> +Set the video frame size. Default is the full desktop or window.
>>
>>  @item grab_x
>>  @item grab_y
>> diff --git a/libavdevice/xcbgrab.c b/libavdevice/xcbgrab.c
>> index be5d5ea..7697090 100644
>> --- a/libavdevice/xcbgrab.c
>> +++ b/libavdevice/xcbgrab.c
>> @@ -60,6 +60,8 @@ typedef struct XCBGrabContext {
>>      AVRational time_base;
>>      int64_t frame_duration;
>> 
>> +    xcb_window_t window_id;
>
>> +    int win_x, win_y;
>
> The position of the window is always recalculated so I don't think win_x and
> win_y should be part of the of XCBGrabContext.
>

Done. Some functions now require win_x and win_y because they no longer
find them in XCBGrabContext.

>>      int x, y;
>>      int width, height;
>>      int frame_size;
>> @@ -82,6 +84,7 @@ typedef struct XCBGrabContext {
>>  #define OFFSET(x) offsetof(XCBGrabContext, x)
>>  #define D AV_OPT_FLAG_DECODING_PARAM
>>  static const AVOption options[] = {
>> +    { "window_id", "Window to capture", OFFSET(window_id), AV_OPT_TYPE_INT, { .i64 = XCB_NONE }, 0, UINT32_MAX, D },
>>      { "x", "Initial x coordinate.", OFFSET(x), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, D },
>>      { "y", "Initial y coordinate.", OFFSET(y), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, D },
>>      { "grab_x", "Initial x coordinate.", OFFSET(x), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, D },
>> @@ -157,7 +160,7 @@ static int xcbgrab_frame(AVFormatContext *s, AVPacket *pkt)
>>      XCBGrabContext *c = s->priv_data;
>>      xcb_get_image_cookie_t iq;
>>      xcb_get_image_reply_t *img;
>> -    xcb_drawable_t drawable = c->screen->root;
>> +    xcb_drawable_t drawable = c->window_id;
>>      xcb_generic_error_t *e = NULL;
>>      uint8_t *data;
>>      int length;
>> @@ -267,7 +270,7 @@ static int xcbgrab_frame_shm(AVFormatContext *s, AVPacket *pkt)
>>      XCBGrabContext *c = s->priv_data;
>>      xcb_shm_get_image_cookie_t iq;
>>      xcb_shm_get_image_reply_t *img;
>> -    xcb_drawable_t drawable = c->screen->root;
>> +    xcb_drawable_t drawable = c->window_id;
>>      xcb_generic_error_t *e = NULL;
>>      AVBufferRef *buf;
>>      xcb_shm_seg_t segment;
>> @@ -355,17 +358,17 @@ static void xcbgrab_draw_mouse(AVFormatContext *s, AVPacket *pkt,
>>      cx = ci->x - ci->xhot;
>>      cy = ci->y - ci->yhot;
>> 
>> -    x = FFMAX(cx, gr->x);
>> -    y = FFMAX(cy, gr->y);
>> +    x = FFMAX(cx, gr->win_x + gr->x);
>> +    y = FFMAX(cy, gr->win_y + gr->y);
>> 
>> -    w = FFMIN(cx + ci->width,  gr->x + gr->width)  - x;
>> -    h = FFMIN(cy + ci->height, gr->y + gr->height) - y;
>> +    w = FFMIN(cx + ci->width,  gr->win_x + gr->x + gr->width)  - x;
>> +    h = FFMIN(cy + ci->height, gr->win_y + gr->y + gr->height) - y;
>>
>>      c_off = x - cx;
>> -    i_off = x - gr->x;
>> +    i_off = x - gr->x - gr->win_x;
>>
>>      cursor += (y - cy) * ci->width;
>> -    image  += (y - gr->y) * gr->width * stride;
>> +    image  += (y - gr->y - gr->win_y) * gr->width * stride;
>>
>>      for (y = 0; y < h; y++) {
>>          cursor += c_off;
>> @@ -403,8 +406,8 @@ static void xcbgrab_draw_mouse(AVFormatContext *s, AVPacket *pkt,
>>  static void xcbgrab_update_region(AVFormatContext *s)
>>  {
>>      XCBGrabContext *c     = s->priv_data;
>> -    const uint32_t args[] = { c->x - c->region_border,
>> -                              c->y - c->region_border };
>> +    const uint32_t args[] = { c->win_x + c->x - c->region_border,
>> +                              c->win_y + c->y - c->region_border };
>>
>>      xcb_configure_window(c->conn,
>>                           c->window,
>> @@ -417,16 +420,22 @@ static int xcbgrab_read_packet(AVFormatContext *s, AVPacket *pkt)
>>      XCBGrabContext *c = s->priv_data;
>>      xcb_query_pointer_cookie_t pc;
>>      xcb_get_geometry_cookie_t gc;
>> +    xcb_translate_coordinates_cookie_t tc;
>>      xcb_query_pointer_reply_t *p  = NULL;
>>      xcb_get_geometry_reply_t *geo = NULL;
>> +    xcb_translate_coordinates_reply_t *translate = NULL;
>>      int ret = 0;
>>      int64_t pts;
>>
>>      pts = wait_frame(s, pkt);
>> 
>> -    if (c->follow_mouse || c->draw_mouse) {
>> -        pc  = xcb_query_pointer(c->conn, c->screen->root);
>> -        gc  = xcb_get_geometry(c->conn, c->screen->root);
>> +    if (c->window_id == c->screen->root) {
>> +        c->win_x = 0;
>> +	c->win_y = 0;
>> +    }
>> +    if (c->follow_mouse || c->draw_mouse || c->window_id != c->screen->root) {
>> +        pc  = xcb_query_pointer(c->conn, c->window_id);
>> +        gc  = xcb_get_geometry(c->conn, c->window_id);
>>          p   = xcb_query_pointer_reply(c->conn, pc, NULL);
>>          if (!p) {
>>              av_log(s, AV_LOG_ERROR, "Failed to query xcb pointer\n");
>> @@ -438,6 +447,12 @@ static int xcbgrab_read_packet(AVFormatContext *s, AVPacket *pkt)
>>              free(p);
>>              return AVERROR_EXTERNAL;
>>          }
>
>> +        tc = xcb_translate_coordinates(c->conn, c->window_id, c->screen->root, geo->x, geo->y);
>
> It would cleaner if you move win_x,win_y calculation to a separate if statement.
> (I don't think you are going to end up reusing geo->x, geo->y anyway because probably
> that's what causing the mouse/border offset error).
>

Actually, this if is executed in the default case because of draw_mouse. 
But still, the code looks better with a separate if, so I did it.

>> +        translate = xcb_translate_coordinates_reply(c->conn, tc, NULL);
>
>> +        if (!translate)
>> +            return AVERROR_EXTERNAL;
>
> p and geo need to be freed on this error.
> You also need free translate when cleaning up at the end of this function.

Done. I free translate immediately after getting win_x and win_y.

>
>> +        c->win_x = translate->dst_x;
>> +        c->win_y = translate->dst_y;
>>      }
>>
>>      if (c->follow_mouse && p->same_screen)
>> @@ -447,7 +462,7 @@ static int xcbgrab_read_packet(AVFormatContext *s, AVPacket *pkt)
>>          xcbgrab_update_region(s);
>>
>>  #if CONFIG_LIBXCB_SHM
>
>> -    if (c->has_shm && xcbgrab_frame_shm(s, pkt) < 0) {
>> +    if (c->has_shm && (ret = xcbgrab_frame_shm(s, pkt)) == AVERROR(ENOMEM)) {
>>          av_log(s, AV_LOG_WARNING, "Continuing without shared memory.\n");
>>          c->has_shm = 0;
>>      }
>
> IMO this should be a separate commit as it changes how EACCESS is interpreted 
> even when the new option is not used.
>

I now just don't remember why I changed this, so I restored the previous 
condition.

>> @@ -558,7 +573,9 @@ static int create_stream(AVFormatContext *s)
>>      XCBGrabContext *c = s->priv_data;
>>      AVStream *st      = avformat_new_stream(s, NULL);
>>      xcb_get_geometry_cookie_t gc;
>> +    xcb_translate_coordinates_cookie_t tc;
>>      xcb_get_geometry_reply_t *geo;
>> +    xcb_translate_coordinates_reply_t *translate;
>>      int64_t frame_size_bits;
>>      int ret;
>> 
>> @@ -571,11 +588,20 @@ static int create_stream(AVFormatContext *s)
>>
>>      avpriv_set_pts_info(st, 64, 1, 1000000);
>> 
>> -    gc  = xcb_get_geometry(c->conn, c->screen->root);
>> +    gc  = xcb_get_geometry(c->conn, c->window_id);
>>      geo = xcb_get_geometry_reply(c->conn, gc, NULL);
>> -    if (!geo)
>> +    if (!geo) {
>> +        av_log(s, AV_LOG_ERROR, "Can't find window '0x%x', aborting.\n", c->window_id);
>> +        return AVERROR_EXTERNAL;
>> +    }
>> +
>> +    tc = xcb_translate_coordinates(c->conn, c->window_id, c->screen->root, geo->x, geo->y);
>> +    translate = xcb_translate_coordinates_reply(c->conn, tc, NULL);
>
>> +    if (!translate)
>>          return AVERROR_EXTERNAL;
>
> error would leak geo.
> and translate will need to be freed somewhere if there's no error. 
>
>> 
>> +    c->win_x = translate->dst_x;
>> +    c->win_y = translate->dst_y;
>>      if (!c->width || !c->height) {
>>          c->width = geo->width;
>>          c->height = geo->height;
>
> You calculate win_x/win_y, but it actually doesn't end up being used anywhere in 
> xcbgrab_read_header(), and will get recalculated in xcbgrab_read_packet().
> You probably meant to also update setup_window()?
>

This calculation doesn't seem necessary, indeed. I removed it.

> (btw, currently the border doesn't end being displayed until the first
> xcbgrab_read_packet() call because there is no xcb_flush(c->conn) after
> setup_window(s). But it could be added to display the initial border.)
>

I guess this should go in a separate commit, as it also affect the case 
when window_id is not set.

>
>> @@ -750,7 +776,7 @@ static int select_region(AVFormatContext *s)
>>              press_position = (xcb_point_t){ press->event_x, press->event_y };
>>              rectangle.x = press_position.x;
>>              rectangle.y = press_position.y;
>> -            xcb_poly_rectangle(conn, root_window, gc, 1, &rectangle);
>> +            xcb_poly_rectangle(conn, c->window_id, gc, 1, &rectangle);
>>              was_pressed = 1;
>>              break;
>>          }
>> @@ -759,14 +785,14 @@ static int select_region(AVFormatContext *s)
>>                  xcb_motion_notify_event_t *motion =
>>                      (xcb_motion_notify_event_t *)event;
>>                  xcb_point_t cursor_position = { motion->event_x, motion->event_y };
>> -                xcb_poly_rectangle(conn, root_window, gc, 1, &rectangle);
>> +                xcb_poly_rectangle(conn, c->window_id, gc, 1, &rectangle);
>>                  rectangle = rectangle_from_corners(&press_position, &cursor_position);
>> -                xcb_poly_rectangle(conn, root_window, gc, 1, &rectangle);
>> +                xcb_poly_rectangle(conn, c->window_id, gc, 1, &rectangle);
>>              }
>>              break;
>>          }
>>          case XCB_BUTTON_RELEASE: {
>> -            xcb_poly_rectangle(conn, root_window, gc, 1, &rectangle);
>> +            xcb_poly_rectangle(conn, c->window_id, gc, 1, &rectangle);
>>              done = 1;
>>              break;
>>          }
>> @@ -830,6 +856,19 @@ static av_cold int xcbgrab_read_header(AVFormatContext *s)
>>          return AVERROR(EIO);
>>      }
>
> You disabled select_region() earlier. These changes can be skipped then.
>

I disagree. The captured window is c->window_id now. Even if this part of
code is currently unreachable if window_id is not the root window,
a future commit may make it reachable again.

>> 
>> +    if (c->window_id == XCB_NONE)
>> +        c->window_id = c->screen->root;
>> +    else {
>> +        if (c->select_region) {
>> +            av_log(s, AV_LOG_WARNING, "select_region ignored with window_id.\n");
>> +            c->select_region = 0;
>> +        }
>> +        if (c->follow_mouse) {
>> +            av_log(s, AV_LOG_WARNING, "follow_mouse ignored with window_id.\n");
>> +            c->follow_mouse = 0;
>> +        }
>> +    }
>> +
>>      if (c->select_region) {
>>          ret = select_region(s);
>
> Thanks,
> -- 
> Andriy
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Thanks!
From 36e6c5a858415106d4d2d9a76fa45cbd59717c77 Mon Sep 17 00:00:00 2001
From: sgerwk <sgerwk@aol.com>
Date: Wed, 10 Feb 2021 17:36:00 +0100
Subject: [PATCH] libavdevice/xcbgrab: option for grabbing a window instead of
 desktop

---
 doc/indevs.texi       | 14 ++++++-
 libavdevice/xcbgrab.c | 93 +++++++++++++++++++++++++++++++------------
 2 files changed, 80 insertions(+), 27 deletions(-)

diff --git a/doc/indevs.texi b/doc/indevs.texi
index 3924d03..48fd2b1 100644
--- a/doc/indevs.texi
+++ b/doc/indevs.texi
@@ -1564,8 +1564,20 @@ With @var{follow_mouse}:
 ffmpeg -f x11grab -follow_mouse centered -show_region 1 -framerate 25 -video_size cif -i :0.0 out.mpg
 @end example
 
+@item window_id
+Grab this window, instead of the whole screen.
+
+The id of a window can be found by xwininfo(1), possibly with options -tree and
+-root.
+
+If the window is later enlarged, the new area is not recorded. Video ends when
+the window is closed, unmapped (i.e., iconified) or shrunk beyond the video
+size (which defaults to the initial window size).
+
+This option disables options @option{follow_mouse} and @option{select_region}.
+
 @item video_size
-Set the video frame size. Default is the full desktop.
+Set the video frame size. Default is the full desktop or window.
 
 @item grab_x
 @item grab_y
diff --git a/libavdevice/xcbgrab.c b/libavdevice/xcbgrab.c
index be5d5ea..ae935ad 100644
--- a/libavdevice/xcbgrab.c
+++ b/libavdevice/xcbgrab.c
@@ -60,6 +60,7 @@ typedef struct XCBGrabContext {
     AVRational time_base;
     int64_t frame_duration;
 
+    xcb_window_t window_id;
     int x, y;
     int width, height;
     int frame_size;
@@ -82,6 +83,7 @@ typedef struct XCBGrabContext {
 #define OFFSET(x) offsetof(XCBGrabContext, x)
 #define D AV_OPT_FLAG_DECODING_PARAM
 static const AVOption options[] = {
+    { "window_id", "Window to capture", OFFSET(window_id), AV_OPT_TYPE_INT, { .i64 = XCB_NONE }, 0, UINT32_MAX, D },
     { "x", "Initial x coordinate.", OFFSET(x), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, D },
     { "y", "Initial y coordinate.", OFFSET(y), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, D },
     { "grab_x", "Initial x coordinate.", OFFSET(x), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, D },
@@ -108,7 +110,8 @@ static const AVClass xcbgrab_class = {
 
 static int xcbgrab_reposition(AVFormatContext *s,
                               xcb_query_pointer_reply_t *p,
-                              xcb_get_geometry_reply_t *geo)
+                              xcb_get_geometry_reply_t *geo,
+                              int win_x, int win_y)
 {
     XCBGrabContext *c = s->priv_data;
     int x = c->x, y = c->y;
@@ -118,8 +121,8 @@ static int xcbgrab_reposition(AVFormatContext *s,
     if (!p || !geo)
         return AVERROR(EIO);
 
-    p_x = p->win_x;
-    p_y = p->win_y;
+    p_x = win_x;
+    p_y = win_y;
 
     if (f == FOLLOW_CENTER) {
         x = p_x - w / 2;
@@ -157,7 +160,7 @@ static int xcbgrab_frame(AVFormatContext *s, AVPacket *pkt)
     XCBGrabContext *c = s->priv_data;
     xcb_get_image_cookie_t iq;
     xcb_get_image_reply_t *img;
-    xcb_drawable_t drawable = c->screen->root;
+    xcb_drawable_t drawable = c->window_id;
     xcb_generic_error_t *e = NULL;
     uint8_t *data;
     int length;
@@ -267,7 +270,7 @@ static int xcbgrab_frame_shm(AVFormatContext *s, AVPacket *pkt)
     XCBGrabContext *c = s->priv_data;
     xcb_shm_get_image_cookie_t iq;
     xcb_shm_get_image_reply_t *img;
-    xcb_drawable_t drawable = c->screen->root;
+    xcb_drawable_t drawable = c->window_id;
     xcb_generic_error_t *e = NULL;
     AVBufferRef *buf;
     xcb_shm_seg_t segment;
@@ -333,7 +336,8 @@ static int check_xfixes(xcb_connection_t *conn)
 
 static void xcbgrab_draw_mouse(AVFormatContext *s, AVPacket *pkt,
                                xcb_query_pointer_reply_t *p,
-                               xcb_get_geometry_reply_t *geo)
+                               xcb_get_geometry_reply_t *geo,
+                               int win_x, int win_y)
 {
     XCBGrabContext *gr = s->priv_data;
     uint32_t *cursor;
@@ -355,17 +359,17 @@ static void xcbgrab_draw_mouse(AVFormatContext *s, AVPacket *pkt,
     cx = ci->x - ci->xhot;
     cy = ci->y - ci->yhot;
 
-    x = FFMAX(cx, gr->x);
-    y = FFMAX(cy, gr->y);
+    x = FFMAX(cx, win_x + gr->x);
+    y = FFMAX(cy, win_y + gr->y);
 
-    w = FFMIN(cx + ci->width,  gr->x + gr->width)  - x;
-    h = FFMIN(cy + ci->height, gr->y + gr->height) - y;
+    w = FFMIN(cx + ci->width,  win_x + gr->x + gr->width)  - x;
+    h = FFMIN(cy + ci->height, win_y + gr->y + gr->height) - y;
 
     c_off = x - cx;
-    i_off = x - gr->x;
+    i_off = x - gr->x - win_x;
 
     cursor += (y - cy) * ci->width;
-    image  += (y - gr->y) * gr->width * stride;
+    image  += (y - gr->y - win_y) * gr->width * stride;
 
     for (y = 0; y < h; y++) {
         cursor += c_off;
@@ -400,11 +404,11 @@ static void xcbgrab_draw_mouse(AVFormatContext *s, AVPacket *pkt,
 }
 #endif /* CONFIG_LIBXCB_XFIXES */
 
-static void xcbgrab_update_region(AVFormatContext *s)
+static void xcbgrab_update_region(AVFormatContext *s, int win_x, int win_y)
 {
     XCBGrabContext *c     = s->priv_data;
-    const uint32_t args[] = { c->x - c->region_border,
-                              c->y - c->region_border };
+    const uint32_t args[] = { win_x + c->x - c->region_border,
+                              win_y + c->y - c->region_border };
 
     xcb_configure_window(c->conn,
                          c->window,
@@ -417,16 +421,23 @@ static int xcbgrab_read_packet(AVFormatContext *s, AVPacket *pkt)
     XCBGrabContext *c = s->priv_data;
     xcb_query_pointer_cookie_t pc;
     xcb_get_geometry_cookie_t gc;
+    xcb_translate_coordinates_cookie_t tc;
     xcb_query_pointer_reply_t *p  = NULL;
     xcb_get_geometry_reply_t *geo = NULL;
+    xcb_translate_coordinates_reply_t *translate = NULL;
     int ret = 0;
     int64_t pts;
+    int win_x, win_y;
 
     pts = wait_frame(s, pkt);
 
+    if (c->window_id == c->screen->root) {
+        win_x = 0;
+        win_y = 0;
+    }
     if (c->follow_mouse || c->draw_mouse) {
-        pc  = xcb_query_pointer(c->conn, c->screen->root);
-        gc  = xcb_get_geometry(c->conn, c->screen->root);
+        pc  = xcb_query_pointer(c->conn, c->window_id);
+        gc  = xcb_get_geometry(c->conn, c->window_id);
         p   = xcb_query_pointer_reply(c->conn, pc, NULL);
         if (!p) {
             av_log(s, AV_LOG_ERROR, "Failed to query xcb pointer\n");
@@ -439,12 +450,27 @@ static int xcbgrab_read_packet(AVFormatContext *s, AVPacket *pkt)
             return AVERROR_EXTERNAL;
         }
     }
+    if (c->window_id != c->screen->root) {
+        tc = xcb_translate_coordinates(c->conn, c->window_id, c->screen->root, 0, 0);
+        translate = xcb_translate_coordinates_reply(c->conn, tc, NULL);
+        if (!translate) {
+            if (p != NULL)
+                free(p);
+            if (geo != NULL)
+                free(geo);
+            av_log(s, AV_LOG_ERROR, "Failed to translate xcb geometry\n");
+            return AVERROR_EXTERNAL;
+        }
+        win_x = translate->dst_x;
+        win_y = translate->dst_y;
+        free(translate);
+    }
 
     if (c->follow_mouse && p->same_screen)
-        xcbgrab_reposition(s, p, geo);
+        xcbgrab_reposition(s, p, geo, win_x, win_y);
 
     if (c->show_region)
-        xcbgrab_update_region(s);
+        xcbgrab_update_region(s, win_x, win_y);
 
 #if CONFIG_LIBXCB_SHM
     if (c->has_shm && xcbgrab_frame_shm(s, pkt) < 0) {
@@ -459,7 +485,7 @@ static int xcbgrab_read_packet(AVFormatContext *s, AVPacket *pkt)
 
 #if CONFIG_LIBXCB_XFIXES
     if (ret >= 0 && c->draw_mouse && p->same_screen)
-        xcbgrab_draw_mouse(s, pkt, p, geo);
+        xcbgrab_draw_mouse(s, pkt, p, geo, win_x, win_y);
 #endif
 
     free(p);
@@ -571,10 +597,12 @@ static int create_stream(AVFormatContext *s)
 
     avpriv_set_pts_info(st, 64, 1, 1000000);
 
-    gc  = xcb_get_geometry(c->conn, c->screen->root);
+    gc  = xcb_get_geometry(c->conn, c->window_id);
     geo = xcb_get_geometry_reply(c->conn, gc, NULL);
-    if (!geo)
+    if (!geo) {
+        av_log(s, AV_LOG_ERROR, "Can't find window '0x%x', aborting.\n", c->window_id);
         return AVERROR_EXTERNAL;
+    }
 
     if (!c->width || !c->height) {
         c->width = geo->width;
@@ -750,7 +778,7 @@ static int select_region(AVFormatContext *s)
             press_position = (xcb_point_t){ press->event_x, press->event_y };
             rectangle.x = press_position.x;
             rectangle.y = press_position.y;
-            xcb_poly_rectangle(conn, root_window, gc, 1, &rectangle);
+            xcb_poly_rectangle(conn, c->window_id, gc, 1, &rectangle);
             was_pressed = 1;
             break;
         }
@@ -759,14 +787,14 @@ static int select_region(AVFormatContext *s)
                 xcb_motion_notify_event_t *motion =
                     (xcb_motion_notify_event_t *)event;
                 xcb_point_t cursor_position = { motion->event_x, motion->event_y };
-                xcb_poly_rectangle(conn, root_window, gc, 1, &rectangle);
+                xcb_poly_rectangle(conn, c->window_id, gc, 1, &rectangle);
                 rectangle = rectangle_from_corners(&press_position, &cursor_position);
-                xcb_poly_rectangle(conn, root_window, gc, 1, &rectangle);
+                xcb_poly_rectangle(conn, c->window_id, gc, 1, &rectangle);
             }
             break;
         }
         case XCB_BUTTON_RELEASE: {
-            xcb_poly_rectangle(conn, root_window, gc, 1, &rectangle);
+            xcb_poly_rectangle(conn, c->window_id, gc, 1, &rectangle);
             done = 1;
             break;
         }
@@ -830,6 +858,19 @@ static av_cold int xcbgrab_read_header(AVFormatContext *s)
         return AVERROR(EIO);
     }
 
+    if (c->window_id == XCB_NONE)
+        c->window_id = c->screen->root;
+    else {
+        if (c->select_region) {
+            av_log(s, AV_LOG_WARNING, "select_region ignored with window_id.\n");
+            c->select_region = 0;
+        }
+        if (c->follow_mouse) {
+            av_log(s, AV_LOG_WARNING, "follow_mouse ignored with window_id.\n");
+            c->follow_mouse = 0;
+        }
+    }
+
     if (c->select_region) {
         ret = select_region(s);
         if (ret < 0) {
diff mbox series

Patch

diff --git a/doc/indevs.texi b/doc/indevs.texi
index 3924d03..48fd2b1 100644
--- a/doc/indevs.texi
+++ b/doc/indevs.texi
@@ -1564,8 +1564,20 @@  With @var{follow_mouse}:
 ffmpeg -f x11grab -follow_mouse centered -show_region 1 -framerate 25 -video_size cif -i :0.0 out.mpg
 @end example
 
+@item window_id
+Grab this window, instead of the whole screen.
+
+The id of a window can be found by xwininfo(1), possibly with options -tree and
+-root.
+
+If the window is later enlarged, the new area is not recorded. Video ends when
+the window is closed, unmapped (i.e., iconified) or shrunk beyond the video
+size (which defaults to the initial window size).
+
+This option disables options @option{follow_mouse} and @option{select_region}.
+
 @item video_size
-Set the video frame size. Default is the full desktop.
+Set the video frame size. Default is the full desktop or window.
 
 @item grab_x
 @item grab_y
diff --git a/libavdevice/xcbgrab.c b/libavdevice/xcbgrab.c
index be5d5ea..7697090 100644
--- a/libavdevice/xcbgrab.c
+++ b/libavdevice/xcbgrab.c
@@ -60,6 +60,8 @@  typedef struct XCBGrabContext {
     AVRational time_base;
     int64_t frame_duration;
 
+    xcb_window_t window_id;
+    int win_x, win_y;
     int x, y;
     int width, height;
     int frame_size;
@@ -82,6 +84,7 @@  typedef struct XCBGrabContext {
 #define OFFSET(x) offsetof(XCBGrabContext, x)
 #define D AV_OPT_FLAG_DECODING_PARAM
 static const AVOption options[] = {
+    { "window_id", "Window to capture", OFFSET(window_id), AV_OPT_TYPE_INT, { .i64 = XCB_NONE }, 0, UINT32_MAX, D },
     { "x", "Initial x coordinate.", OFFSET(x), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, D },
     { "y", "Initial y coordinate.", OFFSET(y), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, D },
     { "grab_x", "Initial x coordinate.", OFFSET(x), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, D },
@@ -157,7 +160,7 @@  static int xcbgrab_frame(AVFormatContext *s, AVPacket *pkt)
     XCBGrabContext *c = s->priv_data;
     xcb_get_image_cookie_t iq;
     xcb_get_image_reply_t *img;
-    xcb_drawable_t drawable = c->screen->root;
+    xcb_drawable_t drawable = c->window_id;
     xcb_generic_error_t *e = NULL;
     uint8_t *data;
     int length;
@@ -267,7 +270,7 @@  static int xcbgrab_frame_shm(AVFormatContext *s, AVPacket *pkt)
     XCBGrabContext *c = s->priv_data;
     xcb_shm_get_image_cookie_t iq;
     xcb_shm_get_image_reply_t *img;
-    xcb_drawable_t drawable = c->screen->root;
+    xcb_drawable_t drawable = c->window_id;
     xcb_generic_error_t *e = NULL;
     AVBufferRef *buf;
     xcb_shm_seg_t segment;
@@ -355,17 +358,17 @@  static void xcbgrab_draw_mouse(AVFormatContext *s, AVPacket *pkt,
     cx = ci->x - ci->xhot;
     cy = ci->y - ci->yhot;
 
-    x = FFMAX(cx, gr->x);
-    y = FFMAX(cy, gr->y);
+    x = FFMAX(cx, gr->win_x + gr->x);
+    y = FFMAX(cy, gr->win_y + gr->y);
 
-    w = FFMIN(cx + ci->width,  gr->x + gr->width)  - x;
-    h = FFMIN(cy + ci->height, gr->y + gr->height) - y;
+    w = FFMIN(cx + ci->width,  gr->win_x + gr->x + gr->width)  - x;
+    h = FFMIN(cy + ci->height, gr->win_y + gr->y + gr->height) - y;
 
     c_off = x - cx;
-    i_off = x - gr->x;
+    i_off = x - gr->x - gr->win_x;
 
     cursor += (y - cy) * ci->width;
-    image  += (y - gr->y) * gr->width * stride;
+    image  += (y - gr->y - gr->win_y) * gr->width * stride;
 
     for (y = 0; y < h; y++) {
         cursor += c_off;
@@ -403,8 +406,8 @@  static void xcbgrab_draw_mouse(AVFormatContext *s, AVPacket *pkt,
 static void xcbgrab_update_region(AVFormatContext *s)
 {
     XCBGrabContext *c     = s->priv_data;
-    const uint32_t args[] = { c->x - c->region_border,
-                              c->y - c->region_border };
+    const uint32_t args[] = { c->win_x + c->x - c->region_border,
+                              c->win_y + c->y - c->region_border };
 
     xcb_configure_window(c->conn,
                          c->window,
@@ -417,16 +420,22 @@  static int xcbgrab_read_packet(AVFormatContext *s, AVPacket *pkt)
     XCBGrabContext *c = s->priv_data;
     xcb_query_pointer_cookie_t pc;
     xcb_get_geometry_cookie_t gc;
+    xcb_translate_coordinates_cookie_t tc;
     xcb_query_pointer_reply_t *p  = NULL;
     xcb_get_geometry_reply_t *geo = NULL;
+    xcb_translate_coordinates_reply_t *translate = NULL;
     int ret = 0;
     int64_t pts;
 
     pts = wait_frame(s, pkt);
 
-    if (c->follow_mouse || c->draw_mouse) {
-        pc  = xcb_query_pointer(c->conn, c->screen->root);
-        gc  = xcb_get_geometry(c->conn, c->screen->root);
+    if (c->window_id == c->screen->root) {
+        c->win_x = 0;
+	c->win_y = 0;
+    }
+    if (c->follow_mouse || c->draw_mouse || c->window_id != c->screen->root) {
+        pc  = xcb_query_pointer(c->conn, c->window_id);
+        gc  = xcb_get_geometry(c->conn, c->window_id);
         p   = xcb_query_pointer_reply(c->conn, pc, NULL);
         if (!p) {
             av_log(s, AV_LOG_ERROR, "Failed to query xcb pointer\n");
@@ -438,6 +447,12 @@  static int xcbgrab_read_packet(AVFormatContext *s, AVPacket *pkt)
             free(p);
             return AVERROR_EXTERNAL;
         }
+        tc = xcb_translate_coordinates(c->conn, c->window_id, c->screen->root, geo->x, geo->y);
+        translate = xcb_translate_coordinates_reply(c->conn, tc, NULL);
+        if (!translate)
+            return AVERROR_EXTERNAL;
+        c->win_x = translate->dst_x;
+        c->win_y = translate->dst_y;
     }
 
     if (c->follow_mouse && p->same_screen)
@@ -447,7 +462,7 @@  static int xcbgrab_read_packet(AVFormatContext *s, AVPacket *pkt)
         xcbgrab_update_region(s);
 
 #if CONFIG_LIBXCB_SHM
-    if (c->has_shm && xcbgrab_frame_shm(s, pkt) < 0) {
+    if (c->has_shm && (ret = xcbgrab_frame_shm(s, pkt)) == AVERROR(ENOMEM)) {
         av_log(s, AV_LOG_WARNING, "Continuing without shared memory.\n");
         c->has_shm = 0;
     }
@@ -558,7 +573,9 @@  static int create_stream(AVFormatContext *s)
     XCBGrabContext *c = s->priv_data;
     AVStream *st      = avformat_new_stream(s, NULL);
     xcb_get_geometry_cookie_t gc;
+    xcb_translate_coordinates_cookie_t tc;
     xcb_get_geometry_reply_t *geo;
+    xcb_translate_coordinates_reply_t *translate;
     int64_t frame_size_bits;
     int ret;
 
@@ -571,11 +588,20 @@  static int create_stream(AVFormatContext *s)
 
     avpriv_set_pts_info(st, 64, 1, 1000000);
 
-    gc  = xcb_get_geometry(c->conn, c->screen->root);
+    gc  = xcb_get_geometry(c->conn, c->window_id);
     geo = xcb_get_geometry_reply(c->conn, gc, NULL);
-    if (!geo)
+    if (!geo) {
+        av_log(s, AV_LOG_ERROR, "Can't find window '0x%x', aborting.\n", c->window_id);
+        return AVERROR_EXTERNAL;
+    }
+
+    tc = xcb_translate_coordinates(c->conn, c->window_id, c->screen->root, geo->x, geo->y);
+    translate = xcb_translate_coordinates_reply(c->conn, tc, NULL);
+    if (!translate)
         return AVERROR_EXTERNAL;
 
+    c->win_x = translate->dst_x;
+    c->win_y = translate->dst_y;
     if (!c->width || !c->height) {
         c->width = geo->width;
         c->height = geo->height;
@@ -750,7 +776,7 @@  static int select_region(AVFormatContext *s)
             press_position = (xcb_point_t){ press->event_x, press->event_y };
             rectangle.x = press_position.x;
             rectangle.y = press_position.y;
-            xcb_poly_rectangle(conn, root_window, gc, 1, &rectangle);
+            xcb_poly_rectangle(conn, c->window_id, gc, 1, &rectangle);
             was_pressed = 1;
             break;
         }
@@ -759,14 +785,14 @@  static int select_region(AVFormatContext *s)
                 xcb_motion_notify_event_t *motion =
                     (xcb_motion_notify_event_t *)event;
                 xcb_point_t cursor_position = { motion->event_x, motion->event_y };
-                xcb_poly_rectangle(conn, root_window, gc, 1, &rectangle);
+                xcb_poly_rectangle(conn, c->window_id, gc, 1, &rectangle);
                 rectangle = rectangle_from_corners(&press_position, &cursor_position);
-                xcb_poly_rectangle(conn, root_window, gc, 1, &rectangle);
+                xcb_poly_rectangle(conn, c->window_id, gc, 1, &rectangle);
             }
             break;
         }
         case XCB_BUTTON_RELEASE: {
-            xcb_poly_rectangle(conn, root_window, gc, 1, &rectangle);
+            xcb_poly_rectangle(conn, c->window_id, gc, 1, &rectangle);
             done = 1;
             break;
         }
@@ -830,6 +856,19 @@  static av_cold int xcbgrab_read_header(AVFormatContext *s)
         return AVERROR(EIO);
     }
 
+    if (c->window_id == XCB_NONE)
+        c->window_id = c->screen->root;
+    else {
+        if (c->select_region) {
+            av_log(s, AV_LOG_WARNING, "select_region ignored with window_id.\n");
+            c->select_region = 0;
+        }
+        if (c->follow_mouse) {
+            av_log(s, AV_LOG_WARNING, "follow_mouse ignored with window_id.\n");
+            c->follow_mouse = 0;
+        }
+    }
+
     if (c->select_region) {
         ret = select_region(s);
         if (ret < 0) {