On Wed, 10. Feb 18:15, sgerwk-at-aol....@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-requ...@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 <sge...@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, -- 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-requ...@ffmpeg.org with subject "unsubscribe".