Re: [FFmpeg-devel] [PATCH 16/21] fftools/ffmpeg: rework audio-decode timestamp handling

From: Anton Khirnov <anton@khirnov.net>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH 16/21] fftools/ffmpeg: rework audio-decode timestamp handling
Date: Fri, 28 Apr 2023 15:11:16 +0200
Message-ID: <168268747635.3843.467810343388947412@lain.khirnov.net> (raw)
In-Reply-To: <20230428114217.GE275832@pb2>

Quoting Michael Niedermayer (2023-04-28 13:42:17)
> On Thu, Apr 27, 2023 at 04:25:56PM +0200, Anton Khirnov wrote:
> > Stop using InputStream.dts for generating missing timestamps for decoded
> > frames, because it contains pre-decoding timestamps and there may be
> > arbitrary amount of delay between input packets and output frames (e.g.
> > dependent on the thread count when frame threading is used). It is also
> > in AV_TIME_BASE (i.e. microseconds), which may introduce unnecessary
> > rounding issues.
> > 
> 
> > New code maintains a timebase that is the inverse of the LCM of all the
> > samplerates seen so far, and thus can accurately represent every audio
> 
> if the LCM fits in int32
> 
> This can hit some pathologic cases though
> consider a 192khz stream that starts with a damaged packet thats read as 11.197 khz
> lcm of 192000 and 11197 > 2^31 so the whole stream will then be stuck with 11.197khz
> that seems like a bad choice
> the code should favor standard sample rates as well as the higher sample rate if
> the lcm is not representable
> 
> also if lets say there are 48khz and 48.001khz where again lcm doesnt work 
> then a multiple of 48khz may be a better choice than either itself

Thank you, there are good points. I'm wondering if just picking the LCM
of all common samplerates (28224000 = lcm(192000, 44100)) wouldn't be
sufficient for all these pathological cases. Or do you have a better
general algorithm in mind? Maybe fall back on AV_TIME_BASE instead?

> also what happens if the audio timestamps are known more precissely than
> the audio sample rate ?

I'm not sure that is very relevant in real-world uses cases, but I've
added this snippet which should address this:

// keep the frame timebase if it is strictly better than
// the samplerate-defined one
if (frame->time_base.num == 1 && frame->time_base.den > tb_new.den &&
    !(frame->time_base.den % tb_new.den))
    tb_new = frame->time_base;

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".