On Sun, 28. Feb 13:57, sgerwk-at-aol....@ffmpeg.org wrote:
> Hi,
>
> On Sat, 27 Feb 2021, Andriy Gelman wrote:
>
> > On Mon, 22. Feb 17:53, sgerwk-at-aol....@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 <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.
> > > >
> > >
> > > 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.
>
> Ok, I removed those changes. But I still believe it's not the best choice to
> first generalize the grabbed window to be c->window_id but then assume it's
> c->screen->root_window instead only in this case.
The two cases are a bit different. One you grabbing the window, the second you
are drawing a rectangle for the region selection.
If I take you previous patch and force it to go into the region selection path,
the rectangle doesn't get drawn on the non root window.
>
> >
> > >
> > > > > > > + 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".
> > >
> > > Thanks!
> >
> > > From 36e6c5a858415106d4d2d9a76fa45cbd59717c77 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
> >
> >
> > 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
> >
>
> I used the wrong win_x and win_y in xcbgrab_reposition(). I reverted that
> function to its previous state, which also fixed the problem.
>
> > >
> > > ---
> > > 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;
> > > + }
> >
> > You could zero initialize win_x/win_y and skip this if statement.
>
> Done.
>
> >
> > > 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.
>
> Done.
>
> >
> > > + 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.
> >
>
> Done.
>
> >
> > > 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@ffmpeg.org
> > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >
> > > To unsubscribe, visit link above, or email
> > > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
> >
> >
> > 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".
> From 1a350c9de2acbf133d62c1eeb6b5e90dd0a5dc9d 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 | 70 ++++++++++++++++++++++++++++++++-----------
> 2 files changed, 65 insertions(+), 19 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..956ea32 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 },
> @@ -157,7 +159,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 +269,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 +335,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 +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, 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 +403,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 +420,19 @@ 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 = 0, win_y = 0;
>
> 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);
> + 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 +445,25 @@ 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) {
> + free(p);
> + 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);
>
> 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 +478,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 +590,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;
> @@ -830,6 +851,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
>
Thanks, your latest patch looks ok to me.
The code in setup_window could be improved to properly draw the bounding box and
flush the connection, but I can submit a patch for that later.
I hope someone else can look over the patch and give a second opinion.
If no one replies I'll apply the patch in a couple weeks.
--
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".
