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

[Salah-Eddin Shaban]

On Fri, Mar 3, 2017 at 7:14 PM, Rémi Denis-Courmont <remi at remlab.net> wrote:
> 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.
>

Resize events cause the subtitle to be re-rendered, and these two
fields will be updated correctly in that case. The resize issue was
caused by the subtitle being marked absolute.

These fields are not meant for resizes. They are meant for when two
subtitles are rendered at the same time with, say
SUBPICTURE_ALIGN_BOTTOM. When passed to the text renderer they are
given their normal positions as if each one of them were the only one.

During the time they appear simultaneously on the screen, the vout
core takes care of repositioning them to avoid overlapping. This
positioning happens on each frame, so if one of the subs expires the
other is returned to its normal position.

There might indeed be more elegant ways to do it. But I myself am new
to the core parts of the SPU code, and am learning as I go.