| Message ID | 20210930103617.286579-1-onemda@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avformat/mux: fix overflow in case one of dts in not set | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
On Thu, Sep 30, 2021 at 12:36:17PM +0200, Paul B Mahol wrote: > Signed-off-by: Paul B Mahol <onemda@gmail.com> > --- > libavformat/mux.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavformat/mux.c b/libavformat/mux.c > index 2053a5636e..583328b123 100644 > --- a/libavformat/mux.c > +++ b/libavformat/mux.c > @@ -949,7 +949,7 @@ int ff_interleave_packet_per_dts(AVFormatContext *s, AVPacket *out, > last_dts = av_rescale_q(last->pkt.dts, > st->time_base, > AV_TIME_BASE_Q); > - delta_dts = FFMAX(delta_dts, last_dts - top_dts); > + delta_dts = FFMAX(delta_dts, (top_dts == AV_NOPTS_VALUE || last_dts == AV_NOPTS_VALUE) ? 0 : last_dts - top_dts); > } In which case are packets without DTS interleaved by DTS ? thx [...]
Michael Niedermayer: > On Thu, Sep 30, 2021 at 12:36:17PM +0200, Paul B Mahol wrote: >> Signed-off-by: Paul B Mahol <onemda@gmail.com> >> --- >> libavformat/mux.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libavformat/mux.c b/libavformat/mux.c >> index 2053a5636e..583328b123 100644 >> --- a/libavformat/mux.c >> +++ b/libavformat/mux.c >> @@ -949,7 +949,7 @@ int ff_interleave_packet_per_dts(AVFormatContext *s, AVPacket *out, >> last_dts = av_rescale_q(last->pkt.dts, >> st->time_base, >> AV_TIME_BASE_Q); >> - delta_dts = FFMAX(delta_dts, last_dts - top_dts); >> + delta_dts = FFMAX(delta_dts, (top_dts == AV_NOPTS_VALUE || last_dts == AV_NOPTS_VALUE) ? 0 : last_dts - top_dts); >> } > > In which case are packets without DTS interleaved by DTS ? > E.g. in case of attached pictures. Other possible cases are e.g. the null muxer or more generally muxers with AVFMT_NOTIMESTAMPS. An alternative solution for this that I have been thinking about for a while but which I have not implemented yet goes as follows: 1. Use a dedicated interleave function for the null muxer that does not interleave at all, but just returns the packets as they are (it doesn't matter for the null muxer). 2. For attached pictures (not timed thumbnails) the concept of timestamps does not really make sense; ergo don't interleave them, but let the generic muxing code put them into AVStream->attached_pic or AVStream->attachment (to be added as a replacement for attached_pic) before the interleavement functions get called. Also check generically that there is only one packet for such a stream and remove the checks in e.g. flacenc and several other muxers for this purpose. These muxers allow only a single non-attached-pic stream, so there is no problem with interleavement at all. (Attached pics are currently somewhat broken: Such streams are counted among the interleaved streams, so that after the attached pics have been received and consumed the ordinary stream is only written if there are so many packets that the max_interleave_delta criterion kicks in, leading to completely unnecessary delay.) - Andreas
On Thu, Sep 30, 2021 at 1:54 PM Michael Niedermayer <michael@niedermayer.cc> wrote: > On Thu, Sep 30, 2021 at 12:36:17PM +0200, Paul B Mahol wrote: > > Signed-off-by: Paul B Mahol <onemda@gmail.com> > > --- > > libavformat/mux.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavformat/mux.c b/libavformat/mux.c > > index 2053a5636e..583328b123 100644 > > --- a/libavformat/mux.c > > +++ b/libavformat/mux.c > > @@ -949,7 +949,7 @@ int ff_interleave_packet_per_dts(AVFormatContext *s, > AVPacket *out, > > last_dts = av_rescale_q(last->pkt.dts, > > st->time_base, > > AV_TIME_BASE_Q); > > - delta_dts = FFMAX(delta_dts, last_dts - top_dts); > > + delta_dts = FFMAX(delta_dts, (top_dts == AV_NOPTS_VALUE || > last_dts == AV_NOPTS_VALUE) ? 0 : last_dts - top_dts); > > } > > In which case are packets without DTS interleaved by DTS ? > In many cases, for example few of them being exposed by FATE, and --enable-ftrapv See fate.ffmpeg.org > > thx > > [...] > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Dictatorship: All citizens are under surveillance, all their steps and > actions recorded, for the politicians to enforce control. > Democracy: All politicians are under surveillance, all their steps and > actions recorded, for the citizens to enforce control. > _______________________________________________ > 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". >
diff --git a/libavformat/mux.c b/libavformat/mux.c index 2053a5636e..583328b123 100644 --- a/libavformat/mux.c +++ b/libavformat/mux.c @@ -949,7 +949,7 @@ int ff_interleave_packet_per_dts(AVFormatContext *s, AVPacket *out, last_dts = av_rescale_q(last->pkt.dts, st->time_base, AV_TIME_BASE_Q); - delta_dts = FFMAX(delta_dts, last_dts - top_dts); + delta_dts = FFMAX(delta_dts, (top_dts == AV_NOPTS_VALUE || last_dts == AV_NOPTS_VALUE) ? 0 : last_dts - top_dts); } if (delta_dts > s->max_interleave_delta) {
Signed-off-by: Paul B Mahol <onemda@gmail.com> --- libavformat/mux.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)