[FFmpeg-devel] [PATCH] x11grab: capture a window instead of the whole screen
Andriy Gelman
andriy.gelman at gmail.com
Sun Feb 28 02:22:23 EET 2021
On Mon, 22. Feb 17:53, sgerwk-at-aol.com at ffmpeg.org wrote:
> 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 at 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
> > >
> > > + at 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.
The selection rectangle may still be be drawn on root window.
The partial change will make it more confusing in the commit history.
It's better to leave it IMO.
>
> > >
> > > + 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 at ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>
> Thanks!
> From 36e6c5a858415106d4d2d9a76fa45cbd59717c77 Mon Sep 17 00:00:00 2001
> From: sgerwk <sgerwk at aol.com>
> Date: Wed, 10 Feb 2021 17:36:00 +0100
> Subject: [PATCH] libavdevice/xcbgrab: option for grabbing a window instead of
> desktop
The behavior with -select_region 1 -follow_mouse 1 is not correct now. It will
always reset the origin to 0,0, i.e.
./ffmpeg -show_region 1 select_region 1 -follow_mouse 1 -f x11grab -i :0 -codec:v mpeg2video out.mp4
>
> ---
> 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
>
> + at 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;
> + }
You could zero initialize win_x/win_y and skip this if statement.
> 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);
It's fine to parse NULL to free, so the checks are not needed.
> + 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);
I don't think we should change the xcbgrab_reposition() code. It's not that
useful to follow a mouse region in a window, and it's disabled with your option.
>
> 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) {
> --
> 2.30.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
Thanks,
--
Andriy
More information about the ffmpeg-devel
mailing list