[vlc-devel] [PATCH] Cursor integration on X11 video screenshots thanks to xfixes

[Rémi Denis-Courmont]

	Hello,

Le lundi 14 mai 2012 13:23:09 Mehdi Lauters, vous avez écrit :
> refactoring the cursor integration on video screenshot on X servers system
> ---
>  modules/access/screen/xcb.c |  128
> ++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 127
> insertions(+), 1 deletions(-)
> 
> diff --git a/modules/access/screen/xcb.c b/modules/access/screen/xcb.c
> index c31d380..6e96bb6 100644
> --- a/modules/access/screen/xcb.c
> +++ b/modules/access/screen/xcb.c
> @@ -137,6 +137,12 @@ static int Open (vlc_object_t *obj)
>      }
>      p_sys->conn = conn;
> 
> +    // initializing xfixes (to get the cursor image)
> +    xcb_xfixes_query_version_reply(conn,

Doesn't this leak memory?

> +            xcb_xfixes_query_version(conn,XCB_XFIXES_MAJOR_VERSION,
> +                    XCB_XFIXES_MINOR_VERSION),

I could be wrong, but I thought the version numbers where OUTPUT parameters?!

> +            NULL);
> +
>     /* Find configured screen */
>      if (!strcmp (demux->psz_access, "screen"))
>      {
> @@ -295,6 +301,79 @@ static int Control (demux_t *demux, int query, va_list
> args) return VLC_EGENERIC;
>  }
> 
> +int integrateCursor(uint8_t *screenImageData,xcb_get_geometry_reply_t
> *geo, xcb_xfixes_get_cursor_image_reply_t* cursorImgReply)
> +{
> +    uint8_t *pointerImageData;
> +
> +    // indices to browse images
> +    int iScreen;
> +    int iPointer=0;
> +
> +    // we get the cursor image data
> +    pointerImageData =
> xcb_xfixes_get_cursor_image_cursor_image(cursorImgReply);
> +
> +    //indices to browse the pointer picture
> +    int ixCursor=0,
> +        iyCursor=0;
> +
> +    // top left corner of the pointer picture on the screen picture
> +    int xMinScreen=cursorImgReply->x-cursorImgReply->xhot,
> +
> +    // top right corner of the pointer picture on the screen picture
> +        yMinScreen=cursorImgReply->y-cursorImgReply->yhot,
> +
> +    // bottom left corner of the pointer picture on the screen picture
> +       
> xMaxScreen=cursorImgReply->x+(cursorImgReply->width-cursorImgReply->xhot),
> +
> +    // bottom right corner of the pointer picture on the screen picture
> +       
> yMaxScreen=cursorImgReply->y+(cursorImgReply->width-cursorImgReply->yhot);
> +
> +
> +    // be sure that the entire pointer is on the screenshot
> +    if(!(xMinScreen<0 || yMinScreen <0 || xMaxScreen > geo->width
> +            || yMaxScreen > geo->height))
> +    {
> +
> +        // we browse the screen picture
> +        for(int ix=xMinScreen; // to center on the right point
> +                ix<xMaxScreen;
> +                ix++)
> +        {
> +            iyCursor=0;
> +            for(int iy=yMinScreen; // to center on the right point
> +                    iy<yMaxScreen;
> +                    iy++)
> +            {
> +                //get the indice of the (ix,iy) pixel in the screen data
> buffer +                iScreen=iy*geo->width+ix;
> +
> +
> +                //get the relative indice of the cursor
> +                iPointer=iyCursor*cursorImgReply->width+ixCursor;
> +
> +                // get the indice of the first color of the current pixel
> +                iScreen=iScreen*4;
> +                iPointer=iPointer*4;
> +
> +                // /!\ no transparency
> +                if(((uint8_t *)pointerImageData)[iPointer+3]>100)

Is it warranted that the pointer is RGBA?

> +                {
> +                    // copying pixels colors
> +                    screenImageData[iScreen]=pointerImageData[iPointer];
> // r +                   
> screenImageData[iScreen+1]=pointerImageData[iPointer+1];//g+             
>       screenImageData[iScreen+2]=pointerImageData[iPointer+2];//b +       
>         }

I guess this assumes RV32 screen image.

Maybe we don't care about cursor on screens of other depth, but at least 
sanity checking is required.

> +                iyCursor++;
> +            }
> +            ixCursor++;
> +        }
> +    }
> +    else
> +    {
> +        return VLC_EGENERIC;
> +    }
> +    return VLC_SUCCESS;
> +}
> 
>  /**
>   * Processing callback
> @@ -402,14 +481,61 @@ discard:
>      /* Capture screen */
>      xcb_drawable_t drawable =
>          (sys->window != geo->root) ? sys->pixmap : sys->window;
> -    free (geo);
> 
>      xcb_get_image_reply_t *img;
>      img = xcb_get_image_reply (conn,
>          xcb_get_image (conn, XCB_IMAGE_FORMAT_Z_PIXMAP, drawable,
>                         x, y, w, h, ~0), NULL);
>      if (img == NULL)
> +    {
> +        free (geo);
>          return;
> +    }
> +
> +    // we ask for pointer informations ( on the same screen or not )
> +    xcb_query_pointer_cookie_t pointerCookie = xcb_query_pointer(conn,
> drawable);
> +    xcb_query_pointer_reply_t * pointerReply =
> xcb_query_pointer_reply (conn,
> +            pointerCookie,NULL);

This is still not interleaved.

> +
> +
> +    if(pointerReply == NULL)
> +    {
> +        msg_Warn (demux, "unable to locate cursor");
> +    }
> +    else
> +    {
> +        // if the cursor is on the same screen
> +        if(pointerReply->same_screen == 1)
> +        {
> +            // we get more information about the cursor
> +            xcb_xfixes_get_cursor_image_cookie_t cursorCookie =
> xcb_xfixes_get_cursor_image(conn);
> +           
> xcb_xfixes_get_cursor_image_reply_t* cursorImgReply =
> xcb_xfixes_get_cursor_image_reply (conn, cursorCookie, NULL);
> +
> +            if(cursorImgReply != NULL)
> +            {
> +
> +                // images data
> +                uint8_t *screenImageData=NULL;
> +
> +                // we get screen image data
> +                screenImageData = xcb_get_image_data (img);
> +
> +                if(integrateCursor(screenImageData,geo,cursorImgReply) !=
> VLC_SUCCESS)
> +                {
> +                    msg_Warn (demux, "Cursor not on the picture");
> +                }
> +
> +                free(cursorImgReply);
> +            }
> +            else
> +            {
> +                msg_Warn (demux, "unable to find cursor image");
> +            }
> +        }
> +    }
> +
> +    free(pointerReply);
> +    free (geo);
> 
>      uint8_t *data = xcb_get_image_data (img);
>      size_t datalen = xcb_get_image_data_length (img);


-- 
Rémi Denis-Courmont
http://www.remlab.net/
http://fi.linkedin.com/in/remidenis