[vlc-devel] [PATCH] demux: avformat: use native rescaling

[Steve Lhomme]

On 2020-11-30 11:25, Francois Cartegnie wrote:
> Comparison & arithmetic through different time bases
> are bogus (start_offset timebase vs streams pts timebase) when
> rescaled using our own helpers.
> We need to use the same rescaling functions to avoid different
> roundings, then errors.
> refs #25117
> ---
>   modules/codec/avcodec/avcommon.h |  5 +++++
>   modules/demux/avformat/demux.c   | 33 ++++++++++----------------------
>   2 files changed, 15 insertions(+), 23 deletions(-)
> 
> diff --git a/modules/codec/avcodec/avcommon.h b/modules/codec/avcodec/avcommon.h
> index acd4790651..0efc6dfc7c 100644
> --- a/modules/codec/avcodec/avcommon.h
> +++ b/modules/codec/avcodec/avcommon.h
> @@ -42,9 +42,14 @@
>   # include <libavutil/cpu.h>
>   # include <libavutil/log.h>
>   
> +#define VLC_TIME_BASE_Q     (AVRational){1, CLOCK_FREQ}
> +
>   #if (CLOCK_FREQ == AV_TIME_BASE)
>   #define FROM_AV_TS(x)  (x)
>   #define TO_AV_TS(x)    (x)
> +#elif defined(USE_AV_RESCALEQ) /* until we migrate all conversions */
> +#define FROM_AV_TS(x)  av_rescale_q(x, AV_TIME_BASE_Q, VLC_TIME_BASE_Q)
> +#define TO_AV_TS(x)    av_rescale_q(x, VLC_TIME_BASE_Q, AV_TIME_BASE_Q)
>   #elif (CLOCK_FREQ % AV_TIME_BASE) == 0
>   #define FROM_AV_TS(x)  ((x) * (CLOCK_FREQ / AV_TIME_BASE))
>   #define TO_AV_TS(x)    ((x) / (CLOCK_FREQ / AV_TIME_BASE))
> diff --git a/modules/demux/avformat/demux.c b/modules/demux/avformat/demux.c
> index 91c4e005f4..834f672931 100644
> --- a/modules/demux/avformat/demux.c
> +++ b/modules/demux/avformat/demux.c
> @@ -37,6 +37,8 @@
>   #include <vlc_charset.h>
>   #include <vlc_avcodec.h>
>   
> +#define USE_AV_RESCALEQ
> +
>   #include "../../codec/avcodec/avcodec.h"
>   #include "../../codec/avcodec/chroma.h"
>   #include "../../codec/avcodec/avcommon_compat.h"
> @@ -748,10 +750,9 @@ int avformat_OpenDemux( vlc_object_t *p_this )
>               EnsureUTF8( s->psz_name );
>               msg_Dbg( p_demux, "    - chapter %d: %s", i, s->psz_name );
>           }
> -        s->i_time_offset = vlc_tick_from_samples( p_sys->ic->chapters[i]->start *
> -            p_sys->ic->chapters[i]->time_base.num,
> -            p_sys->ic->chapters[i]->time_base.den ) -
> -            (i_start_time != VLC_TICK_INVALID ? i_start_time : 0 );
> +        s->i_time_offset = av_rescale_q( p_sys->ic->chapters[i]->start,
> +                                         p_sys->ic->chapters[i]->time_base, VLC_TIME_BASE_Q )

I think you should do a macro/inline function that explicitely outputs a 
vlc_tick_t using av_rescale_q(x,y,VLC_TIME_BASE_Q)

> +                 - (i_start_time != VLC_TICK_INVALID ? i_start_time : 0 );
>           TAB_APPEND( p_sys->p_title->i_seekpoint, p_sys->p_title->seekpoint, s );
>       }
>   
> @@ -859,7 +860,7 @@ static int Demux( demux_t *p_demux )
>       /* Used to avoid timestamps overlow */
>       if( p_sys->ic->start_time != (int64_t)AV_NOPTS_VALUE )
>       {
> -        i_start_time = vlc_tick_from_frac(p_sys->ic->start_time, AV_TIME_BASE);
> +        i_start_time = FROM_AV_TS(p_sys->ic->start_time);
>       }
>       else
>           i_start_time = 0;
> @@ -868,7 +869,7 @@ static int Demux( demux_t *p_demux )
>           p_frame->i_dts = VLC_TICK_INVALID;
>       else
>       {
> -        p_frame->i_dts = vlc_tick_from_frac( pkt.dts * p_stream->time_base.num, p_stream->time_base.den )
> +        p_frame->i_dts = av_rescale_q( pkt.dts, p_stream->time_base, VLC_TIME_BASE_Q )
>                   - i_start_time + VLC_TICK_0;
>       }
>   
> @@ -876,13 +877,11 @@ static int Demux( demux_t *p_demux )
>           p_frame->i_pts = VLC_TICK_INVALID;
>       else
>       {
> -        p_frame->i_pts = vlc_tick_from_frac( pkt.pts * p_stream->time_base.num, p_stream->time_base.den )
> +        p_frame->i_pts = av_rescale_q( pkt.pts, p_stream->time_base, VLC_TIME_BASE_Q )
>                   - i_start_time + VLC_TICK_0;
>       }
>       if( pkt.duration > 0 && p_frame->i_length <= 0 )
> -        p_frame->i_length = vlc_tick_from_samples(pkt.duration *
> -            p_stream->time_base.num,
> -            p_stream->time_base.den );
> +        p_frame->i_length = av_rescale_q( pkt.duration, p_stream->time_base, VLC_TIME_BASE_Q );
>   
>       /* Add here notoriously bugged file formats/samples */
>       if( !strcmp( p_sys->fmt->name, "flv" ) )
> @@ -970,21 +969,9 @@ static void ResetTime( demux_t *p_demux, int64_t i_time )
>       vlc_tick_t t;
>   
>       if( p_sys->ic->start_time == (int64_t)AV_NOPTS_VALUE || i_time < 0 )
> -    {
>           t = VLC_TICK_INVALID;
> -    }
>       else
> -    {
> -#if CLOCK_FREQ == AV_TIME_BASE
> -        t = FROM_AV_TS(i_time);
> -#else
> -        lldiv_t q = lldiv( i_time, AV_TIME_BASE );
> -        t = vlc_tick_from_sec(q.quot) + FROM_AV_TS(q.rem);
> -#endif
> -
> -        if( t == VLC_TICK_INVALID )
> -            t = VLC_TICK_0;
> -    }
> +        t = VLC_TICK_0 + TO_AV_TS( i_time );

The original value was not shifted by VLC_TICK_0, is this change on 
purpose ?

>   
>       p_sys->i_pcr = t;
>       for( unsigned i = 0; i < p_sys->i_tracks; i++ )
> -- 
> 2.26.2
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>