codec/avcodec/vaapi: Release subpicture associations (corner cases).

When surfaces are reclaimed or released before they are being displayed,
then the subpicture association still exists. This code tries to catch
such pictures and will release the subpictures association.

modules/codec/avcodec/vaapi.c

@@ -213,8 +213,6 @@ static int CreateSurfaces( vlc_va_vaapi_t *p_va, void **pp_hw_ctx, vlc_fourcc_t
         p_surface->i_id = pi_surface_id[i];
         p_surface->i_refcount = 0;
         p_surface->i_order = 0;
-        p_surface->i_sub = 0;
-        p_surface->subpicture = NULL;
         vlc_mutex_init(&p_surface->lock);
     }
 
@@ -403,10 +401,6 @@ static vlc_va_surface_t *FindSurface( vlc_va_t *p_external, const VASurfaceID i_
              */
             p_tmp->i_refcount++;
             p_surface = p_tmp;
-            assert(p_surface->subpicture == NULL);
-            free(p_surface->subpicture);
-            p_surface->subpicture = NULL;
-            p_surface->i_sub = 0;
             vlc_mutex_unlock(&p_tmp->lock);
             break;
         }

modules/codec/avcodec/vaapi.h

@@ -34,6 +34,7 @@ void vlc_va_Terminate(vlc_va_conn_t *conn);
 typedef struct
 {
     VASubpictureID  i_id;  /* VASubpictureIDs */
+    VASurfaceID     i_surface;
     VAImage         image;
 } vlc_va_subpicture_t;
 
@@ -43,16 +44,18 @@ typedef struct
     int          i_refcount;
     unsigned int i_order;
 
-    /* subpicture */
-    vlc_va_subpicture_t *subpicture;
-    unsigned int         i_sub; /* number of vlc_va_subpicures_t */
-
     vlc_mutex_t  lock;
 } vlc_va_surface_t;
 
 struct picture_sys_t
 {
+    /* surface */
     vlc_va_surface_t *surface;
+
+    /* subpicture */
+    vlc_va_subpicture_t *subpicture;
+    unsigned int         i_sub; /* number of vlc_va_subpicures_t */
+
     bool b_own_vout; /* surface owned by vout? */
 };
 #endif

modules/codec/avcodec/vaapi_x11.c

@@ -361,9 +361,8 @@ static int Control (vout_display_t *vd, int query, va_list ap)
     }
 }
 
-static VASubpictureID SubpictureCreate(vout_display_t *vd, vlc_va_surface_t *surface,
-                const subpicture_t *subpicture, const subpicture_region_t *region,
-                vlc_va_subpicture_t *vasubpicture)
+static VASubpictureID SubpictureCreate(vout_display_t *vd, const subpicture_t *subpicture,
+                const subpicture_region_t *region, vlc_va_subpicture_t *vasubpicture)
 {
     vout_display_sys_t *sys = vd->sys;
 
@@ -423,11 +422,12 @@ static VASubpictureID SubpictureCreate(vout_display_t *vd, vlc_va_surface_t *sur
 
     /* Associate subpicture with surface */
     status = vaAssociateSubpicture(sys->conn->p_display, subpic,
-                                   &surface->i_id, 1, /* array of surfaces */
+                                   &vasubpicture->i_surface, 1, /* array of surfaces */
                                    /* src rectangle */
                                    src_left, src_top, src_right, src_bottom,
                                    /* dest rectangle */
-                                   ((video.width - dst_right) / 2), dst_top, dst_right - dst_left, dst_bottom - dst_top,
+                                   ((video.width - dst_right) / 2), dst_top,
+                                   dst_right - dst_left, dst_bottom - dst_top,
                                    flags);
     if (status != VA_STATUS_SUCCESS)
     {
@@ -437,25 +437,19 @@ static VASubpictureID SubpictureCreate(vout_display_t *vd, vlc_va_surface_t *sur
     return subpic;
 }
 
-static void SubpictureDestroy(vout_display_t *vd, vlc_va_surface_t *surface)
+static void SubpictureDestroy(vout_display_t *vd, vlc_va_subpicture_t *subpicture, const int i_sub)
 {
     vout_display_sys_t *sys = vd->sys;
-    vlc_va_subpicture_t *subpicture = surface->subpicture;
-
-    int i_sub = surface->i_sub;
-
-    surface->i_sub = 0;
-    surface->subpicture = NULL;
 
     VAStatus status;
     for (int i = 0; i < i_sub; i++)
     {
-        if ((surface->i_id != VA_INVALID_ID) &&
+        if ((subpicture[i].i_surface != VA_INVALID_SURFACE) &&
             (subpicture[i].i_id != VA_INVALID_ID))
         {
             status = vaDeassociateSubpicture(sys->conn->p_display,
                                              subpicture[i].i_id,
-                                             &surface->i_id, 1);
+                                             &subpicture[i].i_surface, 1);
             if (status != VA_STATUS_SUCCESS)
                 msg_Err(vd, "Failed unlinking subpicture from surface (%d)", status);
         }
@@ -475,6 +469,11 @@ static void SubpictureDestroy(vout_display_t *vd, vlc_va_surface_t *surface)
             if (status != VA_STATUS_SUCCESS)
                 msg_Err(vd, "Failed destroying subpicture image");
         }
+
+        subpicture[i].i_surface = VA_INVALID_SURFACE;
+        subpicture[i].image.image_id = VA_INVALID_ID;
+        subpicture[i].image.buf = VA_INVALID_ID;
+        subpicture[i].i_id = VA_INVALID_ID;
     }
 
     /* */
@@ -637,7 +636,7 @@ static int RenderDirectSubpicture(vout_display_t *vd, picture_t *picture, subpic
 {
     vout_display_sys_t *sys = vd->sys;
 
-    assert(picture->p_sys->surface->subpicture == NULL);
+    assert(picture->p_sys->subpicture == NULL);
 
     vlc_va_subpicture_t *vasubpic = (vlc_va_subpicture_t*) calloc(i_region, sizeof(vlc_va_subpicture_t));
     if (!vasubpic)
@@ -668,9 +667,10 @@ static int RenderDirectSubpicture(vout_display_t *vd, picture_t *picture, subpic
         vasubpic[i_sub].image.format.blue_mask = region->p_picture->format.i_bmask;
         vasubpic[i_sub].image.format.green_mask = region->p_picture->format.i_gmask;
 
+        vasubpic[i_sub].i_surface = picture->p_sys->surface->i_id;
+
         /* Create Subpicture */
-        vasubpic[i_sub].i_id = SubpictureCreate(vd, picture->p_sys->surface,
-                                                    subpicture, region,
+        vasubpic[i_sub].i_id = SubpictureCreate(vd, subpicture, region,
                                                     &vasubpic[i_sub]);
         if (vasubpic[i_sub].i_id == VA_INVALID_ID)
         {
@@ -683,8 +683,8 @@ static int RenderDirectSubpicture(vout_display_t *vd, picture_t *picture, subpic
     }
 
     /* remember what was created, so it can be released in DisplayPicture() */
-    picture->p_sys->surface->subpicture = vasubpic;
-    picture->p_sys->surface->i_sub = i_sub;
+    picture->p_sys->subpicture = vasubpic;
+    picture->p_sys->i_sub = i_sub;
 
     return VLC_SUCCESS;
 }
@@ -709,7 +709,7 @@ static void PictureReleaseVoutOwned(picture_t *picture)
 #if 0
     /* FIXME: pictures discarded by core will leak VASurfaces */
     if (surface->i_id != VA_INVALID_SURFACE)
-        vaDestroySurfaces(sys->conn->p_display, &psurface->i_id, 1);
+        vaDestroySurfaces(sys->conn->p_display, &surface->i_id, 1);
 #endif
     vlc_mutex_destroy(&surface->lock);
     free(surface);
@@ -725,6 +725,16 @@ static void Render(vout_display_t *vd, picture_t *picture, subpicture_t *subpict
     if (subpicture->b_ephemer)
         return;
 
+    /* Cleanup for reclaimed surface */
+    if (picture->p_sys &&
+        picture->p_sys->subpicture &&
+        (picture->format.i_chroma == VLC_CODEC_VAAPI_SURFACE))
+    {
+        SubpictureDestroy(vd, picture->p_sys->subpicture, picture->p_sys->i_sub);
+        picture->p_sys->subpicture = NULL;
+        picture->p_sys->i_sub = 0;
+    }
+
     if (picture->format.i_chroma != VLC_CODEC_VAAPI_SURFACE)
     {
         assert(picture->p_sys);
@@ -736,7 +746,7 @@ static void Render(vout_display_t *vd, picture_t *picture, subpicture_t *subpict
         picture->pf_release = PictureReleaseVoutOwned;
     }
 
-    if (picture->p_sys->surface == NULL)
+    if (!picture->p_sys->surface)
         return;
 
     vlc_mutex_lock(&picture->p_sys->surface->lock);
@@ -806,8 +816,12 @@ static void DisplayPicture(vout_display_t *vd, picture_t *pic, subpicture_t *sub
     {
         assert(!pic->p_sys->b_own_vout);
         msg_Err(vd, "Discarding picture without surface information");
+        if (pic->p_sys->subpicture)
+            SubpictureDestroy(vd, pic->p_sys->subpicture, pic->p_sys->i_sub);
         if (subpicture)
             subpicture_Delete(subpicture);
+        pic->p_sys->subpicture = NULL;
+        pic->p_sys->i_sub = 0;
         picture_Release(pic);
         return;
     }
@@ -832,22 +846,26 @@ static void DisplayPicture(vout_display_t *vd, picture_t *pic, subpicture_t *sub
     DisplayVASurface(vd, pic->p_sys->surface->i_id, pic);
 
 cleanup:
+    if (pic->p_sys->subpicture)
+        SubpictureDestroy(vd, pic->p_sys->subpicture, pic->p_sys->i_sub);
+
     if (subpicture)
-    {
-        SubpictureDestroy(vd, pic->p_sys->surface);
         subpicture_Delete(subpicture);
-    }
 
     if (pic->p_sys->b_own_vout)
     {
         if (pic->p_sys->surface->i_id != VA_INVALID_SURFACE)
             vaDestroySurfaces(sys->conn->p_display, &pic->p_sys->surface->i_id, 1);
     }
+
     ReleaseSurfaceLocked(pic->p_sys->surface);
     vlc_mutex_unlock(&pic->p_sys->surface->lock);
 
     if (!pic->p_sys->b_own_vout)
         pic->p_sys->surface = NULL;
+    pic->p_sys->subpicture = NULL;
+    pic->p_sys->i_sub = 0;
+
     picture_Release(pic);
 }
 
@@ -873,14 +891,18 @@ static picture_pool_t *Pool (vout_display_t *vd, unsigned requested_count)
         if (!pic)
             break;
         assert(pic->p_sys == NULL);
-        picture_sys_t *sys = (picture_sys_t *) calloc(1, sizeof(picture_sys_t));
+        picture_sys_t *sys = (picture_sys_t *) malloc(sizeof(picture_sys_t));
         if (!sys)
         {
             picture_Release(pic);
             break;
         }
         pic->format.i_chroma = VLC_CODEC_VAAPI_SURFACE;
+
         sys->b_own_vout = false;
+        sys->subpicture = NULL;
+        sys->surface = NULL;
+        sys->i_sub = 0;
 
         pic_array[count] = pic;
         pic_array[count]->b_progressive = true;