XCB: rework picture allocation, fix a memory leak in case of error

modules/video_output/xcb/pictures.c

@@ -70,13 +70,13 @@ bool XCB_shm_Check (vlc_object_t *obj, xcb_connection_t *conn)
  * format. If a attach is true, the segment is attached to
  * the X server (MIT-SHM extension).
  */
-int XCB_pictures_Alloc (vout_display_t *vd, picture_resource_t *res,
-                        size_t size, xcb_connection_t *conn,
-                        xcb_shm_seg_t segment)
+void *XCB_pictures_Alloc (vout_display_t *vd, picture_sys_t **sysp,
+                          size_t size, xcb_connection_t *conn,
+                          xcb_shm_seg_t segment)
 {
-    res->p_sys = malloc (sizeof(*res->p_sys));
-    if (!res->p_sys)
-        return VLC_EGENERIC;
+    picture_sys_t *picsys = malloc (sizeof (*picsys));
+    if (unlikely(picsys == NULL))
+        return NULL;
 
 #ifdef HAVE_SYS_SHM_H
     /* Allocate shared memory segment */
@@ -84,8 +84,8 @@ int XCB_pictures_Alloc (vout_display_t *vd, picture_resource_t *res,
     if (id == -1)
     {
         msg_Err (vd, "shared memory allocation error: %m");
-        free (res->p_sys);
-        return VLC_EGENERIC;
+        free (picsys);
+        return NULL;
     }
 
     /* Attach the segment to VLC */
@@ -94,8 +94,8 @@ int XCB_pictures_Alloc (vout_display_t *vd, picture_resource_t *res,
     {
         msg_Err (vd, "shared memory attachment error: %m");
         shmctl (id, IPC_RMID, 0);
-        free (res->p_sys);
-        return VLC_EGENERIC;
+        free (picsys);
+        return NULL;
     }
 
     if (segment != 0)
@@ -127,21 +127,18 @@ int XCB_pictures_Alloc (vout_display_t *vd, picture_resource_t *res,
     }
 
     shmctl (id, IPC_RMID, NULL);
-    res->p_sys->segment = segment;
-    res->p->p_pixels = shm;
+    picsys->segment = segment;
 #else
     assert (!attach);
-    res->p_sys->segment = 0;
+    picsys->segment = 0;
 
     /* XXX: align on 32 bytes for VLC chroma filters */
-    res->p->p_pixels = malloc (size);
-    if (unlikely(res->p->p_pixels == NULL))
-    {
-        free (res->p_sys);
-        return VLC_EGENERIC;
-    }
+    void *shm = malloc (size);
+    if (unlikely(shm == NULL))
+        free (picsys);
 #endif
-    return VLC_SUCCESS;
+    *sysp = picsys;
+    return shm;
 }
 
 /**

modules/video_output/xcb/pictures.h

@@ -36,6 +36,6 @@ struct picture_sys_t
 {
     xcb_shm_seg_t segment;
 };
-int XCB_pictures_Alloc (vout_display_t *, picture_resource_t *, size_t size,
-                        xcb_connection_t *, xcb_shm_seg_t);
+void *XCB_pictures_Alloc (vout_display_t *, picture_sys_t **, size_t size,
+                          xcb_connection_t *, xcb_shm_seg_t);
 void XCB_pictures_Free (void *);

modules/video_output/xcb/x11.c

@@ -380,29 +380,33 @@ static picture_pool_t *Pool (vout_display_t *vd, unsigned requested_count)
 
     assert (pic->i_planes == 1);
 
-    unsigned count;
-    picture_t *pic_array[MAX_PICTURES];
+    picture_resource_t res = {
+       .p = {
+           [0] = {
+               .i_lines = pic->p->i_lines,
+               .i_pitch = pic->p->i_pitch,
+           },
+       },
+    };
+    picture_Release (pic);
 
-    for (count = 0; count < MAX_PICTURES; count++)
+    for (unsigned count = 0; count < MAX_PICTURES; count++)
         sys->segments[count] = NULL;
+
+    unsigned count;
+    picture_t *pic_array[MAX_PICTURES];
     for (count = 0; count < MAX_PICTURES; count++)
     {
-        picture_resource_t res = {
-           .p = {
-               [0] = {
-                   .i_lines = pic->p->i_lines,
-                   .i_pitch = pic->p->i_pitch,
-               },
-           },
-        };
         xcb_shm_seg_t seg = (sys->seg_base != 0) ? (sys->seg_base + count) : 0;
 
-        if (XCB_pictures_Alloc (vd, &res, res.p->i_pitch * res.p->i_lines,
-                                sys->conn, seg))
+        res.p[0].p_pixels = XCB_pictures_Alloc (vd, &res.p_sys,
+                              res.p->i_pitch * res.p->i_lines, sys->conn, seg);
+        if (res.p[0].p_pixels == NULL)
             break;
         pic_array[count] = picture_NewFromResource (&vd->fmt, &res);
         if (!pic_array[count])
         {
+            free (res.p_sys);
             XCB_pictures_Free (res.p[0].p_pixels);
             if (seg != 0)
                 xcb_shm_detach (sys->conn, seg);
@@ -410,7 +414,6 @@ static picture_pool_t *Pool (vout_display_t *vd, unsigned requested_count)
         }
         sys->segments[count] = res.p[0].p_pixels;
     }
-    picture_Release (pic);
 
     if (count == 0)
         return NULL;

modules/video_output/xcb/xvideo.c

@@ -620,33 +620,35 @@ static void PoolAlloc (vout_display_t *vd, unsigned requested_count)
 {
     vout_display_sys_t *p_sys = vd->sys;
 
+    for (unsigned count = 0; count < MAX_PICTURES; count++)
+        p_sys->segments[count] = NULL;
+
     const uint32_t *pitches= xcb_xv_query_image_attributes_pitches (p_sys->att);
     const uint32_t *offsets= xcb_xv_query_image_attributes_offsets (p_sys->att);
     const unsigned num_planes= __MIN(p_sys->att->num_planes, PICTURE_PLANE_MAX);
     p_sys->data_size = p_sys->att->data_size;
 
+    picture_resource_t res = { NULL };
+    for (unsigned i = 0; i < num_planes; i++)
+    {
+        uint32_t data_size;
+        data_size = (i < num_planes - 1) ? offsets[i+1] : p_sys->data_size;
+
+        res.p[i].i_lines = (data_size - offsets[i]) / pitches[i];
+        res.p[i].i_pitch = pitches[i];
+    }
+
     picture_t *pic_array[MAX_PICTURES];
     requested_count = __MIN(requested_count, MAX_PICTURES);
 
     unsigned count;
-    for (count = 0; count < MAX_PICTURES; count++)
-        p_sys->segments[count] = NULL;
     for (count = 0; count < requested_count; count++)
     {
         xcb_shm_seg_t seg = p_sys->shm ? xcb_generate_id (p_sys->conn) : 0;
-        picture_resource_t res = { NULL };
-
-        for (unsigned i = 0; i < num_planes; i++)
-        {
-            uint32_t data_size;
-            data_size = (i < num_planes - 1) ? offsets[i+1] : p_sys->data_size;
-
-            res.p[i].i_lines = (data_size - offsets[i]) / pitches[i];
-            res.p[i].i_pitch = pitches[i];
-        }
 
-        if (XCB_pictures_Alloc (vd, &res, p_sys->att->data_size,
-                                p_sys->conn, seg))
+        res.p[0].p_pixels = XCB_pictures_Alloc (vd, &res.p_sys,
+                                           p_sys->data_size, p_sys->conn, seg);
+        if (res.p[0].p_pixels == NULL)
             break;
 
         /* Allocate further planes as specified by XVideo */
@@ -664,6 +666,7 @@ static void PoolAlloc (vout_display_t *vd, unsigned requested_count)
         pic_array[count] = picture_NewFromResource (&vd->fmt, &res);
         if (!pic_array[count])
         {
+            free (res.p_sys);
             XCB_pictures_Free (res.p[0].p_pixels);
             if (seg != 0)
                 xcb_shm_detach (p_sys->conn, seg);