On 12/7/2019 6:57 PM, Marton Balint wrote:
> Also by wrapping the SHM buffer in an AVBufferRef we eliminate yet another
> possible memcpy improving performance.
>
> Signed-off-by: Marton Balint <c...@passwd.hu>
> ---
> libavdevice/xcbgrab.c | 79
> ++++++++++++++++++++++++++++++---------------------
> 1 file changed, 47 insertions(+), 32 deletions(-)
>
> diff --git a/libavdevice/xcbgrab.c b/libavdevice/xcbgrab.c
> index 158f381c04..82ea09ca96 100644
> --- a/libavdevice/xcbgrab.c
> +++ b/libavdevice/xcbgrab.c
> @@ -49,13 +49,11 @@
> typedef struct XCBGrabContext {
> const AVClass *class;
>
> - uint8_t *buffer;
> -
> xcb_connection_t *conn;
> xcb_screen_t *screen;
> xcb_window_t window;
> #if CONFIG_LIBXCB_SHM
> - xcb_shm_seg_t segment;
> + AVBufferPool *shm_pool;
> #endif
> int64_t time_frame;
> AVRational time_base;
> @@ -231,31 +229,35 @@ static int check_shm(xcb_connection_t *conn)
> return 0;
> }
>
> -static int allocate_shm(AVFormatContext *s)
> +static void free_shm_buffer(void *opaque, uint8_t *data)
> {
> - XCBGrabContext *c = s->priv_data;
> - int size = c->frame_size + AV_INPUT_BUFFER_PADDING_SIZE;
> + shmdt(data);
> +}
> +
> +static AVBufferRef *allocate_shm_buffer(void *opaque, int size)
> +{
> + xcb_connection_t *conn = opaque;
> + xcb_shm_seg_t segment;
> + AVBufferRef *ref;
> uint8_t *data;
> int id;
>
> - if (c->buffer)
> - return 0;
> id = shmget(IPC_PRIVATE, size, IPC_CREAT | 0777);
> - if (id == -1) {
> - char errbuf[1024];
> - int err = AVERROR(errno);
> - av_strerror(err, errbuf, sizeof(errbuf));
> - av_log(s, AV_LOG_ERROR, "Cannot get %d bytes of shared memory:
> %s.\n",
> - size, errbuf);
> - return err;
> - }
> - xcb_shm_attach(c->conn, c->segment, id, 0);
> + if (id == -1)
> + return NULL;
> +
> + segment = xcb_generate_id(conn);
> + xcb_shm_attach(conn, segment, id, 0);
> data = shmat(id, NULL, 0);
> shmctl(id, IPC_RMID, 0);
> if ((intptr_t)data == -1 || !data)
> - return AVERROR(errno);
> - c->buffer = data;
> - return 0;
> + return NULL;
> +
> + ref = av_buffer_create(data, size, free_shm_buffer, (void
> *)(ptrdiff_t)segment, 0);
> + if (!ref)
> + shmdt(data);
> +
> + return ref;
> }
>
> static int xcbgrab_frame_shm(AVFormatContext *s, AVPacket *pkt)
> @@ -265,15 +267,19 @@ static int xcbgrab_frame_shm(AVFormatContext *s,
> AVPacket *pkt)
> xcb_shm_get_image_reply_t *img;
> xcb_drawable_t drawable = c->screen->root;
> xcb_generic_error_t *e = NULL;
> - int ret;
> + AVBufferRef *buf;
> + xcb_shm_seg_t segment;
>
> - ret = allocate_shm(s);
> - if (ret < 0)
> - return ret;
> + buf = av_buffer_pool_get(c->shm_pool);
> + if (!buf) {
> + av_log(s, AV_LOG_ERROR, "Could not get shared memory buffer.\n");
> + return AVERROR(ENOMEM);
> + }
> + segment = (xcb_shm_seg_t)av_buffer_pool_buffer_get_opaque(buf);
>
> 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);
> + XCB_IMAGE_FORMAT_Z_PIXMAP, segment, 0);
> img = xcb_shm_get_image_reply(c->conn, iq, &e);
>
> xcb_flush(c->conn);
> @@ -287,12 +293,16 @@ static int xcbgrab_frame_shm(AVFormatContext *s,
> AVPacket *pkt)
> e->sequence, e->resource_id, e->minor_code, e->major_code);
>
> free(e);
> + av_buffer_unref(&buf);
> return AVERROR(EACCES);
> }
>
> free(img);
>
> - pkt->data = c->buffer;
> + av_init_packet(pkt);
> +
> + pkt->buf = buf;
> + pkt->data = buf->data;
> pkt->size = c->frame_size;
>
> return 0;
> @@ -425,8 +435,10 @@ 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 && xcbgrab_frame_shm(s, pkt) < 0) {
> + av_log(s, AV_LOG_WARNING, "Continuing without shared memory.\n");
> c->has_shm = 0;
> + }
> #endif
> if (!c->has_shm)
> ret = xcbgrab_frame(s, pkt);
> @@ -447,9 +459,7 @@ static av_cold int xcbgrab_read_close(AVFormatContext *s)
> XCBGrabContext *ctx = s->priv_data;
>
> #if CONFIG_LIBXCB_SHM
> - if (ctx->buffer) {
> - shmdt(ctx->buffer);
> - }
> + av_buffer_pool_uninit(&ctx->shm_pool);
> #endif
>
> xcb_disconnect(ctx->conn);
> @@ -515,6 +525,12 @@ static int pixfmt_from_pixmap_format(AVFormatContext *s,
> int depth,
> if (*pix_fmt) {
> c->bpp = fmt->bits_per_pixel;
> c->frame_size = c->width * c->height * fmt->bits_per_pixel / 8;
> +#if CONFIG_LIBXCB_SHM
> + c->shm_pool = av_buffer_pool_init2(c->frame_size +
> AV_INPUT_BUFFER_PADDING_SIZE,
> + c->conn, allocate_shm_buffer,
> NULL);
I didn't look too much into it, but shouldn't you add a free() callback
here and move the xcb_disconnect(conn) call from xcbgrab_read_close() to
it? If the AVBufferRefs created here outlive the demuxer/outdev, then it
may result in issues if the xcb_connection_t is expected to be valid
until the last one of them is freed.
> + if (!c->shm_pool)
> + return AVERROR(ENOMEM);
> +#endif
> return 0;
> }
>
> @@ -680,8 +696,7 @@ static av_cold int xcbgrab_read_header(AVFormatContext *s)
> }
>
> #if CONFIG_LIBXCB_SHM
> - if ((c->has_shm = check_shm(c->conn)))
> - c->segment = xcb_generate_id(c->conn);
> + c->has_shm = check_shm(c->conn);
> #endif
>
> #if CONFIG_LIBXCB_XFIXES
>
_______________________________________________
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".
