[FFmpeg-devel] [PATCH] x11grab: capture a window instead of the whole screen
Andriy Gelman
andriy.gelman at gmail.com
Sun Feb 21 09:30:02 EET 2021
On Wed, 10. Feb 18:15, sgerwk-at-aol.com at 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.
>
> > >
> > > + at 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 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".
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 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.
> 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,
--
Andriy
More information about the ffmpeg-devel
mailing list