| Message ID | 1478573842-14809-1-git-send-email-rp@t4d.cz |
|---|---|
| State | New |
| Headers | show |
Thanks for the patch. See comments below. > Subject: Re: [FFmpeg-devel] [PATCH] feature: xcbgrab single window The recommended syntax would be "lavd/xcbgrab: add single window mode". > Allows to grab only a single window by using syntax: > ffmpeg -f x11grab -r 15 -i ":0/0x0580001b/parent2" ... > The syntax for input is "display/grab_window_id/focus_window_id". I am not sure the "file name" is the best place for that, since it requires parsing. An option would be easier and probably more convenient, especially for focus_window_id. > If focus_window_id is omitted it is set as grab_window_id. > There are special constants for focus_window_id: > - this: grab_window_id > - parent: parent window id of grab_window, > - parent2: grand parent window id of grab_window, > - parent3: great grand parent window id of grab_window. This should go in the documentation. > Has a single command line option repeat_frame which controls > out of focus streaming. Turned on in default for repeating > the last frame. If turned off sends empty (zero length) packet > to the ffmpeg backend. I think the focus feature should go in a separate patch. You will find easier to get several smaller patches accepted. Actually, I think there are three separate features here: (1) following a window, (2) disabling the capture according to focus and (3) duplicating the frame when the capture is disabled or not possible. I am not entirely sure about the usefulness of duplicating the frame. The framerate controls later in the system would do the job nicely. > --- > Changelog | 1 + > libavdevice/xcbgrab.c | 210 +++++++++++++++++++++++++++++++++++++++++++++++--- > 2 files changed, 201 insertions(+), 10 deletions(-) > > diff --git a/Changelog b/Changelog > index 11a769a..eb83365 100644 > --- a/Changelog > +++ b/Changelog > @@ -3,6 +3,7 @@ releases are sorted from youngest to oldest. > > version <next>: > - CrystalHD decoder moved to new decode API > +- XCB-based screen-grabbing of single window > > version 3.2: > - libopenmpt demuxer > diff --git a/libavdevice/xcbgrab.c b/libavdevice/xcbgrab.c > index 702e66c..a27cdee 100644 > --- a/libavdevice/xcbgrab.c > +++ b/libavdevice/xcbgrab.c > @@ -1,6 +1,8 @@ > /* > * XCB input grabber > * Copyright (C) 2014 Luca Barbato <lu_zero@gentoo.org> > + * Copyright (C) 2016 Radomír Polách <rp@t4d.cz> > + * Copyright (C) 2016 Kristýna Dudová <kd@t4d.cz> > * > * This file is part of FFmpeg. > * > @@ -70,11 +72,21 @@ typedef struct XCBGrabContext { > int show_region; > int region_border; > int centered; > + int repeat_frame; > > const char *video_size; > const char *framerate; > > int has_shm; > + > + char *focus_name; > + char *grab_name; > + xcb_window_t focus_window; > + xcb_window_t grab_window; > + > + uint8_t *data; > + int size; > + int warned; > } XCBGrabContext; > > #define FOLLOW_CENTER -1 > @@ -88,6 +100,7 @@ static const AVOption options[] = { > { "grab_y", "Initial y coordinate.", OFFSET(y), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, D }, > { "video_size", "A string describing frame size, such as 640x480 or hd720.", OFFSET(video_size), AV_OPT_TYPE_STRING, {.str = "vga" }, 0, 0, D }, > { "framerate", "", OFFSET(framerate), AV_OPT_TYPE_STRING, {.str = "ntsc" }, 0, 0, D }, > + { "repeat_frame", "Repeat last frame, when window is not active or data are not available.", OFFSET(repeat_frame), AV_OPT_TYPE_INT, { .i64 = 1 }, 0, 1, D }, Looks like a boolean. (Boolean did not exist when draw_mouse or follow_mouse were implemented here.) > { "draw_mouse", "Draw the mouse pointer.", OFFSET(draw_mouse), AV_OPT_TYPE_INT, { .i64 = 1 }, 0, 1, D }, > { "follow_mouse", "Move the grabbing region when the mouse pointer reaches within specified amount of pixels to the edge of region.", > OFFSET(follow_mouse), AV_OPT_TYPE_INT, { .i64 = 0 }, FOLLOW_CENTER, INT_MAX, D, "follow_mouse" }, > @@ -156,9 +169,13 @@ static int xcbgrab_frame(AVFormatContext *s, AVPacket *pkt) > uint8_t *data; > int length, ret; > > - iq = xcb_get_image(c->conn, XCB_IMAGE_FORMAT_Z_PIXMAP, drawable, > - c->x, c->y, c->width, c->height, ~0); > - > + if (c->focus_name) { > + iq = xcb_get_image(c->conn, XCB_IMAGE_FORMAT_Z_PIXMAP, c->grab_window, > + 0, 0, c->width, c->height, ~0); > + } else { > + iq = xcb_get_image(c->conn, XCB_IMAGE_FORMAT_Z_PIXMAP, drawable, > + c->x, c->y, c->width, c->height, ~0); > + } It would be better to avoid duplicating the function call and instead make sure drawable, c->x and c->y have the correct value. Your patch does not seem to make use of c->x and c->y: capturing only part of a window would be useful too. > img = xcb_get_image_reply(c->conn, iq, &e); > > if (e) { > @@ -261,9 +278,16 @@ static int xcbgrab_frame_shm(AVFormatContext *s, AVPacket *pkt) > if (ret < 0) > return ret; > > - iq = xcb_shm_get_image(c->conn, drawable, > - c->x, c->y, c->width, c->height, ~0, > - XCB_IMAGE_FORMAT_Z_PIXMAP, c->segment, 0); > + if (c->focus_name) { > + iq = xcb_shm_get_image(c->conn, c->grab_window, > + 0, 0, c->width, c->height, ~0, > + XCB_IMAGE_FORMAT_Z_PIXMAP, c->segment, 0); > + } else { > + iq = xcb_shm_get_image(c->conn, drawable, > + c->x, c->y, c->width, c->height, ~0, > + XCB_IMAGE_FORMAT_Z_PIXMAP, c->segment, 0); > + } Same remark as above. > + Stray empty line. > img = xcb_shm_get_image_reply(c->conn, iq, &e); > > xcb_flush(c->conn); > @@ -329,8 +353,13 @@ static void xcbgrab_draw_mouse(AVFormatContext *s, AVPacket *pkt, > if (!cursor) > return; > > - cx = ci->x - ci->xhot; > - cy = ci->y - ci->yhot; > + if (gr->focus_name) { > + cx = p->win_x - ci->xhot; > + cy = p->win_y - ci->yhot; > + } else { > + cx = ci->x - ci->xhot; > + cy = ci->y - ci->yhot; > + } > > x = FFMAX(cx, gr->x); > y = FFMAX(cy, gr->y); > @@ -389,6 +418,68 @@ static void xcbgrab_update_region(AVFormatContext *s) > args); > } > > +static xcb_window_t get_window_focus(AVFormatContext *s) > +{ > + XCBGrabContext *ctx = s->priv_data; > + xcb_window_t w = 0; > + xcb_get_input_focus_cookie_t c; > + xcb_get_input_focus_reply_t *r; > + > + c = xcb_get_input_focus(ctx->conn); > + r = xcb_get_input_focus_reply(ctx->conn, c, NULL); > + if (!r) > + return -1; xcb_window_t is unsigned. If you use that type, you can only use the special values specified by the X11 protocol. Since you only use the result to compare to the current focused window, you could take the comparison peer as an argument and return a boolean. > + > + w = r->focus; > + free(r); > + return w; > +} > + > +static xcb_window_t get_window_parent(AVFormatContext *s, xcb_window_t w) > +{ > + XCBGrabContext *ctx = s->priv_data; > + xcb_query_tree_cookie_t c; > + xcb_query_tree_reply_t *r; > + xcb_generic_error_t *e; > + > + c = xcb_query_tree(ctx->conn, w); > + r = xcb_query_tree_reply(ctx->conn, c, &e); > + if (!r) > + return -1; Same problem here. > + > + w = r->parent; > + free(r); > + return w; > +} > + > +static void xcbgrab_store_packet(AVFormatContext *s, AVPacket *pkt) { Inconsistent placement of brace. Another similar below. > + XCBGrabContext *c = s->priv_data; > + > + c->size = pkt->size; > + > + if (c->size) { > + if (!c->data) { > + c->data = av_malloc(c->size); > + } > + memcpy(c->data, pkt->data, c->size); > + } > +} This looks strange. Was this made before my commit to disable refcounting of the packets? If so, it will need to be rebased and updated. But on the other hand, the logic to keep the current packet should be much simpler. > + > +static int xcbgrab_load_packet(AVFormatContext *s, AVPacket *pkt) { > + XCBGrabContext *c = s->priv_data; > + int ret; > + > + if (!c->size) > + return 0; > + > + ret = av_new_packet(pkt, c->size); > + > + if (!ret) > + memcpy(pkt->data, c->data, c->size); > + > + return ret; > +} > + > static int xcbgrab_read_packet(AVFormatContext *s, AVPacket *pkt) > { > XCBGrabContext *c = s->priv_data; > @@ -400,11 +491,56 @@ static int xcbgrab_read_packet(AVFormatContext *s, AVPacket *pkt) > > wait_frame(s, pkt); > > + if (c->focus_name) { > + xcb_window_t w = get_window_focus(s); > + if (w != c->focus_window) { > + if (!c->warned) { > + c->warned = 1; > + av_log(s, AV_LOG_WARNING, > + "Not grabbing, focus window not focused, focused window is 0x%08x.\n", w); w needs to be cast to int. > + } > + if (c->repeat_frame) { > + return xcbgrab_load_packet(s, pkt); > + } else { > + return 0; > + } > + } else { > + if (c->warned == 1) { > + c->warned = 0; > + av_log(s, AV_LOG_WARNING, > + "Grabbing resumed.\n"); > + } > + } > + } > + > 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->focus_name) { > + pc = xcb_query_pointer(c->conn, c->grab_window); > + gc = xcb_get_geometry(c->conn, c->grab_window); > + } else { > + pc = xcb_query_pointer(c->conn, c->screen->root); > + gc = xcb_get_geometry(c->conn, c->screen->root); > + } Same as before: better set a variable with the correct grab id than duplicating the calls. > p = xcb_query_pointer_reply(c->conn, pc, NULL); > geo = xcb_get_geometry_reply(c->conn, gc, NULL); > + if (geo->width < c->width || geo->height < c->height) { > + if (!c->warned) { > + c->warned = 2; > + av_log(s, AV_LOG_WARNING, > + "Not grabbing, grab window width or height lower than initial.\n"); > + } > + if (c->repeat_frame) { > + return xcbgrab_load_packet(s, pkt); > + } else { > + return 0; Is this returning an empty packet? I am not sure it is really legal here. > + } > + } else { > + if (c->warned == 2) { > + c->warned = 0; > + av_log(s, AV_LOG_WARNING, > + "Grabbing resumed.\n"); > + } > + } > } > > if (c->follow_mouse && p->same_screen) > @@ -425,6 +561,10 @@ static int xcbgrab_read_packet(AVFormatContext *s, AVPacket *pkt) > xcbgrab_draw_mouse(s, pkt, p, geo); > #endif > > + if (c->repeat_frame) { > + xcbgrab_store_packet(s, pkt); > + } > + > free(p); > free(geo); > > @@ -443,6 +583,10 @@ static av_cold int xcbgrab_read_close(AVFormatContext *s) > > xcb_disconnect(ctx->conn); > > + if (ctx->data) { > + free(ctx->data); av_free(). > + } > + > return 0; > } > > @@ -535,6 +679,19 @@ static int create_stream(AVFormatContext *s) > > avpriv_set_pts_info(st, 64, 1, 1000000); > > + if (c->focus_name) { > + gc = xcb_get_geometry(c->conn, c->grab_window); > + geo = xcb_get_geometry_reply(c->conn, gc, NULL); > + if (!geo) { > + av_log(s, AV_LOG_ERROR, > + "Grab window 0x%08x does not exist.\n", > + c->grab_window); Cast to int. > + return AVERROR(EINVAL); > + } > + c->width = geo->width & ~1; > + c->height = geo->height & ~1; Why the & ~1? > + } > + > gc = xcb_get_geometry(c->conn, c->screen->root); > geo = xcb_get_geometry_reply(c->conn, gc, NULL); > > @@ -630,6 +787,17 @@ static av_cold int xcbgrab_read_header(AVFormatContext *s) > int screen_num, ret; > const xcb_setup_t *setup; > char *display_name = av_strdup(s->filename); > + if (c->focus_name = strchr(s->filename, '/')) { > + c->focus_name[0] = '\0'; > + ++c->focus_name; > + } > + if (c->grab_name = strchr(c->focus_name, '/')) { > + c->grab_name[0] = '\0'; > + ++c->grab_name; > + } I do not think you are authorized to change filename like that. > + c->data = NULL; > + c->size = 0; > + c->warned = 0; Not necessary. > > if (!display_name) > return AVERROR(ENOMEM); > @@ -658,6 +826,28 @@ static av_cold int xcbgrab_read_header(AVFormatContext *s) > return AVERROR(EIO); > } > > + if (c->focus_name) { > + c->focus_window = strtoul(c->focus_name, NULL, 16); Use 0 as base, that allows users to use either decimal or 0xHEXADECIMAL. > + if (c->grab_name) { > + if (!strcmp(c->grab_name,"this")) { > + c->grab_window = c->focus_window; > + } else if (!strcmp(c->grab_name,"parent")) { > + c->grab_window = get_window_parent(s, c->focus_window); > + } else if (!strcmp(c->grab_name,"parent2")) { > + c->grab_window = get_window_parent(s, c->focus_window); > + c->grab_window = get_window_parent(s, c->grab_window); > + } else if (!strcmp(c->grab_name,"parent3")) { > + c->grab_window = get_window_parent(s, c->focus_window); > + c->grab_window = get_window_parent(s, c->grab_window); > + c->grab_window = get_window_parent(s, c->grab_window); > + } else { > + c->grab_window = strtoul(c->grab_name, NULL, 16); > + } > + } else { > + c->grab_window = get_window_parent(s, c->focus_window); > + } > + } > + > ret = create_stream(s); > > if (ret < 0) { Regards,
2016-11-11 11:35 GMT+01:00 Nicolas George <george@nsup.org>: > Thanks for the patch. See comments below. > > > Subject: Re: [FFmpeg-devel] [PATCH] feature: xcbgrab single window > > The recommended syntax would be "lavd/xcbgrab: add single window mode". > Ok. > > > Allows to grab only a single window by using syntax: > > ffmpeg -f x11grab -r 15 -i ":0/0x0580001b/parent2" ... > > > The syntax for input is "display/grab_window_id/focus_window_id". > > I am not sure the "file name" is the best place for that, since it > requires parsing. An option would be easier and probably more > convenient, especially for focus_window_id. > I am not sure either, but I think it is better this way, because you can easily use it to pass result from a search script: ./ffmpeg -i $(./findmypaint.sh) ... Search script is a simple script which finds for you screen, focus window and grab window of some program. For example findmypaint.sh here finds this information about MyPaint. The search script just needs to produce output like this: :0/0x0580001b/parent2 It is very flexible this way. > > If focus_window_id is omitted it is set as grab_window_id. > > There are special constants for focus_window_id: > > - this: grab_window_id > > - parent: parent window id of grab_window, > > - parent2: grand parent window id of grab_window, > > - parent3: great grand parent window id of grab_window. > > This should go in the documentation. Ok. > > > Has a single command line option repeat_frame which controls > > out of focus streaming. Turned on in default for repeating > > the last frame. If turned off sends empty (zero length) packet > > to the ffmpeg backend. > > I think the focus feature should go in a separate patch. You will find > easier to get several smaller patches accepted. > > Actually, I think there are three separate features here: (1) following > a window, (2) disabling the capture according to focus and (3) > duplicating the frame when the capture is disabled or not possible. > > I am not entirely sure about the usefulness of duplicating the frame. > The framerate controls later in the system would do the job nicely. > I tought so too, but I tested it with RTMP streaming and players disconnects without repeat_frame. I am not sure where the problem is. Didn't know about existence of boolean there. Should I also take care of draw_mouse and follow_mouse in a separate patch? > > > { "draw_mouse", "Draw the mouse pointer.", OFFSET(draw_mouse), > AV_OPT_TYPE_INT, { .i64 = 1 }, 0, 1, D }, > > { "follow_mouse", "Move the grabbing region when the mouse pointer > reaches within specified amount of pixels to the edge of region.", > > OFFSET(follow_mouse), AV_OPT_TYPE_INT, { .i64 = 0 }, > FOLLOW_CENTER, INT_MAX, D, "follow_mouse" }, > > @@ -156,9 +169,13 @@ static int xcbgrab_frame(AVFormatContext *s, > AVPacket *pkt) > > uint8_t *data; > > int length, ret; > > > > > - iq = xcb_get_image(c->conn, XCB_IMAGE_FORMAT_Z_PIXMAP, drawable, > > - c->x, c->y, c->width, c->height, ~0); > > - > > + if (c->focus_name) { > > + iq = xcb_get_image(c->conn, XCB_IMAGE_FORMAT_Z_PIXMAP, > c->grab_window, > > + 0, 0, c->width, c->height, ~0); > > + } else { > > + iq = xcb_get_image(c->conn, XCB_IMAGE_FORMAT_Z_PIXMAP, > drawable, > > + c->x, c->y, c->width, c->height, ~0); > > + } > > It would be better to avoid duplicating the function call and instead > make sure drawable, c->x and c->y have the correct value. Your patch > does not seem to make use of c->x and c->y: capturing only part of a > window would be useful too. > Good idea about capturing part of a window and single call. Will redo this. > > > img = xcb_get_image_reply(c->conn, iq, &e); > > > > if (e) { > > @@ -261,9 +278,16 @@ static int xcbgrab_frame_shm(AVFormatContext *s, > AVPacket *pkt) > > if (ret < 0) > > return ret; > > > > > - iq = xcb_shm_get_image(c->conn, drawable, > > - c->x, c->y, c->width, c->height, ~0, > > - XCB_IMAGE_FORMAT_Z_PIXMAP, c->segment, 0); > > + if (c->focus_name) { > > + iq = xcb_shm_get_image(c->conn, c->grab_window, > > + 0, 0, c->width, c->height, ~0, > > + XCB_IMAGE_FORMAT_Z_PIXMAP, c->segment, > 0); > > + } else { > > + iq = xcb_shm_get_image(c->conn, drawable, > > + c->x, c->y, c->width, c->height, ~0, > > + XCB_IMAGE_FORMAT_Z_PIXMAP, c->segment, > 0); > > + } > > Same remark as above. > > Ok. > > + > > Stray empty line. > > Ok. Will improve this. > > + > > + w = r->focus; > > + free(r); > > + return w; > > +} > > + > > +static xcb_window_t get_window_parent(AVFormatContext *s, xcb_window_t > w) > > +{ > > + XCBGrabContext *ctx = s->priv_data; > > + xcb_query_tree_cookie_t c; > > + xcb_query_tree_reply_t *r; > > + xcb_generic_error_t *e; > > + > > + c = xcb_query_tree(ctx->conn, w); > > + r = xcb_query_tree_reply(ctx->conn, c, &e); > > > + if (!r) > > + return -1; > > Same problem here. > Ok. > > > + > > + w = r->parent; > > + free(r); > > + return w; > > +} > > + > > > +static void xcbgrab_store_packet(AVFormatContext *s, AVPacket *pkt) { > > Inconsistent placement of brace. Another similar below. > Ok. > > > + XCBGrabContext *c = s->priv_data; > > + > > + c->size = pkt->size; > > + > > + if (c->size) { > > + if (!c->data) { > > + c->data = av_malloc(c->size); > > + } > > + memcpy(c->data, pkt->data, c->size); > > + } > > +} > > This looks strange. Was this made before my commit to disable > refcounting of the packets? If so, it will need to be rebased and > updated. But on the other hand, the logic to keep the current packet > should be much simpler. > Could you point me out to your commit and the problem? This is my first patch to ffmpeg and I am not that familiar with the project. Ok. Ok. > > p = xcb_query_pointer_reply(c->conn, pc, NULL); > > geo = xcb_get_geometry_reply(c->conn, gc, NULL); > > + if (geo->width < c->width || geo->height < c->height) { > > + if (!c->warned) { > > + c->warned = 2; > > + av_log(s, AV_LOG_WARNING, > > + "Not grabbing, grab window width or height lower > than initial.\n"); > > + } > > + if (c->repeat_frame) { > > + return xcbgrab_load_packet(s, pkt); > > + } else { > > > + return 0; > > Is this returning an empty packet? I am not sure it is really legal > here. > Maybe this is the reason for RTMP disconnects. Is there a way to not return any data? I thought this would work. > > > + } > > + } else { > > + if (c->warned == 2) { > > + c->warned = 0; > > + av_log(s, AV_LOG_WARNING, > > + "Grabbing resumed.\n"); > > + } > > + } > > } > > > > if (c->follow_mouse && p->same_screen) > > @@ -425,6 +561,10 @@ static int xcbgrab_read_packet(AVFormatContext *s, > AVPacket *pkt) > > xcbgrab_draw_mouse(s, pkt, p, geo); > > #endif > > > > + if (c->repeat_frame) { > > + xcbgrab_store_packet(s, pkt); > > + } > > + > > free(p); > > free(geo); > > > > @@ -443,6 +583,10 @@ static av_cold int xcbgrab_read_close(AVFormatContext > *s) > > > > xcb_disconnect(ctx->conn); > > > > + if (ctx->data) { > > > + free(ctx->data); > > av_free(). > Ok. > > > + } > > + > > return 0; > > } > > > > @@ -535,6 +679,19 @@ static int create_stream(AVFormatContext *s) > > > > avpriv_set_pts_info(st, 64, 1, 1000000); > > > > + if (c->focus_name) { > > + gc = xcb_get_geometry(c->conn, c->grab_window); > > + geo = xcb_get_geometry_reply(c->conn, gc, NULL); > > + if (!geo) { > > + av_log(s, AV_LOG_ERROR, > > + "Grab window 0x%08x does not exist.\n", > > > + c->grab_window); > > Cast to int. > Ok. > > > + return AVERROR(EINVAL); > > + } > > > + c->width = geo->width & ~1; > > + c->height = geo->height & ~1; > > Why the & ~1? > I remember doint it for a reason, not sure what the reason was. Will remove it and test how it behaves. > > > + } > > + > > gc = xcb_get_geometry(c->conn, c->screen->root); > > geo = xcb_get_geometry_reply(c->conn, gc, NULL); > > > > @@ -630,6 +787,17 @@ static av_cold int xcbgrab_read_header(AVFormatContext > *s) > > int screen_num, ret; > > const xcb_setup_t *setup; > > char *display_name = av_strdup(s->filename); > > > + if (c->focus_name = strchr(s->filename, '/')) { > > + c->focus_name[0] = '\0'; > > + ++c->focus_name; > > + } > > + if (c->grab_name = strchr(c->focus_name, '/')) { > > + c->grab_name[0] = '\0'; > > + ++c->grab_name; > > + } > > I do not think you are authorized to change filename like that. > I can duplicate the variable, but didn't notice breaking of anything. > > > + c->data = NULL; > > + c->size = 0; > > + c->warned = 0; > > Not necessary. > Ok. > > > > > if (!display_name) > > return AVERROR(ENOMEM); > > @@ -658,6 +826,28 @@ static av_cold int xcbgrab_read_header(AVFormatContext > *s) > > return AVERROR(EIO); > > } > > > > + if (c->focus_name) { > > > + c->focus_window = strtoul(c->focus_name, NULL, 16); > > Use 0 as base, that allows users to use either decimal or 0xHEXADECIMAL. > Good idea. > > > + if (c->grab_name) { > > + if (!strcmp(c->grab_name,"this")) { > > + c->grab_window = c->focus_window; > > + } else if (!strcmp(c->grab_name,"parent")) { > > + c->grab_window = get_window_parent(s, c->focus_window); > > + } else if (!strcmp(c->grab_name,"parent2")) { > > + c->grab_window = get_window_parent(s, c->focus_window); > > + c->grab_window = get_window_parent(s, c->grab_window); > > + } else if (!strcmp(c->grab_name,"parent3")) { > > + c->grab_window = get_window_parent(s, c->focus_window); > > + c->grab_window = get_window_parent(s, c->grab_window); > > + c->grab_window = get_window_parent(s, c->grab_window); > > + } else { > > + c->grab_window = strtoul(c->grab_name, NULL, 16); > > + } > > + } else { > > + c->grab_window = get_window_parent(s, c->focus_window); > > + } > > + } > > + > > ret = create_stream(s); > > > > if (ret < 0) { > > Regards, > > -- > Nicolas George 2016-11-11 11:35 GMT+01:00 Nicolas George <george@nsup.org>: > Thanks for the patch. See comments below. > > > Subject: Re: [FFmpeg-devel] [PATCH] feature: xcbgrab single window > > The recommended syntax would be "lavd/xcbgrab: add single window mode". > > > Allows to grab only a single window by using syntax: > > ffmpeg -f x11grab -r 15 -i ":0/0x0580001b/parent2" ... > > > The syntax for input is "display/grab_window_id/focus_window_id". > > I am not sure the "file name" is the best place for that, since it > requires parsing. An option would be easier and probably more > convenient, especially for focus_window_id. > > > If focus_window_id is omitted it is set as grab_window_id. > > There are special constants for focus_window_id: > > - this: grab_window_id > > - parent: parent window id of grab_window, > > - parent2: grand parent window id of grab_window, > > - parent3: great grand parent window id of grab_window. > > This should go in the documentation. > > > Has a single command line option repeat_frame which controls > > out of focus streaming. Turned on in default for repeating > > the last frame. If turned off sends empty (zero length) packet > > to the ffmpeg backend. > > I think the focus feature should go in a separate patch. You will find > easier to get several smaller patches accepted. > > Actually, I think there are three separate features here: (1) following > a window, (2) disabling the capture according to focus and (3) > duplicating the frame when the capture is disabled or not possible. > > I am not entirely sure about the usefulness of duplicating the frame. > The framerate controls later in the system would do the job nicely. > > > --- > > Changelog | 1 + > > libavdevice/xcbgrab.c | 210 ++++++++++++++++++++++++++++++ > +++++++++++++++++--- > > 2 files changed, 201 insertions(+), 10 deletions(-) > > > > diff --git a/Changelog b/Changelog > > index 11a769a..eb83365 100644 > > --- a/Changelog > > +++ b/Changelog > > @@ -3,6 +3,7 @@ releases are sorted from youngest to oldest. > > > > version <next>: > > - CrystalHD decoder moved to new decode API > > +- XCB-based screen-grabbing of single window > > > > version 3.2: > > - libopenmpt demuxer > > diff --git a/libavdevice/xcbgrab.c b/libavdevice/xcbgrab.c > > index 702e66c..a27cdee 100644 > > --- a/libavdevice/xcbgrab.c > > +++ b/libavdevice/xcbgrab.c > > @@ -1,6 +1,8 @@ > > /* > > * XCB input grabber > > * Copyright (C) 2014 Luca Barbato <lu_zero@gentoo.org> > > + * Copyright (C) 2016 Radomír Polách <rp@t4d.cz> > > + * Copyright (C) 2016 Kristýna Dudová <kd@t4d.cz> > > * > > * This file is part of FFmpeg. > > * > > @@ -70,11 +72,21 @@ typedef struct XCBGrabContext { > > int show_region; > > int region_border; > > int centered; > > + int repeat_frame; > > > > const char *video_size; > > const char *framerate; > > > > int has_shm; > > + > > + char *focus_name; > > + char *grab_name; > > + xcb_window_t focus_window; > > + xcb_window_t grab_window; > > + > > + uint8_t *data; > > + int size; > > + int warned; > > } XCBGrabContext; > > > > #define FOLLOW_CENTER -1 > > @@ -88,6 +100,7 @@ static const AVOption options[] = { > > { "grab_y", "Initial y coordinate.", OFFSET(y), AV_OPT_TYPE_INT, { > .i64 = 0 }, 0, INT_MAX, D }, > > { "video_size", "A string describing frame size, such as 640x480 or > hd720.", OFFSET(video_size), AV_OPT_TYPE_STRING, {.str = "vga" }, 0, 0, D }, > > { "framerate", "", OFFSET(framerate), AV_OPT_TYPE_STRING, {.str = > "ntsc" }, 0, 0, D }, > > > + { "repeat_frame", "Repeat last frame, when window is not active or > data are not available.", OFFSET(repeat_frame), AV_OPT_TYPE_INT, { .i64 = 1 > }, 0, 1, D }, > > Looks like a boolean. > > (Boolean did not exist when draw_mouse or follow_mouse were implemented > here.) > > > { "draw_mouse", "Draw the mouse pointer.", OFFSET(draw_mouse), > AV_OPT_TYPE_INT, { .i64 = 1 }, 0, 1, D }, > > { "follow_mouse", "Move the grabbing region when the mouse pointer > reaches within specified amount of pixels to the edge of region.", > > OFFSET(follow_mouse), AV_OPT_TYPE_INT, { .i64 = 0 }, > FOLLOW_CENTER, INT_MAX, D, "follow_mouse" }, > > @@ -156,9 +169,13 @@ static int xcbgrab_frame(AVFormatContext *s, > AVPacket *pkt) > > uint8_t *data; > > int length, ret; > > > > > - iq = xcb_get_image(c->conn, XCB_IMAGE_FORMAT_Z_PIXMAP, drawable, > > - c->x, c->y, c->width, c->height, ~0); > > - > > + if (c->focus_name) { > > + iq = xcb_get_image(c->conn, XCB_IMAGE_FORMAT_Z_PIXMAP, > c->grab_window, > > + 0, 0, c->width, c->height, ~0); > > + } else { > > + iq = xcb_get_image(c->conn, XCB_IMAGE_FORMAT_Z_PIXMAP, > drawable, > > + c->x, c->y, c->width, c->height, ~0); > > + } > > It would be better to avoid duplicating the function call and instead > make sure drawable, c->x and c->y have the correct value. Your patch > does not seem to make use of c->x and c->y: capturing only part of a > window would be useful too. > > > img = xcb_get_image_reply(c->conn, iq, &e); > > > > if (e) { > > @@ -261,9 +278,16 @@ static int xcbgrab_frame_shm(AVFormatContext *s, > AVPacket *pkt) > > if (ret < 0) > > return ret; > > > > > - iq = xcb_shm_get_image(c->conn, drawable, > > - c->x, c->y, c->width, c->height, ~0, > > - XCB_IMAGE_FORMAT_Z_PIXMAP, c->segment, 0); > > + if (c->focus_name) { > > + iq = xcb_shm_get_image(c->conn, c->grab_window, > > + 0, 0, c->width, c->height, ~0, > > + XCB_IMAGE_FORMAT_Z_PIXMAP, c->segment, > 0); > > + } else { > > + iq = xcb_shm_get_image(c->conn, drawable, > > + c->x, c->y, c->width, c->height, ~0, > > + XCB_IMAGE_FORMAT_Z_PIXMAP, c->segment, > 0); > > + } > > Same remark as above. > > > + > > Stray empty line. > > > img = xcb_shm_get_image_reply(c->conn, iq, &e); > > > > xcb_flush(c->conn); > > @@ -329,8 +353,13 @@ static void xcbgrab_draw_mouse(AVFormatContext *s, > AVPacket *pkt, > > if (!cursor) > > return; > > > > - cx = ci->x - ci->xhot; > > - cy = ci->y - ci->yhot; > > + if (gr->focus_name) { > > + cx = p->win_x - ci->xhot; > > + cy = p->win_y - ci->yhot; > > + } else { > > + cx = ci->x - ci->xhot; > > + cy = ci->y - ci->yhot; > > + } > > > > x = FFMAX(cx, gr->x); > > y = FFMAX(cy, gr->y); > > @@ -389,6 +418,68 @@ static void xcbgrab_update_region(AVFormatContext > *s) > > args); > > } > > > > +static xcb_window_t get_window_focus(AVFormatContext *s) > > +{ > > + XCBGrabContext *ctx = s->priv_data; > > + xcb_window_t w = 0; > > + xcb_get_input_focus_cookie_t c; > > + xcb_get_input_focus_reply_t *r; > > + > > + c = xcb_get_input_focus(ctx->conn); > > + r = xcb_get_input_focus_reply(ctx->conn, c, NULL); > > > + if (!r) > > + return -1; > > xcb_window_t is unsigned. If you use that type, you can only use the > special values specified by the X11 protocol. > > Since you only use the result to compare to the current focused window, > you could take the comparison peer as an argument and return a boolean. > > > + > > + w = r->focus; > > + free(r); > > + return w; > > +} > > + > > +static xcb_window_t get_window_parent(AVFormatContext *s, xcb_window_t > w) > > +{ > > + XCBGrabContext *ctx = s->priv_data; > > + xcb_query_tree_cookie_t c; > > + xcb_query_tree_reply_t *r; > > + xcb_generic_error_t *e; > > + > > + c = xcb_query_tree(ctx->conn, w); > > + r = xcb_query_tree_reply(ctx->conn, c, &e); > > > + if (!r) > > + return -1; > > Same problem here. > > > + > > + w = r->parent; > > + free(r); > > + return w; > > +} > > + > > > +static void xcbgrab_store_packet(AVFormatContext *s, AVPacket *pkt) { > > Inconsistent placement of brace. Another similar below. > > > + XCBGrabContext *c = s->priv_data; > > + > > + c->size = pkt->size; > > + > > + if (c->size) { > > + if (!c->data) { > > + c->data = av_malloc(c->size); > > + } > > + memcpy(c->data, pkt->data, c->size); > > + } > > +} > > This looks strange. Was this made before my commit to disable > refcounting of the packets? If so, it will need to be rebased and > updated. But on the other hand, the logic to keep the current packet > should be much simpler. > > > + > > +static int xcbgrab_load_packet(AVFormatContext *s, AVPacket *pkt) { > > + XCBGrabContext *c = s->priv_data; > > + int ret; > > + > > + if (!c->size) > > + return 0; > > + > > + ret = av_new_packet(pkt, c->size); > > + > > + if (!ret) > > + memcpy(pkt->data, c->data, c->size); > > + > > + return ret; > > +} > > + > > static int xcbgrab_read_packet(AVFormatContext *s, AVPacket *pkt) > > { > > XCBGrabContext *c = s->priv_data; > > @@ -400,11 +491,56 @@ static int xcbgrab_read_packet(AVFormatContext > *s, AVPacket *pkt) > > > > wait_frame(s, pkt); > > > > + if (c->focus_name) { > > + xcb_window_t w = get_window_focus(s); > > > + if (w != c->focus_window) { > > + if (!c->warned) { > > + c->warned = 1; > > + av_log(s, AV_LOG_WARNING, > > > + "Not grabbing, focus window not focused, focused > window is 0x%08x.\n", w); > > w needs to be cast to int. > > > + } > > + if (c->repeat_frame) { > > + return xcbgrab_load_packet(s, pkt); > > + } else { > > + return 0; > > + } > > + } else { > > + if (c->warned == 1) { > > + c->warned = 0; > > + av_log(s, AV_LOG_WARNING, > > + "Grabbing resumed.\n"); > > + } > > + } > > + } > > + > > 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->focus_name) { > > + pc = xcb_query_pointer(c->conn, c->grab_window); > > + gc = xcb_get_geometry(c->conn, c->grab_window); > > + } else { > > + pc = xcb_query_pointer(c->conn, c->screen->root); > > + gc = xcb_get_geometry(c->conn, c->screen->root); > > + } > > Same as before: better set a variable with the correct grab id than > duplicating the calls. > > > p = xcb_query_pointer_reply(c->conn, pc, NULL); > > geo = xcb_get_geometry_reply(c->conn, gc, NULL); > > + if (geo->width < c->width || geo->height < c->height) { > > + if (!c->warned) { > > + c->warned = 2; > > + av_log(s, AV_LOG_WARNING, > > + "Not grabbing, grab window width or height lower > than initial.\n"); > > + } > > + if (c->repeat_frame) { > > + return xcbgrab_load_packet(s, pkt); > > + } else { > > > + return 0; > > Is this returning an empty packet? I am not sure it is really legal > here. > > > + } > > + } else { > > + if (c->warned == 2) { > > + c->warned = 0; > > + av_log(s, AV_LOG_WARNING, > > + "Grabbing resumed.\n"); > > + } > > + } > > } > > > > if (c->follow_mouse && p->same_screen) > > @@ -425,6 +561,10 @@ static int xcbgrab_read_packet(AVFormatContext *s, > AVPacket *pkt) > > xcbgrab_draw_mouse(s, pkt, p, geo); > > #endif > > > > + if (c->repeat_frame) { > > + xcbgrab_store_packet(s, pkt); > > + } > > + > > free(p); > > free(geo); > > > > @@ -443,6 +583,10 @@ static av_cold int xcbgrab_read_close(AVFormatContext > *s) > > > > xcb_disconnect(ctx->conn); > > > > + if (ctx->data) { > > > + free(ctx->data); > > av_free(). > > > + } > > + > > return 0; > > } > > > > @@ -535,6 +679,19 @@ static int create_stream(AVFormatContext *s) > > > > avpriv_set_pts_info(st, 64, 1, 1000000); > > > > + if (c->focus_name) { > > + gc = xcb_get_geometry(c->conn, c->grab_window); > > + geo = xcb_get_geometry_reply(c->conn, gc, NULL); > > + if (!geo) { > > + av_log(s, AV_LOG_ERROR, > > + "Grab window 0x%08x does not exist.\n", > > > + c->grab_window); > > Cast to int. > > > + return AVERROR(EINVAL); > > + } > > > + c->width = geo->width & ~1; > > + c->height = geo->height & ~1; > > Why the & ~1? > > > + } > > + > > gc = xcb_get_geometry(c->conn, c->screen->root); > > geo = xcb_get_geometry_reply(c->conn, gc, NULL); > > > > @@ -630,6 +787,17 @@ static av_cold int xcbgrab_read_header(AVFormatContext > *s) > > int screen_num, ret; > > const xcb_setup_t *setup; > > char *display_name = av_strdup(s->filename); > > > + if (c->focus_name = strchr(s->filename, '/')) { > > + c->focus_name[0] = '\0'; > > + ++c->focus_name; > > + } > > + if (c->grab_name = strchr(c->focus_name, '/')) { > > + c->grab_name[0] = '\0'; > > + ++c->grab_name; > > + } > > I do not think you are authorized to change filename like that. > > > + c->data = NULL; > > + c->size = 0; > > + c->warned = 0; > > Not necessary. > > > > > if (!display_name) > > return AVERROR(ENOMEM); > > @@ -658,6 +826,28 @@ static av_cold int xcbgrab_read_header(AVFormatContext > *s) > > return AVERROR(EIO); > > } > > > > + if (c->focus_name) { > > > + c->focus_window = strtoul(c->focus_name, NULL, 16); > > Use 0 as base, that allows users to use either decimal or 0xHEXADECIMAL. > > > + if (c->grab_name) { > > + if (!strcmp(c->grab_name,"this")) { > > + c->grab_window = c->focus_window; > > + } else if (!strcmp(c->grab_name,"parent")) { > > + c->grab_window = get_window_parent(s, c->focus_window); > > + } else if (!strcmp(c->grab_name,"parent2")) { > > + c->grab_window = get_window_parent(s, c->focus_window); > > + c->grab_window = get_window_parent(s, c->grab_window); > > + } else if (!strcmp(c->grab_name,"parent3")) { > > + c->grab_window = get_window_parent(s, c->focus_window); > > + c->grab_window = get_window_parent(s, c->grab_window); > > + c->grab_window = get_window_parent(s, c->grab_window); > > + } else { > > + c->grab_window = strtoul(c->grab_name, NULL, 16); > > + } > > + } else { > > + c->grab_window = get_window_parent(s, c->focus_window); > > + } > > + } > > + > > ret = create_stream(s); > > > > if (ret < 0) { > > Regards, > > -- > Nicolas George >
Le duodi 22 brumaire, an CCXXV, Ing. Radomír Polách a écrit : > I am not sure either, but I think it is better this way, because you can > easily use it to pass result from a search script: > ./ffmpeg -i $(./findmypaint.sh) ... > Search script is a simple script which finds for you screen, focus window > and grab window of some program. > For example findmypaint.sh here finds this information about MyPaint. The > search script just needs to produce output like this: > :0/0x0580001b/parent2 > It is very flexible this way. I see the point, but it could work with options too. Remember that command substitutions $(...) are re-split (and re-expanded, but there are no special characters here), and thus can contain several words. Therefore, your findmypaint.sh script could output its findings like this: -window 0x0580001b -focus parent2 -i :0 and you would be able to use the same way (almost: remove -i from the command line). > I tought so too, but I tested it with RTMP streaming and players > disconnects without repeat_frame. I am not sure where the problem is. I see. I suppose they consider it as a broken connection after a reasonable timeout. Since you have a working RTMP setup, maybe you could test: feed them from a plain file (using -re), and then suspend the ffmpeg process: do the clients disconnect the same way? If so, I think the solution should not belong in an individual device but rather in the common parts of the code. Still, I suspect the final code for frame duplication would be rather small eventually while changes in the framework are not for today, so it is not a real objection. > Didn't know about existence of boolean there. Should I also take care of > draw_mouse and follow_mouse in a separate patch? You can if you want, and it will be appreciated. But do not feel obligated to do it just because you are changing a nearby and similar part of the code. > > This looks strange. Was this made before my commit to disable > > refcounting of the packets? If so, it will need to be rebased and > > updated. But on the other hand, the logic to keep the current packet > > should be much simpler. > Could you point me out to your commit and the problem? The problem is very simple: I made a non-trivial change to xcbgrab.c recently, and it was applied. If you started working on your patch before that, odds are it will not work now since we probably changed the same lines in different ways. The specific commit is: https://git.ffmpeg.org/gitweb/ffmpeg.git/commit/0bd1be65e88d6a4f367e698d7a2b105424eb1905 Before continuing working on your changes, you need to "rebase" your work tree. There are plenty of tutorial, but the gist of it: git fetch origin (to download the recent changes) git rebase origin/master If it succeeds, good for you. But it will probably point you to conflicts. Use "git diff" to see them, edit the files and fix them. Once it works, use "git add" to prepare the commit, and finish with "git rebase --continue" instead of "git commit". This commit changes how packets are handled in the SHM case. With it, duplicating frames should be trivial: just skip all the image reading capture and skip directly to the part that outputs the packet. The non-SHM case still needs to be handled, though. > Maybe this is the reason for RTMP disconnects. Possibly. I think empty packets may be handled quite inconsistently across the code. > Is there a way to not return > any data? I thought this would work. You cannot not return any data, because when the applications ask for data, they want it. What you can do is return FFERROR_REDO: that will tell the framework to re-call the function immediately, until it actually returns something. It works exactly the same as putting all the read_packet() code in a big loop. From the applications' point of view, it means the function call to get a packet will block until the window is focused again. This is similar to network protocols: if an application is reading and you unplug the cable, it will block until you plug the cable back. Most applications run this part in a separate thread for that reason. (There is also a non-blocking mode, but it is not implemented at all in xcbgrab. And blocking mode is the default, so it needs to work.) > > I do not think you are authorized to change filename like that. > I can duplicate the variable, but didn't notice breaking of anything. I think it would be wiser. But I would still prefer using individual options and avoid the issue entirely and spare the parsing code. Regards,
diff --git a/Changelog b/Changelog index 11a769a..eb83365 100644 --- a/Changelog +++ b/Changelog @@ -3,6 +3,7 @@ releases are sorted from youngest to oldest. version <next>: - CrystalHD decoder moved to new decode API +- XCB-based screen-grabbing of single window version 3.2: - libopenmpt demuxer diff --git a/libavdevice/xcbgrab.c b/libavdevice/xcbgrab.c index 702e66c..a27cdee 100644 --- a/libavdevice/xcbgrab.c +++ b/libavdevice/xcbgrab.c @@ -1,6 +1,8 @@ /* * XCB input grabber * Copyright (C) 2014 Luca Barbato <lu_zero@gentoo.org> + * Copyright (C) 2016 Radomír Polách <rp@t4d.cz> + * Copyright (C) 2016 Kristýna Dudová <kd@t4d.cz> * * This file is part of FFmpeg. * @@ -70,11 +72,21 @@ typedef struct XCBGrabContext { int show_region; int region_border; int centered; + int repeat_frame; const char *video_size; const char *framerate; int has_shm; + + char *focus_name; + char *grab_name; + xcb_window_t focus_window; + xcb_window_t grab_window; + + uint8_t *data; + int size; + int warned; } XCBGrabContext; #define FOLLOW_CENTER -1 @@ -88,6 +100,7 @@ static const AVOption options[] = { { "grab_y", "Initial y coordinate.", OFFSET(y), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, D }, { "video_size", "A string describing frame size, such as 640x480 or hd720.", OFFSET(video_size), AV_OPT_TYPE_STRING, {.str = "vga" }, 0, 0, D }, { "framerate", "", OFFSET(framerate), AV_OPT_TYPE_STRING, {.str = "ntsc" }, 0, 0, D }, + { "repeat_frame", "Repeat last frame, when window is not active or data are not available.", OFFSET(repeat_frame), AV_OPT_TYPE_INT, { .i64 = 1 }, 0, 1, D }, { "draw_mouse", "Draw the mouse pointer.", OFFSET(draw_mouse), AV_OPT_TYPE_INT, { .i64 = 1 }, 0, 1, D }, { "follow_mouse", "Move the grabbing region when the mouse pointer reaches within specified amount of pixels to the edge of region.", OFFSET(follow_mouse), AV_OPT_TYPE_INT, { .i64 = 0 }, FOLLOW_CENTER, INT_MAX, D, "follow_mouse" }, @@ -156,9 +169,13 @@ static int xcbgrab_frame(AVFormatContext *s, AVPacket *pkt) uint8_t *data; int length, ret; - iq = xcb_get_image(c->conn, XCB_IMAGE_FORMAT_Z_PIXMAP, drawable, - c->x, c->y, c->width, c->height, ~0); - + if (c->focus_name) { + iq = xcb_get_image(c->conn, XCB_IMAGE_FORMAT_Z_PIXMAP, c->grab_window, + 0, 0, c->width, c->height, ~0); + } else { + iq = xcb_get_image(c->conn, XCB_IMAGE_FORMAT_Z_PIXMAP, drawable, + c->x, c->y, c->width, c->height, ~0); + } img = xcb_get_image_reply(c->conn, iq, &e); if (e) { @@ -261,9 +278,16 @@ static int xcbgrab_frame_shm(AVFormatContext *s, AVPacket *pkt) if (ret < 0) return ret; - iq = xcb_shm_get_image(c->conn, drawable, - c->x, c->y, c->width, c->height, ~0, - XCB_IMAGE_FORMAT_Z_PIXMAP, c->segment, 0); + if (c->focus_name) { + iq = xcb_shm_get_image(c->conn, c->grab_window, + 0, 0, c->width, c->height, ~0, + XCB_IMAGE_FORMAT_Z_PIXMAP, c->segment, 0); + } else { + iq = xcb_shm_get_image(c->conn, drawable, + c->x, c->y, c->width, c->height, ~0, + XCB_IMAGE_FORMAT_Z_PIXMAP, c->segment, 0); + } + img = xcb_shm_get_image_reply(c->conn, iq, &e); xcb_flush(c->conn); @@ -329,8 +353,13 @@ static void xcbgrab_draw_mouse(AVFormatContext *s, AVPacket *pkt, if (!cursor) return; - cx = ci->x - ci->xhot; - cy = ci->y - ci->yhot; + if (gr->focus_name) { + cx = p->win_x - ci->xhot; + cy = p->win_y - ci->yhot; + } else { + cx = ci->x - ci->xhot; + cy = ci->y - ci->yhot; + } x = FFMAX(cx, gr->x); y = FFMAX(cy, gr->y); @@ -389,6 +418,68 @@ static void xcbgrab_update_region(AVFormatContext *s) args); } +static xcb_window_t get_window_focus(AVFormatContext *s) +{ + XCBGrabContext *ctx = s->priv_data; + xcb_window_t w = 0; + xcb_get_input_focus_cookie_t c; + xcb_get_input_focus_reply_t *r; + + c = xcb_get_input_focus(ctx->conn); + r = xcb_get_input_focus_reply(ctx->conn, c, NULL); + if (!r) + return -1; + + w = r->focus; + free(r); + return w; +} + +static xcb_window_t get_window_parent(AVFormatContext *s, xcb_window_t w) +{ + XCBGrabContext *ctx = s->priv_data; + xcb_query_tree_cookie_t c; + xcb_query_tree_reply_t *r; + xcb_generic_error_t *e; + + c = xcb_query_tree(ctx->conn, w); + r = xcb_query_tree_reply(ctx->conn, c, &e); + if (!r) + return -1; + + w = r->parent; + free(r); + return w; +} + +static void xcbgrab_store_packet(AVFormatContext *s, AVPacket *pkt) { + XCBGrabContext *c = s->priv_data; + + c->size = pkt->size; + + if (c->size) { + if (!c->data) { + c->data = av_malloc(c->size); + } + memcpy(c->data, pkt->data, c->size); + } +} + +static int xcbgrab_load_packet(AVFormatContext *s, AVPacket *pkt) { + XCBGrabContext *c = s->priv_data; + int ret; + + if (!c->size) + return 0; + + ret = av_new_packet(pkt, c->size); + + if (!ret) + memcpy(pkt->data, c->data, c->size); + + return ret; +} + static int xcbgrab_read_packet(AVFormatContext *s, AVPacket *pkt) { XCBGrabContext *c = s->priv_data; @@ -400,11 +491,56 @@ static int xcbgrab_read_packet(AVFormatContext *s, AVPacket *pkt) wait_frame(s, pkt); + if (c->focus_name) { + xcb_window_t w = get_window_focus(s); + if (w != c->focus_window) { + if (!c->warned) { + c->warned = 1; + av_log(s, AV_LOG_WARNING, + "Not grabbing, focus window not focused, focused window is 0x%08x.\n", w); + } + if (c->repeat_frame) { + return xcbgrab_load_packet(s, pkt); + } else { + return 0; + } + } else { + if (c->warned == 1) { + c->warned = 0; + av_log(s, AV_LOG_WARNING, + "Grabbing resumed.\n"); + } + } + } + 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->focus_name) { + pc = xcb_query_pointer(c->conn, c->grab_window); + gc = xcb_get_geometry(c->conn, c->grab_window); + } else { + pc = xcb_query_pointer(c->conn, c->screen->root); + gc = xcb_get_geometry(c->conn, c->screen->root); + } p = xcb_query_pointer_reply(c->conn, pc, NULL); geo = xcb_get_geometry_reply(c->conn, gc, NULL); + if (geo->width < c->width || geo->height < c->height) { + if (!c->warned) { + c->warned = 2; + av_log(s, AV_LOG_WARNING, + "Not grabbing, grab window width or height lower than initial.\n"); + } + if (c->repeat_frame) { + return xcbgrab_load_packet(s, pkt); + } else { + return 0; + } + } else { + if (c->warned == 2) { + c->warned = 0; + av_log(s, AV_LOG_WARNING, + "Grabbing resumed.\n"); + } + } } if (c->follow_mouse && p->same_screen) @@ -425,6 +561,10 @@ static int xcbgrab_read_packet(AVFormatContext *s, AVPacket *pkt) xcbgrab_draw_mouse(s, pkt, p, geo); #endif + if (c->repeat_frame) { + xcbgrab_store_packet(s, pkt); + } + free(p); free(geo); @@ -443,6 +583,10 @@ static av_cold int xcbgrab_read_close(AVFormatContext *s) xcb_disconnect(ctx->conn); + if (ctx->data) { + free(ctx->data); + } + return 0; } @@ -535,6 +679,19 @@ static int create_stream(AVFormatContext *s) avpriv_set_pts_info(st, 64, 1, 1000000); + if (c->focus_name) { + gc = xcb_get_geometry(c->conn, c->grab_window); + geo = xcb_get_geometry_reply(c->conn, gc, NULL); + if (!geo) { + av_log(s, AV_LOG_ERROR, + "Grab window 0x%08x does not exist.\n", + c->grab_window); + return AVERROR(EINVAL); + } + c->width = geo->width & ~1; + c->height = geo->height & ~1; + } + gc = xcb_get_geometry(c->conn, c->screen->root); geo = xcb_get_geometry_reply(c->conn, gc, NULL); @@ -630,6 +787,17 @@ static av_cold int xcbgrab_read_header(AVFormatContext *s) int screen_num, ret; const xcb_setup_t *setup; char *display_name = av_strdup(s->filename); + if (c->focus_name = strchr(s->filename, '/')) { + c->focus_name[0] = '\0'; + ++c->focus_name; + } + if (c->grab_name = strchr(c->focus_name, '/')) { + c->grab_name[0] = '\0'; + ++c->grab_name; + } + c->data = NULL; + c->size = 0; + c->warned = 0; if (!display_name) return AVERROR(ENOMEM); @@ -658,6 +826,28 @@ static av_cold int xcbgrab_read_header(AVFormatContext *s) return AVERROR(EIO); } + if (c->focus_name) { + c->focus_window = strtoul(c->focus_name, NULL, 16); + if (c->grab_name) { + if (!strcmp(c->grab_name,"this")) { + c->grab_window = c->focus_window; + } else if (!strcmp(c->grab_name,"parent")) { + c->grab_window = get_window_parent(s, c->focus_window); + } else if (!strcmp(c->grab_name,"parent2")) { + c->grab_window = get_window_parent(s, c->focus_window); + c->grab_window = get_window_parent(s, c->grab_window); + } else if (!strcmp(c->grab_name,"parent3")) { + c->grab_window = get_window_parent(s, c->focus_window); + c->grab_window = get_window_parent(s, c->grab_window); + c->grab_window = get_window_parent(s, c->grab_window); + } else { + c->grab_window = strtoul(c->grab_name, NULL, 16); + } + } else { + c->grab_window = get_window_parent(s, c->focus_window); + } + } + ret = create_stream(s); if (ret < 0) {