diff mbox

[FFmpeg-devel] mpegts: prevent division by zero

Message ID 0cf22af6-5e88-8e78-1229-22b04824874a@googlemail.com
State Superseded
Headers show

Commit Message

Andreas Cadhalpun Nov. 8, 2016, 6:47 p.m. UTC 
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.

Best regards,
Andreas

Comments

Michael Niedermayer Nov. 8, 2016, 8:09 p.m. UTC | #1 
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 (especially
if all pcr are systematically bad, n which case this would need to be
adjusted to not fail)

thx

[...]
diff mbox

Patch

From 9d7a34dbdd9a5b97c9b51c47f918df7237b42fa6 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 | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
index fad10c6..048073b 100644
--- a/libavformat/mpegts.c
+++ b/libavformat/mpegts.c
@@ -2681,8 +2681,16 @@  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 {
+                        pcrs[0] = pcrs[1];
+                        packet_count[0] = packet_count[1];
+                        nb_pcrs--;
+                    }
+                }
             } else {
                 finished_reading_packet(s, ts->raw_packet_size);
             }
-- 
2.10.2