[FFmpeg-devel] avformat/mux: allow non-monotonic ts with AVFMT_NOTIMESTAMPS

Allow non-monotonic input timestamps for muxers with no timestamps at
all (AVFMT_NOTIMESTAMPS).

This is frequently hit when muxing TrueHD with spdifenc as many MKV
files use 1ms timestamps while TrueHD frames are shorter than that
(1/1200 sec for 48kHz-based and 1/1102.5 sec for 44.1kHz-based rates).
---
 libavformat/mux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Anssi Hannula:
> Allow non-monotonic input timestamps for muxers with no timestamps at
> all (AVFMT_NOTIMESTAMPS).
> 
> This is frequently hit when muxing TrueHD with spdifenc as many MKV
> files use 1ms timestamps while TrueHD frames are shorter than that
> (1/1200 sec for 48kHz-based and 1/1102.5 sec for 44.1kHz-based rates).
> ---
>  libavformat/mux.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/mux.c b/libavformat/mux.c
> index 3533932a58..ba6695badb 100644
> --- a/libavformat/mux.c
> +++ b/libavformat/mux.c
> @@ -622,7 +622,7 @@ static int compute_muxer_pkt_fields(AVFormatContext *s, AVStream *st, AVPacket *
>      }
>  
>      if (st->cur_dts && st->cur_dts != AV_NOPTS_VALUE &&
> -        ((!(s->oformat->flags & AVFMT_TS_NONSTRICT) &&
> +        ((!(s->oformat->flags & (AVFMT_NOTIMESTAMPS | AVFMT_TS_NONSTRICT)) &&
>            st->codecpar->codec_type != AVMEDIA_TYPE_SUBTITLE &&
>            st->codecpar->codec_type != AVMEDIA_TYPE_DATA &&
>            st->cur_dts >= pkt->dts) || st->cur_dts > pkt->dts)) {
> 
The change looks good, but the description is suboptimal: Your change
does not blindly allow non-monotonic timestamps. It just drops the
requirement of strictly monotonic dts for muxers with
AVFMT_NOTIMESTAMPS. This is enough to fix the issue with TrueHD and
1/1000 timebases (which also exists for the null muxer).

- Andreas

Quoting Andreas Rheinhardt (2020-02-26 16:06:00)
> Anssi Hannula:
> > Allow non-monotonic input timestamps for muxers with no timestamps at
> > all (AVFMT_NOTIMESTAMPS).
> > 
> > This is frequently hit when muxing TrueHD with spdifenc as many MKV
> > files use 1ms timestamps while TrueHD frames are shorter than that
> > (1/1200 sec for 48kHz-based and 1/1102.5 sec for 44.1kHz-based rates).
> > ---
> >  libavformat/mux.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libavformat/mux.c b/libavformat/mux.c
> > index 3533932a58..ba6695badb 100644
> > --- a/libavformat/mux.c
> > +++ b/libavformat/mux.c
> > @@ -622,7 +622,7 @@ static int compute_muxer_pkt_fields(AVFormatContext *s, AVStream *st, AVPacket *
> >      }
> >  
> >      if (st->cur_dts && st->cur_dts != AV_NOPTS_VALUE &&
> > -        ((!(s->oformat->flags & AVFMT_TS_NONSTRICT) &&
> > +        ((!(s->oformat->flags & (AVFMT_NOTIMESTAMPS | AVFMT_TS_NONSTRICT)) &&
> >            st->codecpar->codec_type != AVMEDIA_TYPE_SUBTITLE &&
> >            st->codecpar->codec_type != AVMEDIA_TYPE_DATA &&
> >            st->cur_dts >= pkt->dts) || st->cur_dts > pkt->dts)) {
> > 
> The change looks good, but the description is suboptimal: Your change
> does not blindly allow non-monotonic timestamps. It just drops the
> requirement of strictly monotonic dts for muxers with

You mean 'strictly increasing'. The timestamps should still always be
monotonic - i.e. non-decreasing, but possibly staying the same in
adjacent packets.

Unrelated to the patch - that if() blocks is starting to look utterly
unreadable and should be split.

Anton Khirnov:
> Quoting Andreas Rheinhardt (2020-02-26 16:06:00)
>> Anssi Hannula:
>>> Allow non-monotonic input timestamps for muxers with no timestamps at
>>> all (AVFMT_NOTIMESTAMPS).
>>>
>>> This is frequently hit when muxing TrueHD with spdifenc as many MKV
>>> files use 1ms timestamps while TrueHD frames are shorter than that
>>> (1/1200 sec for 48kHz-based and 1/1102.5 sec for 44.1kHz-based rates).
>>> ---
>>>  libavformat/mux.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavformat/mux.c b/libavformat/mux.c
>>> index 3533932a58..ba6695badb 100644
>>> --- a/libavformat/mux.c
>>> +++ b/libavformat/mux.c
>>> @@ -622,7 +622,7 @@ static int compute_muxer_pkt_fields(AVFormatContext *s, AVStream *st, AVPacket *
>>>      }
>>>  
>>>      if (st->cur_dts && st->cur_dts != AV_NOPTS_VALUE &&
>>> -        ((!(s->oformat->flags & AVFMT_TS_NONSTRICT) &&
>>> +        ((!(s->oformat->flags & (AVFMT_NOTIMESTAMPS | AVFMT_TS_NONSTRICT)) &&
>>>            st->codecpar->codec_type != AVMEDIA_TYPE_SUBTITLE &&
>>>            st->codecpar->codec_type != AVMEDIA_TYPE_DATA &&
>>>            st->cur_dts >= pkt->dts) || st->cur_dts > pkt->dts)) {
>>>
>> The change looks good, but the description is suboptimal: Your change
>> does not blindly allow non-monotonic timestamps. It just drops the
>> requirement of strictly monotonic dts for muxers with
> 
> You mean 'strictly increasing'. The timestamps should still always be
> monotonic - i.e. non-decreasing, but possibly staying the same in
> adjacent packets.
> 
Yes, the reason I used "non-monotonic" is because the logmessage uses
this verbiage.

- Andreas