[vlc-commits] vout: spu: destroy subpicture from anywhere

[Thomas Guillem]

vlc | branch: master | Thomas Guillem <thomas at gllm.fr> | Wed Jun  5 08:36:31 2019 +0200| [d2eed69717a0e9b610b2d93cf2b95e2e00932563] | committer: Thomas Guillem

vout: spu: destroy subpicture from anywhere

The /* You cannot delete subpicture outside of SpuSelectSubpictures */ comment
does not seem valid anymore.

Went through all subpicture_updater_t implementations in modules and src. I
don't think there is one that need to called from the same thread than
spu_Render(). The implementations  that need synchronization are already using
mutexes. Most of implementation don't need any synchronization but just to free
sys and a list of regions (that are populated before sending the spu to the
vout).

No possible use-after-free either. The subpicture returned by spu_Render() is
a new one with new allocated regions. Output regions fmt and other variables
are copied from the original subpictures regions. The output region pictures
are held from the original one too.

Everything is done with a mutex held. So even if an original subpicture is
deleted from an other thread (the decoder one likely), it won't destroy the
actual picture data returned by spu_Render() until all regions picture are
released.

> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=d2eed69717a0e9b610b2d93cf2b95e2e00932563
---

 src/video_output/vout_subpictures.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/src/video_output/vout_subpictures.c b/src/video_output/vout_subpictures.c
index 0cb80fc1ed..3285c58e2c 100644
--- a/src/video_output/vout_subpictures.c
+++ b/src/video_output/vout_subpictures.c
@@ -60,7 +60,6 @@ typedef struct {
 
 typedef struct {
     subpicture_t *subpicture;
-    bool          reject;
 } spu_heap_entry_t;
 
 typedef struct {
@@ -113,7 +112,6 @@ static void SpuHeapInit(spu_heap_t *heap)
         spu_heap_entry_t *e = &heap->entry[i];
 
         e->subpicture = NULL;
-        e->reject     = false;
     }
 }
 
@@ -126,7 +124,6 @@ static int SpuHeapPush(spu_heap_t *heap, subpicture_t *subpic)
             continue;
 
         e->subpicture = subpic;
-        e->reject     = false;
         return VLC_SUCCESS;
     }
     return VLC_EGENERIC;
@@ -608,7 +605,7 @@ static void SpuSelectSubpictures(spu_t *spu,
 
     for (int index = 0; index < VOUT_MAX_SUBPICTURES; index++) {
         spu_heap_entry_t *entry = &sys->heap.entry[index];
-        if (!entry->subpicture || entry->reject)
+        if (!entry->subpicture)
             continue;
         const int i_channel = entry->subpicture->i_channel;
         int i;
@@ -620,10 +617,6 @@ static void SpuSelectSubpictures(spu_t *spu,
             channel[channel_count++] = i_channel;
     }
 
-    /* Ensure a 1 pass garbage collection for rejected subpictures */
-    if(channel_count == 0)
-        channel[channel_count++] = VOUT_SPU_CHANNEL_INVALID;
-
     /* Fill up the subpicture_array arrays with relevant pictures */
     for (int i = 0; i < channel_count; i++) {
         spu_render_entry_t available_entries[VOUT_MAX_SUBPICTURES];
@@ -644,11 +637,8 @@ static void SpuSelectSubpictures(spu_t *spu,
             bool is_stop_valid;
             bool is_late;
 
-            if (!current || entry->reject) {
-                if (entry->reject)
-                    SpuHeapDeleteAt(&sys->heap, index);
+            if (!current)
                 continue;
-            }
 
             if (current->i_channel != channel[i] ||
                (ignore_osd && !current->b_subtitle))
@@ -1756,8 +1746,7 @@ void spu_ClearChannel(spu_t *spu, int channel)
             (channel != VOUT_SPU_CHANNEL_INVALID || subpic->i_channel == VOUT_SPU_CHANNEL_OSD))
             continue;
 
-        /* You cannot delete subpicture outside of SpuSelectSubpictures */
-        entry->reject = true;
+        SpuHeapDeleteAt(&sys->heap, i);
     }
 
     vlc_mutex_unlock(&sys->lock);