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

[Thomas Guillem]

OK, merged, thanks!

On Sun, Sep 30, 2018, at 03:20, Zhao Zhili wrote:
> 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
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel