[vlc-devel] [PATCH 3/3] transcode: video: fix insertion of fps filter

[Alexandre Janniaux]

On Tue, Jan 26, 2021 at 07:31:44PM +0100, Francois Cartegnie wrote:
> Le 26/01/2021 à 15:01, Alexandre Janniaux a écrit :
> > -    if( id->p_enccfg->video.fps.num > 0 )
> > +    if( id->p_enccfg->video.fps.num > 0 &&
> > +        id->p_enccfg->video.fps.den > 0 &&
> > +      ( id->p_enccfg->video.fps.num != p_src->video.i_frame_rate ||
> > +        id->p_enccfg->video.fps.den != p_src->video.i_frame_rate_base ) )
> >       {
> > -        filter_chain_AppendFilter( id->p_f_chain, "fps", NULL, p_src );
> > +        es_format_t dst;
> > +        es_format_Copy(&dst, p_src);
> > +        dst.video.i_frame_rate = id->p_enccfg->video.fps.num;
> > +        dst.video.i_frame_rate_base = id->p_enccfg->video.fps.den;
> > +        filter_chain_AppendFilter( id->p_f_chain, "fps", NULL, &dst );
> >           p_src = filter_chain_GetFmtOut( id->p_f_chain );
> >           src_ctx = filter_chain_GetVideoCtxOut( id->p_f_chain );
> > +        es_format_Clean(&dst);
>
> I can just keep complaining about the missing
> init() copy() clean() cycle

Is it needed for es_format_Copy?

    int es_format_Copy(es_format_t *restrict dst, const es_format_t *src)
    {
        int ret = VLC_SUCCESS;

        *dst = *src;

The dst es_format_t seems to be overriden no matter what,
before anything even happens, and the documentation doesn't
seem to require it either.

The init() copy() clean() cycle seems to only exist in bluray,
mp4, adaptive and a bit of transcode.

What would seem harmful to me is calling es_format_Clean() on
a non-initialized es_format but it's clearly initialized right
after it's declaration by es_format_Copy, or maybe I miss
something there.

Regards,
--
Alexandre Janniaux
Videolabs