[vlc-devel] [PATCH] video_output: vout_subpictures: prevent deadlock when subpicture heap is full

[Thomas Guillem]

On Fri, Oct 12, 2018, at 03:45, Juan Lledo wrote:
> In SpuSelectSubpictures(), SpuHeapDeleteAt() only runs when selecting
> valid subpictures to display. If the subpicture heap is full and all
> entries are rejected, there will not be any valid subpictures so the
> for loop is skipped due to "channel_count" being 0.
> 
> This fix should yield a similar result while avoiding the deadlock due
> to running in an earlier loop.
> 
> Fixes #19353. I believe scrolling on a trackpad on Mac could sometimes
> send events quickly enough that the whole heap would fill up and get
> invalidated before SpuSelectSubpictures() could run again.
> ---
>  src/video_output/vout_subpictures.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/src/video_output/vout_subpictures.c b/src/video_output/
> vout_subpictures.c
> index 682461a..04c241f 100644
> --- a/src/video_output/vout_subpictures.c
> +++ b/src/video_output/vout_subpictures.c
> @@ -547,8 +547,11 @@ 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 || entry->reject) {
> +            if (entry->reject)
> +                SpuHeapDeleteAt(&sys->heap, index);
>              continue;
> +        }
>          const int i_channel = entry->subpicture->i_channel;
>          int i;
>          for (i = 0; i < channel_count; i++) {
> @@ -578,11 +581,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 || entry->reject)

Then, entry->reject is always false if current is NULL. So the entry-reject check becomes useless here. We could put an assert instead, right ?

>                  continue;
> -            }
>  
>              if (current->i_channel != channel[i] ||
>                 (ignore_osd && !current->b_subtitle))
> -- 
> 2.6.4
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel