[FFmpeg-devel] [PATCH] avcodec/nvenc: fix b-frame DTS behavior with fractional framerates

[Timo Rothenpieler]

On 03.04.2023 00:02, tt2468 wrote:
> When using fractional framerates (or any fraction with a numerator != 1),
> DTS values for packets would be calculated incorrectly.
> 
> Signed-off-by: tt2468 <tt2468 at irltoolkit.com>

If you want to sign your commits off, please use a proper name.
A real name is not strictly required, but without it, a Sign-off becomes 
a bit pointless.

> ---
>   libavcodec/nvenc.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
> index 9acf3e8697..8b02856507 100644
> --- a/libavcodec/nvenc.c
> +++ b/libavcodec/nvenc.c
> @@ -2266,7 +2266,8 @@ static int nvenc_set_timestamp(AVCodecContext *avctx,
>       dts = reorder_queue_dequeue(ctx->reorder_queue, avctx, pkt);
>   
>       if (avctx->codec_descriptor->props & AV_CODEC_PROP_REORDER) {
> -        pkt->dts = dts - FFMAX(ctx->encode_config.frameIntervalP - 1, 0) * FFMAX(avctx->ticks_per_frame, 1);
> +        pkt->dts = dts -
> +            FFMAX(ctx->encode_config.frameIntervalP - 1, 0) * FFMAX(avctx->time_base.num * avctx->ticks_per_frame, avctx->time_base.num);

What if the time_base is unset?
It's not really used anywhere else, so it might potentially never be set 
by some users.
The whole right-side term has to at least stay 1.
So probably something clunky like the following is needed:

> FFMAX(avctx->time_base.num * avctx->ticks_per_frame, FFMAX(avctx->time_base.num, 1))


I'm also not sure if this is the right way to calculate this value still.
What if someone sets a super weird timebase, like the mpeg-ts one?
Not like it'd work correctly as it is right now, so I'm more wondering 
on how to make this more robust in general.

Definitely looks like an improvement though, given that timebases like 
1001/30000 exist, where just subtracting 1 or 2 would do barely anything.