| Message ID | 49386de7-50a4-fcc5-f956-5a3542a3d0ef@googlemail.com |
|---|---|
| State | Accepted |
| Headers | show |
On Tue, Nov 08, 2016 at 09:38:49PM +0100, Andreas Cadhalpun wrote: > On 08.11.2016 21:09, Michael Niedermayer wrote: > > On Tue, Nov 08, 2016 at 07:47:02PM +0100, Andreas Cadhalpun wrote: > >> On 08.11.2016 00:54, Michael Niedermayer wrote: > >>> On Mon, Nov 07, 2016 at 11:49:52PM +0100, Andreas Cadhalpun wrote: > >>>> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> > >>>> --- > >>>> libavformat/mpegts.c | 4 ++++ > >>>> 1 file changed, 4 insertions(+) > >>>> > >>>> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c > >>>> index fad10c6..77d63f2 100644 > >>>> --- a/libavformat/mpegts.c > >>>> +++ b/libavformat/mpegts.c > >>>> @@ -2692,6 +2692,10 @@ static int mpegts_read_header(AVFormatContext *s) > >>>> /* NOTE1: the bitrate is computed without the FEC */ > >>>> /* NOTE2: it is only the bitrate of the start of the stream */ > >>>> ts->pcr_incr = (pcrs[1] - pcrs[0]) / (packet_count[1] - packet_count[0]); > >>>> + if (ts->pcr_incr <= 0) { > >>>> + av_log(s, AV_LOG_ERROR, "invalid pcr increment %d\n", ts->pcr_incr); > >>>> + return AVERROR_INVALIDDATA; > >>>> + } > >>> > >>> if a pcr pair is bad i would suggest to run the loop by another > >>> iteration > >> > >> That's a good idea. New patch attached. > > > > LGTM, maybe add a av_log() so the user knows of the issue > > Makes sense, patch with added log message is attached. > > > (especially > > if all pcr are systematically bad, n which case this would need to be > > adjusted to not fail) > > What do you think should be done in that case? if you want to implement that case before theres a file found that needs it, ignore the pcr, set things that depend on them to "unknown" / AV_NOPTS_VALUE [...]
On 08.11.2016 22:12, Michael Niedermayer wrote: > On Tue, Nov 08, 2016 at 09:38:49PM +0100, Andreas Cadhalpun wrote: >> On 08.11.2016 21:09, Michael Niedermayer wrote: >>> On Tue, Nov 08, 2016 at 07:47:02PM +0100, Andreas Cadhalpun wrote: >>>> On 08.11.2016 00:54, Michael Niedermayer wrote: >>>>> On Mon, Nov 07, 2016 at 11:49:52PM +0100, Andreas Cadhalpun wrote: >>>>>> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> >>>>>> --- >>>>>> libavformat/mpegts.c | 4 ++++ >>>>>> 1 file changed, 4 insertions(+) >>>>>> >>>>>> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c >>>>>> index fad10c6..77d63f2 100644 >>>>>> --- a/libavformat/mpegts.c >>>>>> +++ b/libavformat/mpegts.c >>>>>> @@ -2692,6 +2692,10 @@ static int mpegts_read_header(AVFormatContext *s) >>>>>> /* NOTE1: the bitrate is computed without the FEC */ >>>>>> /* NOTE2: it is only the bitrate of the start of the stream */ >>>>>> ts->pcr_incr = (pcrs[1] - pcrs[0]) / (packet_count[1] - packet_count[0]); >>>>>> + if (ts->pcr_incr <= 0) { >>>>>> + av_log(s, AV_LOG_ERROR, "invalid pcr increment %d\n", ts->pcr_incr); >>>>>> + return AVERROR_INVALIDDATA; >>>>>> + } >>>>> >>>>> if a pcr pair is bad i would suggest to run the loop by another >>>>> iteration >>>> >>>> That's a good idea. New patch attached. >>> >>> LGTM, maybe add a av_log() so the user knows of the issue >> >> Makes sense, patch with added log message is attached. >> >>> (especially >>> if all pcr are systematically bad, n which case this would need to be >>> adjusted to not fail) >> >> What do you think should be done in that case? > > if you want to implement that case before theres a file found that > needs it, > ignore the pcr, set things that depend on them to "unknown" / > AV_NOPTS_VALUE That wouldn't be trivial to implement so I've just pushed the patch as is. Best regards, Andreas
From 02fc2ecef2adb20c3be63a6787c6921f8471ca04 Mon Sep 17 00:00:00 2001 From: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> Date: Mon, 7 Nov 2016 23:37:59 +0100 Subject: [PATCH] mpegts: prevent division by zero Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> --- libavformat/mpegts.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c index fad10c6..0aa0ad7 100644 --- a/libavformat/mpegts.c +++ b/libavformat/mpegts.c @@ -2681,8 +2681,17 @@ static int mpegts_read_header(AVFormatContext *s) packet_count[nb_pcrs] = nb_packets; pcrs[nb_pcrs] = pcr_h * 300 + pcr_l; nb_pcrs++; - if (nb_pcrs >= 2) - break; + if (nb_pcrs >= 2) { + if (pcrs[1] - pcrs[0] > 0) { + /* the difference needs to be positive to make sense for bitrate computation */ + break; + } else { + av_log(ts->stream, AV_LOG_WARNING, "invalid pcr pair %"PRId64" >= %"PRId64"\n", pcrs[0], pcrs[1]); + pcrs[0] = pcrs[1]; + packet_count[0] = packet_count[1]; + nb_pcrs--; + } + } } else { finished_reading_packet(s, ts->raw_packet_size); } -- 2.10.2