[vlc-devel] [PATCH 3/3] Subtitles: fix misplaced subs on video resize

[Rémi Denis-Courmont]

Le perjantaina 3. maaliskuuta 2017, 19.05.53 EET Salah-Eddin Shaban a écrit :
> When handling of overlapped subtitles was introduced all subtitles were
> marked as absolute on first rendering. This caused the position to be
> calculated only once and not be updated when the video size changed. ---
>  include/vlc_subpicture.h            |  8 ++++++++
>  src/misc/subpicture.c               |  6 ++++++
>  src/video_output/vout_subpictures.c | 10 ++++------
>  3 files changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/include/vlc_subpicture.h b/include/vlc_subpicture.h
> index aa2ed0d..3c07a2a 100644
> --- a/include/vlc_subpicture.h
> +++ b/include/vlc_subpicture.h
> @@ -61,6 +61,14 @@ struct subpicture_region_t
> 
>      int             i_x;                             /**< position of
> region */ int             i_y;                             /**< position of
> region */ +
> +    /**
> +     * used to save region position in case it has to be changed
> +     * temporarily e.g. because of overlapped subtitles
> +     */
> +    int             i_original_x;
> +    int             i_original_y;
> +

Frankly, it looks like a kludge around a kludge. I do not know the SPU code 
much but I doubt that this is right.

What if multiple resize occur in short succession? It´s actually likely when 
the user drags the edge of the window.

>      int             i_align;                  /**< alignment within a
> region */
> int             i_alpha;                               /**< transparency */
> 
> diff --git a/src/misc/subpicture.c b/src/misc/subpicture.c
> index f18be17..9f8eb1c 100644
> --- a/src/misc/subpicture.c
> +++ b/src/misc/subpicture.c
> @@ -170,6 +170,12 @@ void subpicture_Update( subpicture_t *p_subpicture,
> 
>      p_upd->pf_update( p_subpicture, p_fmt_src, p_fmt_dst, i_ts );
> 
> +    for( subpicture_region_t *r = p_subpicture->p_region; r; r = r->p_next
> ) +    {
> +        r->i_original_x = r->i_x;
> +        r->i_original_y = r->i_y;
> +    }
> +
>      video_format_Clean( &p_private->src );
>      video_format_Clean( &p_private->dst );
> 
> diff --git a/src/video_output/vout_subpictures.c
> b/src/video_output/vout_subpictures.c index bbccaf4..cc3bf57 100644
> --- a/src/video_output/vout_subpictures.c
> +++ b/src/video_output/vout_subpictures.c
> @@ -1024,13 +1024,13 @@ static subpicture_t *SpuRenderSubpictures(spu_t
> *spu, sys->text->fmt_out.video.i_visible_height =
> subpic->i_original_picture_height; }
> 
> -        /* Render all regions
> -         * We always transform non absolute subtitle into absolute one on
> the -         * first rendering to allow good subtitle overlap support. -  
>       */
> +        /* Render all regions */
>          for (region = subpic->p_region; region != NULL; region =
> region->p_next) { spu_area_t area;
> 
> +            region->i_x = region->i_original_x;
> +            region->i_y = region->i_original_y;
> +
>              /* Compute region scale AR */
>              video_format_t region_fmt = region->fmt;
>              if (region_fmt.i_sar_num <= 0 || region_fmt.i_sar_den <= 0) {
> @@ -1072,8 +1072,6 @@ static subpicture_t *SpuRenderSubpictures(spu_t *spu,
>                      subtitle_area[subtitle_area_count++] = area;
>              }
>          }
> -        if (subpic->b_subtitle && subpic->p_region)
> -            subpic->b_absolute = true;
>      }
> 
>      /* */


-- 
雷米‧德尼-库尔蒙
https://www.remlab.net/