VAAPI-X11: Rework subpicture cache management.

- rework refcounting
- rename functions to have cache_ prefix
- fix resetting of sys->render.i_cache in failure paths

modules/codec/avcodec/vaapi_x11.c

@@ -126,9 +126,9 @@ typedef struct
     vlc_array_t   subpictures; /* subpicture regions */
 } subpicture_cache_t;
 
-static vasubpicture_cache_t *SubpictureCreate(vout_display_t *vd, const subpicture_t *subpicture,
+static vasubpicture_cache_t *cache_SubpictureCreate(vout_display_t *vd, const subpicture_t *subpicture,
                                               const subpicture_region_t *region, const int flags);
-static void SubpictureDestroy(vout_display_t *vd, vlc_va_conn_t *conn, subpicture_cache_t *cache);
+static void cache_SubpictureDestroy(vout_display_t *vd, vlc_va_conn_t *conn, subpicture_cache_t *cache);
 
 static subpicture_cache_t *cache_new(void)
 {
@@ -520,7 +520,7 @@ void CloseVaapiX11(vlc_object_t *obj)
 
         vlc_array_remove(&sys->cache, i);
 
-        SubpictureDestroy(vd, sys->conn, cache);
+        cache_SubpictureDestroy(vd, sys->conn, cache);
         cache_destroy(cache);
         cache = NULL;
     }
@@ -777,10 +777,10 @@ out_va_free:
 /* Create subpicture cache for this subpicture and
  * allocate needed VA API resources. Call this
  * function when holding cache->lock */
-static vasubpicture_cache_t *SubpictureCreate(vout_display_t *vd, const subpicture_t *subpicture,
+static vasubpicture_cache_t *cache_SubpictureCreate(vout_display_t *vd, const subpicture_t *subpicture,
                                               const subpicture_region_t *region, const int flags)
 {
-    vout_display_sys_t *sys = vd->sys;
+    vout_display_sys_t *sys = (vout_display_sys_t *) vd->sys;
 
     sys->conn->lock();
 
@@ -860,12 +860,12 @@ error:
 /* Destroy subpicture cache for this subpicture and
  * release allocated VA API resources. Call this
  * function when holding cache->lock */
-static void SubpictureDestroy(vout_display_t *vd, vlc_va_conn_t *conn, subpicture_cache_t *cache)
+static void cache_SubpictureDestroy(vout_display_t *vd, vlc_va_conn_t *conn, subpicture_cache_t *cache)
 {
-    vout_display_sys_t *sys = (vout_display_sys_t *)vd->sys;
+    VLC_UNUSED(vd);
 
 #ifdef VAAPI_DEBUG
-    msg_Dbg(vd, "SubpictureDestroy cache %d", cache->i_id);
+    msg_Dbg(vd, "cache_SubpictureDestroy cache %d", cache->i_id);
 #endif
     const int num = vlc_array_count(&cache->subpictures);
     for (int i = num - 1; i >= 0; i--)
@@ -883,7 +883,7 @@ static void SubpictureDestroy(vout_display_t *vd, vlc_va_conn_t *conn, subpictur
         assert(subpic->i_id != VA_INVALID_ID);
         assert(subpic->image.image_id != VA_INVALID_ID);
 
-        sys->conn->lock();
+        conn->lock();
 
         vaDestroySubpicture(conn->p_display, subpic->i_id);
         vaDestroyImage(conn->p_display, subpic->image.image_id);
@@ -892,7 +892,7 @@ static void SubpictureDestroy(vout_display_t *vd, vlc_va_conn_t *conn, subpictur
         subpic->image.buf = VA_INVALID_ID;
         subpic->image.image_id = VA_INVALID_ID;
 
-        sys->conn->unlock();
+        conn->unlock();
 
         subpicture_cache_destroy(subpic);
         subpic = NULL;
@@ -916,10 +916,9 @@ static int SubpictureRegionsLink(vout_display_t *vd, const picture_t *picture)
     assert(cache->i_id > 0);
 
     const int count = vlc_array_count(&cache->subpictures);
-    msg_Info(vd, "Subpicture cache count %d", vlc_array_count(&sys->cache));
 #ifdef VAAPI_DEBUG
-    msg_Dbg(vd, "link surface %d cache %d size %d",
-                surface->i_id, cache->i_id, count);
+    msg_Dbg(vd, "  link surface %d cache %d refcount %d",
+                surface->i_id, cache->i_id, cache->i_refcount);
 #endif
 
     int i_sub = 0;
@@ -967,7 +966,6 @@ static int SubpictureRegionsLink(vout_display_t *vd, const picture_t *picture)
         i_sub++;
     }
 
-    cache_ref(cache);
     vlc_mutex_unlock(&sys->cache_lock);
     return VLC_SUCCESS;
 
@@ -1016,7 +1014,8 @@ static void SubpictureRegionsUnlink(vout_display_t *vd, picture_t *picture)
     assert(cache->i_id > 0);
 
 #ifdef VAAPI_DEBUG
-        msg_Dbg(vd, "unlink surface %d cache %d", surface->i_id, surface->i_cache);
+        msg_Dbg(vd, "unlink surface %d cache %d size %d",
+                surface->i_id, surface->i_cache, count);
 #endif
 
     const int count = vlc_array_count(&cache->subpictures);
@@ -1040,24 +1039,56 @@ static void SubpictureRegionsUnlink(vout_display_t *vd, picture_t *picture)
         }
     }
 
+error:
+    vlc_mutex_unlock(&sys->cache_lock);
+}
+
+/* NOTE: call with sys->cache_lock held */
+static void cache_SubpictureRelease(vout_display_t *vd, const int cache_id)
+{
+    vout_display_sys_t *sys = vd->sys;
+
+    assert(cache_id > 0);
+
+    subpicture_cache_t *cache = cache_find(&sys->cache, cache_id);
+    if (cache)
+    {
+#ifdef VAAPI_DEBUG
+        msg_Info(vd, "Release cache %d count %d refcount %d",
+                 cache->i_id, vlc_array_count(&sys->cache), cache->i_refcount);
+#endif
         cache_unref(cache);
 
         /* Do we still need need this subpicture cache ? */
-    if ((cache->i_refcount == 0) &&
-        (sys->render.i_cache != surface->i_cache) &&
-        (mdate() > cache->i_stop))
+        if (cache->i_refcount == 0)
         {
+#ifdef VAAPI_DEBUG
+            msg_Info(vd, "Deleting subpicture cache %d refcount %d",
+                     cache->i_id, cache->i_refcount);
+#endif
             int index = vlc_array_index_of_item(&sys->cache, (void*)cache);
             vlc_array_remove(&sys->cache, index);
-        SubpictureDestroy(vd, sys->conn, cache);
+            cache_SubpictureDestroy(vd, sys->conn, cache);
             cache_destroy(cache);
             cache = NULL;
         }
+    }
+}
 
-    surface->i_cache = 0;
+/* NOTE: call with sys->cache_lock held */
+static void cache_SubpictureHold(vout_display_t *vd, const int cache_id)
+{
+    vout_display_sys_t *sys = vd->sys;
 
-error:
-    vlc_mutex_unlock(&sys->cache_lock);
+    subpicture_cache_t *cache = cache_find(&sys->cache, cache_id);
+    if (cache)
+    {
+#ifdef VAAPI_DEBUG
+        msg_Info(vd, "Subpicture cache %d refcount %d",
+                     cache->i_id, cache->i_refcount);
+#endif
+        cache_ref(cache);
+    }
 }
 
 static int RenderDirectSubpicture(vout_display_t *vd, picture_t *picture, subpicture_t *subpicture)
@@ -1068,9 +1099,13 @@ static int RenderDirectSubpicture(vout_display_t *vd, picture_t *picture, subpic
     subpicture_cache_t *cache = cache_new();
     if (!cache)
         return VLC_EGENERIC;
+
     cache->i_start = subpicture->i_start;
     cache->i_stop  = subpicture->i_stop;
 
+    msg_Info(vd, "Subpicture cache count %d (%d/%d)",
+             vlc_array_count(&sys->cache), sys->render.i_cache, cache->i_id);
+
 #ifdef VAAPI_DEBUG
     msg_Dbg(vd, "new subpicture %p prev %p cache %d surface %d",
             subpicture, sys->render.prev_subpicture, cache->i_id, surface->i_id);
@@ -1089,7 +1124,7 @@ static int RenderDirectSubpicture(vout_display_t *vd, picture_t *picture, subpic
 
         /* Create Subpicture */
         vasubpicture_cache_t *vasub_cache;
-        vasub_cache = SubpictureCreate(vd, subpicture, region, sys->sflags);
+        vasub_cache = cache_SubpictureCreate(vd, subpicture, region, sys->sflags);
         if (vasub_cache == NULL)
             goto cleanup;
 
@@ -1106,17 +1141,16 @@ static int RenderDirectSubpicture(vout_display_t *vd, picture_t *picture, subpic
     /* remember what was created, so it can be released in DisplayPicture() */
     vlc_mutex_lock(&sys->cache_lock);
     vlc_array_append(&sys->cache, (void *)cache);
-    //cache_ref(cache);
+    cache_ref(cache);
     sys->render.i_cache = cache->i_id;
     surface->i_cache = cache->i_id;
     vlc_mutex_unlock(&sys->cache_lock);
     return VLC_SUCCESS;
 
 cleanup:
-    surface->i_cache = VA_INVALID_ID;
     msg_Err(vd, "failed creating subpicture cache %d for surface %d",
             cache->i_id, surface->i_id);
-    SubpictureDestroy(vd, sys->conn, cache);
+    cache_SubpictureDestroy(vd, sys->conn, cache);
     cache_destroy(cache);
     cache = NULL;
     return VLC_EGENERIC;
@@ -1183,7 +1217,7 @@ static int RenderCachedSubpictures(vout_display_t *vd, picture_t *picture,
         goto cleanup;
 
     picture->p_sys->i_cache = cache->i_id;
-    //cache_ref(cache);
+    cache_ref(cache);
     vlc_mutex_unlock(&sys->cache_lock);
     return VLC_SUCCESS;
 
@@ -1191,7 +1225,6 @@ cleanup:
     /* Cannot reuse this cached subpicture */
     msg_Err(vd, "cannot reuse cached subpicture %d for surface %d -- resetting",
             cache->i_id, surface->i_id);
-    sys->render.i_cache = 0;
     sys->render.prev_subpicture = NULL;
     vlc_mutex_unlock(&sys->cache_lock);
     return VLC_EGENERIC;
@@ -1226,21 +1259,26 @@ static void Render(vout_display_t *vd, picture_t *picture, subpicture_t *subpict
 again:
     /* same as previous subpicture? */
     if ((sys->render.prev_subpicture == subpicture) &&
-        (sys->render.i_cache > 0) &&
-        (sys->render.i_start == subpicture->i_start) &&
-        (sys->render.i_stop  == subpicture->i_stop))
+        (sys->render.i_cache > 0))
     {
-        assert(sys->render.i_cache > 0);
         if (RenderCachedSubpictures(vd, picture, subpicture) != VLC_SUCCESS)
             goto again;
     }
     else
     {
+        /* Release exra reference */
+        if (sys->render.i_cache > 0)
+            cache_SubpictureRelease(vd, sys->render.i_cache);
+
+        /* render new subpicture */
         if (RenderDirectSubpicture(vd, picture, subpicture) != VLC_SUCCESS)
             return;
         sys->render.prev_subpicture = subpicture;
         sys->render.i_start = subpicture->i_start;
         sys->render.i_stop  = subpicture->i_stop;
+
+        /* Hold extra reference */
+        cache_SubpictureHold(vd, sys->render.i_cache);
     }
 }
 
@@ -1299,6 +1337,12 @@ out:
     {
         if (ret == VLC_SUCCESS)
             SubpictureRegionsUnlink(vd, picture);
+
+        vlc_mutex_lock(&sys->cache_lock);
+        cache_SubpictureRelease(vd, picture->p_sys->i_cache);
+        picture->p_sys->i_cache = 0;
+        vlc_mutex_unlock(&sys->cache_lock);
+
         subpicture_Delete(subpicture);
     }