[vlc-devel] [PATCH 1/2] demux: subtitle: set length after subtitle reorder

[Zhao Zhili]

Hi Filip,

On 2018年09月30日 04:28, Filip Roséen wrote:
> Hi Zhao,
>
> On 2018-09-30 01:21, Zhao Zhili wrote:
>
>> Ping for review.
> Given that I was the one that suggested these changes to be made, it
> might not be too surprising that I would give you a *LGTM* in terms of
> change in behavior. I am however a little sad to see one comment being
> lost as part of moving the block of code, with that said: the
> code-block is easy enough to understand even without said comment - so
> in my book it is not the end of the world.

This is what the comment comments in 03ed4cc

-    /* *** fix subtitle (order and time) *** */
-    p_sub->i_subtitle = 0;  /* will be modified by sub_fix */
-
-    if( p_sub->i_sub_type != SUB_TYPE_VOBSUB )
+    /* Fix subtitle (order and time) *** */
+    p_sys->i_subtitle = 0;
+    if( p_sys->i_type != SUB_TYPE_VOBSUB )
+    {
+        Fix( p_demux );
+    }

The comment has lost its context so I decide to remove it.

>
> /F
>   
>> 于 2018年9月24日 GMT+08:00 下午5:43:25, Zhao Zhili <quinkblack at foxmail.com> 写到:
>>>
>>>> On Sep 24, 2018, at 5:10 PM, Thomas Guillem <thomas at gllm.fr> wrote:
>>>>
>>>> Why ?
>>> p_sys->i_length =
>>> p_sys->subtitles.p_array[p_sys->subtitles.i_count-1].i_stop;
>>>
>>> The i_length field is set according to the PTS of the last frame. The
>>> last frame may not have the largest PTS before reorder.
>>>
>>> cf.
>>> https://mailman.videolan.org/pipermail/vlc-devel/2018-September/121233.html
>>>
>>>
>>>> On Thu, Sep 20, 2018, at 17:36, Zhao Zhili wrote:
>>>>> ---
>>>>> modules/demux/subtitle.c | 11 +++++------
>>>>> 1 file changed, 5 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/modules/demux/subtitle.c b/modules/demux/subtitle.c
>>>>> index 3b9836587a..4b7eeefd59 100644
>>>>> --- a/modules/demux/subtitle.c
>>>>> +++ b/modules/demux/subtitle.c
>>>>> @@ -676,12 +676,6 @@ static int Open ( vlc_object_t *p_this )
>>>>>
>>>>>      msg_Dbg(p_demux, "loaded %zu subtitles",
>>> p_sys->subtitles.i_count );
>>>>> -    /* Fix subtitle (order and time) *** */
>>>>> -    p_sys->subtitles.i_current = 0;
>>>>> -    p_sys->i_length = 0;
>>>>> -    if( p_sys->subtitles.i_count > 0 )
>>>>> -        p_sys->i_length = p_sys->subtitles.p_array[p_sys-
>>>>>> subtitles.i_count-1].i_stop;
>>>>> -
>>>>>      /* *** add subtitle ES *** */
>>>>>      if( p_sys->props.i_type == SUB_TYPE_SSA1 ||
>>>>>               p_sys->props.i_type == SUB_TYPE_SSA2_4 ||
>>>>> @@ -698,6 +692,11 @@ static int Open ( vlc_object_t *p_this )
>>>>>      else
>>>>>          es_format_Init( &fmt, SPU_ES, VLC_CODEC_SUBT );
>>>>>
>>>>> +    p_sys->subtitles.i_current = 0;
>>>>> +    p_sys->i_length = 0;
>>>>> +    if( p_sys->subtitles.i_count > 0 )
>>>>> +        p_sys->i_length = p_sys->subtitles.p_array[p_sys-
>>>>>> subtitles.i_count-1].i_stop;
>>>>> +
>>>>>      /* Stupid language detection in the filename */
>>>>>      char * psz_language = get_language_from_filename( p_demux-
>>>>>> psz_filepath );
>>>>> -- 
>>>>> 2.19.0.rc1
>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> vlc-devel mailing list
>>>>> To unsubscribe or modify your subscription options:
>>>>> https://mailman.videolan.org/listinfo/vlc-devel
>>>> _______________________________________________
>>>> vlc-devel mailing list
>>>> To unsubscribe or modify your subscription options:
>>>> https://mailman.videolan.org/listinfo/vlc-devel
>> -- 
>> 使用 K-9 Mail 发送自我的Android设备。
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
>
>
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel