[vlc-devel] [vlc-commits] codec: subsvtt: unbreak rendering (fix #22425)

[Thomas Guillem]

On Fri, Jun 14, 2019, at 15:31, Rémi Denis-Courmont wrote:
> Le vendredi 14 juin 2019, 11:28:42 EEST Francois Cartegnie a écrit :
> > vlc | branch: master | Francois Cartegnie <fcvlcdev at free.fr> | Fri Jun 14
> > 10:27:31 2019 +0200| [749b7b3729a5892d1a07036ef0974c17ddbe9e2e] |
> > committer: Francois Cartegnie
> > 
> > codec: subsvtt: unbreak rendering (fix #22425)
> > 
> > 5ef3830f385bfa6a47e9088f9a0d13062cd61c3a regression
> > 
> > > http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=749b7b3729a5892d1a0
> > > 7036ef0974c17ddbe9e2e
> > ---
> > 
> >  modules/codec/webvtt/subsvtt.c | 6 +-----
> >  1 file changed, 1 insertion(+), 5 deletions(-)
> > 
> > diff --git a/modules/codec/webvtt/subsvtt.c b/modules/codec/webvtt/subsvtt.c
> > index f9b686ebda..8e18edefab 100644
> > --- a/modules/codec/webvtt/subsvtt.c
> > +++ b/modules/codec/webvtt/subsvtt.c
> > @@ -1857,11 +1857,7 @@ static void Render( decoder_t *p_dec, vlc_tick_t
> > i_start, vlc_tick_t i_stop )
> > 
> >      GetTimedTags( p_sys->p_root->p_child, i_start, i_stop, &timedtags );
> >      if( timedtags.i_count == 0 )
> > -    {
> > -        vlc_array_clear( &timedtags );
> > -        return;
> > -    }
> > -    qsort( timedtags.pp_elems, timedtags.i_count,
> > sizeof(*timedtags.pp_elems), timedtagsArrayCmp );
> > +        qsort(
> > timedtags.pp_elems, timedtags.i_count, sizeof(*timedtags.pp_elems),
> > timedtagsArrayCmp );
> 
> This is obviously completely nonsensical. Literally, if i_count is zero, it's 
> UB, qsort(NULL). That is blatantly obvious if you just read the description of 
> the cited 5ef3830 Hugo's changeset. Meanwhile if i_count is non-zero it does 
> not sort anymore, making the sort() call useless.
> 
> I really don't think it's appropriate to push this in 1 minute and then 
> backport to LTS another 2 minutes without even proof-reading the code.

+1

Maybe the following code path:

    if( i_substart != i_stop )
    {
        if( i_substart != i_start )
            ClearCSSStyles( (webvtt_dom_node_t *)p_sys->p_root );
        RenderRegions( p_dec, i_substart, i_stop );
    }

need to be also executed when the count is 0. That's the only possible regression I see from Hugo's commit.

> 
> -- 
> Rémi Denis-Courmont
> 
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel