VAAPI-XCB: rework the handling of XCBConnections

Keep track of the xcb connection and properly disconnect on close and errors.

modules/codec/avcodec/vaapi_x11.c

@@ -73,7 +73,8 @@ static const vlc_fourcc_t va_subpicture_chromas[] = {
 
 struct vout_display_sys_t
 {
-    vlc_va_conn_t *conn;
+    vlc_va_conn_t    *vaconn;
+    xcb_connection_t *conn; /**< XCB connection */
 
     vout_window_t *embed;
 
@@ -194,7 +195,7 @@ static int CreateWindow(vout_display_t *vd, xcb_connection_t *conn,
         return VLC_EGENERIC;
 
     msg_Dbg (vd, "using VAAPI X11 window %08"PRIx32, sys->window);
-    xcb_flush(XGetXCBConnection(sys->x11display));
+    xcb_flush(conn);
     return VLC_SUCCESS;
 }
 
@@ -215,6 +216,7 @@ int OpenVaapiX11(vlc_object_t *obj)
     vd->sys = sys;
 
     sys->conn = NULL;
+    sys->vaconn = NULL;
     sys->pool = NULL;
 
     vlc_array_init(&sys->cache);
@@ -227,19 +229,28 @@ int OpenVaapiX11(vlc_object_t *obj)
     if (unlikely(sys->embed == NULL))
         goto error;
 
+    /* Connect to X server */
+    xcb_connection_t *conn = xcb_connect(sys->embed->display.x11, NULL);
+    if (unlikely(xcb_connection_has_error (conn)))
+        goto error;
+
     /* X11 display */
     sys->x11display = XOpenDisplay(sys->embed->display.x11);
     if (!sys->x11display)
+    {
+        xcb_disconnect(conn);
         goto error;
+    }
+    sys->conn = conn;
 
     /* Create a VA display */
-    sys->conn = vlc_va_Initialize(sys->x11display);
-    if (!sys->conn)
+    sys->vaconn = vlc_va_Initialize(sys->x11display);
+    if (!sys->vaconn)
         goto error;
 
     vlc_fourcc_t i_chroma;
     int32_t i_bits_per_pixel;
-    if (FindVAFourCC(sys->conn, &sys->img_fmt, &i_chroma, &i_bits_per_pixel) != VLC_SUCCESS)
+    if (FindVAFourCC(sys->vaconn, &sys->img_fmt, &i_chroma, &i_bits_per_pixel) != VLC_SUCCESS)
         goto error;
 
     vd->fmt.i_chroma = i_chroma;
@@ -247,9 +258,6 @@ int OpenVaapiX11(vlc_object_t *obj)
 
     XSetEventQueueOwner(sys->x11display, XCBOwnsEventQueue);
 
-    xcb_connection_t *conn = XGetXCBConnection(sys->x11display);
-    assert(conn);
-
     RegisterMouseEvents(obj, conn, sys->embed->handle.xid);
 
     /* Find window parameters */
@@ -273,7 +281,7 @@ int OpenVaapiX11(vlc_object_t *obj)
     sys->cursor = CreateBlankCursor(conn, scr);
 
     msg_Info(vd, "using VAAPI XCB video output (libva version %d.%d)",
-            sys->conn->i_version_major, sys->conn->i_version_minor);
+            sys->vaconn->i_version_major, sys->vaconn->i_version_minor);
 
     /* */
     vout_display_info_t info = vd->info;
@@ -283,7 +291,7 @@ int OpenVaapiX11(vlc_object_t *obj)
     info.has_hide_mouse = false;
 
     /* Setup vout_display_t once everything is fine */
-    if (VASubtitleFourCC(sys->conn, VA_FOURCC_RGBA, &sys->sub_fmt, &sys->sflags) == VLC_SUCCESS)
+    if (VASubtitleFourCC(sys->vaconn, VA_FOURCC_RGBA, &sys->sub_fmt, &sys->sflags) == VLC_SUCCESS)
     {
         info.subpicture_chromas = va_subpicture_chromas;
         vd->prepare = Render;
@@ -339,23 +347,11 @@ void CloseVaapiX11(vlc_object_t *obj)
 
         vlc_array_remove(&sys->cache, i);
 
-        cache_SubpictureRelease(obj, sys->conn, &sys->cache, cache_GetCacheID(cache));
-        cache_SubpictureDestroy(obj, sys->conn, cache, true);
+        cache_SubpictureRelease(obj, sys->vaconn, &sys->cache, cache_GetCacheID(cache));
+        cache_SubpictureDestroy(obj, sys->vaconn, cache, true);
     }
     vlc_array_clear(&sys->cache);
 
-    if (sys->x11display)
-    {
-        /* show the default cursor */
-        xcb_change_window_attributes(XGetXCBConnection(sys->x11display),
-                                     sys->embed->handle.xid, XCB_CW_CURSOR,
-                                     &(uint32_t) { XCB_CURSOR_NONE });
-        xcb_flush(XGetXCBConnection(sys->x11display));
-    }
-
-    if (sys->embed)
-        vout_display_DeleteWindow (vd, sys->embed);
-
     /* */
     if (sys->pool)
     {
@@ -363,15 +359,27 @@ void CloseVaapiX11(vlc_object_t *obj)
         sys->pool = NULL;
     }
 
-    if (sys->conn)
+    if (sys->vaconn)
     {
-        sys->conn->destroy_surfaces(sys->conn);
-        vlc_va_Terminate(sys->conn);
+        sys->vaconn->destroy_surfaces(sys->vaconn);
+        vlc_va_Terminate(sys->vaconn);
     }
 
     if (sys->x11display)
         XCloseDisplay(sys->x11display);
 
+    if (sys->conn)
+    {
+        /* show the default cursor */
+        xcb_change_window_attributes(sys->conn, sys->embed->handle.xid,
+                                    XCB_CW_CURSOR, &(uint32_t) { XCB_CURSOR_NONE });
+        xcb_flush(sys->conn);
+        xcb_disconnect(sys->conn);
+    }
+
+    if (sys->embed)
+        vout_display_DeleteWindow (vd, sys->embed);
+
     free(vd->sys);
 }
 
@@ -379,8 +387,7 @@ static void Manage(vout_display_t *vd)
 {
     int canc = vlc_savecancel();
     vout_display_sys_t *sys = vd->sys;
-    xcb_connection_t *conn = XGetXCBConnection(sys->x11display);
-    ManageEvent(vd, conn, &sys->visible);
+    ManageEvent(vd, sys->conn, &sys->visible);
     vlc_restorecancel(canc);
 }
 
@@ -408,8 +415,6 @@ static int Control (vout_display_t *vd, int query, va_list ap)
     case VOUT_DISPLAY_CHANGE_SOURCE_ASPECT:
     case VOUT_DISPLAY_CHANGE_SOURCE_CROP:
     {
-        xcb_connection_t *conn = XGetXCBConnection(sys->x11display);
-
         const vout_display_cfg_t *cfg;
         const video_format_t *source;
         bool is_forced = false;
@@ -444,11 +449,11 @@ static int Control (vout_display_t *vd, int query, va_list ap)
         const uint32_t values[] = { place.x, place.y,
                                     place.width, place.height, };
         xcb_void_cookie_t ck =
-            xcb_configure_window_checked(conn, sys->window,
+            xcb_configure_window_checked(sys->conn, sys->window,
                             XCB_CONFIG_WINDOW_X | XCB_CONFIG_WINDOW_Y
                           | XCB_CONFIG_WINDOW_WIDTH | XCB_CONFIG_WINDOW_HEIGHT,
                               values);
-        if (CheckError(vd, conn, "cannot resize X11 window", ck))
+        if (CheckError(vd, sys->conn, "cannot resize X11 window", ck))
             return VLC_EGENERIC;
 
         return VLC_SUCCESS;
@@ -463,10 +468,9 @@ static int Control (vout_display_t *vd, int query, va_list ap)
      * vout_display_t::info.b_hide_mouse is false */
     case VOUT_DISPLAY_HIDE_MOUSE:
     {
-        xcb_connection_t *conn = XGetXCBConnection(sys->x11display);
-        xcb_change_window_attributes(conn, sys->embed->handle.xid,
+        xcb_change_window_attributes(sys->conn, sys->embed->handle.xid,
                                      XCB_CW_CURSOR, &(uint32_t){ sys->cursor });
-        xcb_flush(conn);
+        xcb_flush(sys->conn);
         return VLC_SUCCESS;
     }
 
@@ -516,7 +520,7 @@ static void Render(vout_display_t *vd, picture_t *picture, subpicture_t *subpict
                 sys->render.i_cache, picture->p_sys->i_id);
 #endif
 
-        int ret = RenderCachedSubpictures(VLC_OBJECT(vd), sys->conn, &sys->cache,
+        int ret = RenderCachedSubpictures(VLC_OBJECT(vd), sys->vaconn, &sys->cache,
                                           sys->render.i_cache, picture, subpicture);
         if (ret == VLC_SUCCESS)
             goto out;
@@ -524,10 +528,10 @@ static void Render(vout_display_t *vd, picture_t *picture, subpicture_t *subpict
 
     /* Release exra reference */
     if (sys->render.i_cache > 0)
-        cache_SubpictureRelease(VLC_OBJECT(vd), sys->conn, &sys->cache, sys->render.i_cache);
+        cache_SubpictureRelease(VLC_OBJECT(vd), sys->vaconn, &sys->cache, sys->render.i_cache);
 
     /* render new subpicture */
-    subpicture_cache_t *cache = RenderDirectSubpicture(VLC_OBJECT(vd), sys->conn, &sys->sub_fmt,
+    subpicture_cache_t *cache = RenderDirectSubpicture(VLC_OBJECT(vd), sys->vaconn, &sys->sub_fmt,
                                                        picture, subpicture, sys->sflags);
     if (cache == NULL)
         goto out;
@@ -557,7 +561,7 @@ static void DisplayVASurface(vout_display_t *vd, picture_t *picture)
     picture_sys_t *surface = picture->p_sys;
     assert(surface);
 
-    sys->conn->lock();
+    sys->vaconn->lock();
 
     int canc = vlc_savecancel();
 
@@ -565,7 +569,7 @@ static void DisplayVASurface(vout_display_t *vd, picture_t *picture)
     vout_display_PlacePicture(&place, &vd->source, vd->cfg, false);
 
     VAStatus status;
-    status = vaPutSurface(sys->conn->p_display, surface->i_id, sys->window,
+    status = vaPutSurface(sys->vaconn->p_display, surface->i_id, sys->window,
                           vd->source.i_x_offset, vd->source.i_y_offset,
                           vd->source.i_visible_width, vd->source.i_visible_height,
                           place.x, place.y, place.width, place.height,
@@ -579,7 +583,7 @@ static void DisplayVASurface(vout_display_t *vd, picture_t *picture)
 
     vlc_restorecancel(canc);
 
-    sys->conn->unlock();
+    sys->vaconn->unlock();
 }
 
 static void DisplayPicture(vout_display_t *vd, picture_t *picture, subpicture_t *subpicture)
@@ -603,7 +607,7 @@ static void DisplayPicture(vout_display_t *vd, picture_t *picture, subpicture_t
     if (subpicture)
     {
         vlc_mutex_lock(&sys->cache_lock);
-        ret = SubpictureRegionsLink(VLC_OBJECT(vd), sys->conn, &sys->cache, picture, sys->sflags);
+        ret = SubpictureRegionsLink(VLC_OBJECT(vd), sys->vaconn, &sys->cache, picture, sys->sflags);
         if (ret == VLC_EGENERIC)
             msg_Err(vd, "Failed linking subpicture to surface");
         vlc_mutex_unlock(&sys->cache_lock);
@@ -616,11 +620,11 @@ out:
         vlc_mutex_lock(&sys->cache_lock);
         if (ret == VLC_SUCCESS)
         {
-            if (SubpictureRegionsUnlink(VLC_OBJECT(vd), sys->conn, &sys->cache, picture) == VLC_EGENERIC)
+            if (SubpictureRegionsUnlink(VLC_OBJECT(vd), sys->vaconn, &sys->cache, picture) == VLC_EGENERIC)
                 msg_Err(vd, "Could not find subpicture in cache");
         }
         if (picture->p_sys->i_cache > 0)
-            cache_SubpictureRelease(VLC_OBJECT(vd), sys->conn, &sys->cache, picture->p_sys->i_cache);
+            cache_SubpictureRelease(VLC_OBJECT(vd), sys->vaconn, &sys->cache, picture->p_sys->i_cache);
         picture->p_sys->i_cache = 0;
         vlc_mutex_unlock(&sys->cache_lock);
 
@@ -712,20 +716,20 @@ static picture_pool_t *Pool (vout_display_t *vd, unsigned requested_count)
         goto error;
 
     /* Create surfaces */
-    sys->conn->lock();
+    sys->vaconn->lock();
 
     VASurfaceID *pi_surface_id;
-    if (sys->conn->i_surface_count < i_surface_count)
+    if (sys->vaconn->i_surface_count < i_surface_count)
     {
-        pi_surface_id = sys->conn->create_surfaces(sys->conn,
+        pi_surface_id = sys->vaconn->create_surfaces(sys->vaconn,
                                 vd->fmt.i_visible_width, vd->fmt.i_visible_height,
                                 VA_RT_FORMAT_YUV420, i_surface_count);
     }
-    else pi_surface_id = sys->conn->p_surface_ids;
+    else pi_surface_id = sys->vaconn->p_surface_ids;
 
     if (pi_surface_id == NULL)
     {
-        sys->conn->unlock();
+        sys->vaconn->unlock();
         goto error;
     }
 
@@ -736,7 +740,7 @@ static picture_pool_t *Pool (vout_display_t *vd, unsigned requested_count)
 
         pic_sys->i_id = pi_surface_id[i];
     }
-    sys->conn->unlock();
+    sys->vaconn->unlock();
 
     /* Register pool with video output core */
     picture_pool_configuration_t cfg;
@@ -751,6 +755,7 @@ static picture_pool_t *Pool (vout_display_t *vd, unsigned requested_count)
     if (!sys->pool)
         goto error;
 
+    xcb_flush(sys->conn);
     free(pic_array);
     return sys->pool;