[FFmpeg-devel] [PATCH] Wrong duration in TS container (Ticket #1836)

Hendrik Leppkes h.leppkes at gmail.com
Tue Oct 30 11:35:27 CET 2012


On Tue, Oct 30, 2012 at 11:17 AM, Steven Liu <lingjiujianke at gmail.com> wrote:
> 2012/10/30 Heesuk Jung <heesuk.jung at lge.com>:
>> Hi,
>>
>> Michael wrote:
>>> Ive tested the patch a bit and it breaks finding the correct duration of:
>>> https://ffmpeg.org/trac/ffmpeg/raw-attachment/ticket/1186/sync1.ogg
>>
>> I update patches and did regression test for 472 files include "sync1.ogg".
>> (I compared extracted duration value with master version and patch applied
>> master.)
>>
>> Test result is ok and duration of 4 files with patch is more improved than
>> original duration.
>>
>> Thanks !
>>
>> From 6af199fbd89ecc9441ce84789e4f1a6800128670 Mon Sep 17 00:00:00 2001
>> From: Heesuk Jung <heesuk.jung at lge.com>
>> Date: Tue, 30 Oct 2012 18:29:47 +0900
>> Subject: [PATCH] Wrong duration in TS container (Ticket #1836)
>>
>> Libavformat somtimes get wrong duration in some TS conatiner.
>> Please refer the problem description and file link in Ticket #1836.
>> (http://ffmpeg.org/trac/ffmpeg/ticket/1836)
>>
>> I have just modified 2 points as below.
>>
>> 1. check if duration estimation is reasonable or not.
>>
>> examine if estimated duration is valid based on bit rate information.
>> The duration is regarded as abnormal value if it is unreasonably bigger
>> than file size / average bit rate.
>>
>> 2. check if PTS time diff is reasonable or not.
>>
>> Duration is determined comparing PTS time diff with estimated duration.
>> (Actually final duration is selected as bigger value in PTS time diff and
>> estimated duration) I suggest that don't beleive duration based on PTS time
>> diff if time diff is bigger than hueristic value (file size / average bit
>> rate * 2).
>> ---
>>  libavformat/utils.c |   87
>> +++++++++++++++++++++++++++++++++++++++++++++++++--
>>  1 files changed, 84 insertions(+), 3 deletions(-)
>>  mode change 100644 => 100755 ffprobe.c
>>  mode change 100644 => 100755 libavformat/utils.c
>>
>> diff --git a/ffprobe.c b/ffprobe.c
>> old mode 100644
>> new mode 100755
>> diff --git a/libavformat/utils.c b/libavformat/utils.c
>> old mode 100644
>> new mode 100755
>> index fb5742b..cf76cc1
>> --- a/libavformat/utils.c
>> +++ b/libavformat/utils.c
>> @@ -2012,6 +2012,68 @@ int avformat_seek_file(AVFormatContext *s, int
>> stream_index, int64_t min_ts, int
>>      // try some generic seek like seek_frame_generic() but with new ts
>> semantics
>>  }
>>
>> +/**
>> + * examine if duration is valid based on bit rate information or not.
>> + * Duration is regarded as invlaid value
>> + * if duration is unreasonably bigger than file size/average bit rate.
>> + * hueristically suggest  limit_filesize_multiple that means double file
>> size
>> + * @return 1 if duration is valid.
>> + *         0 if duration is not valid.
>> + */
>> +static int check_duration_using_bit_rate(const int64_t duration,
>> +                                         const int64_t filesize, int const
>> bit_rate)
>> +{
>> +    const uint8_t limit_filesize_multiple = 3;
>> +
>> +    if (bit_rate > 0 &&
>> +        duration >
>> (filesize*8/(bit_rate/1000)*limit_filesize_multiple*1000)) {
>> +        return 0;
>> +    }
>> +    return 1;
>> +}
>> +
>> +/**
>> + * examine if estimated duration is valid or not.
>> + * Estimated duration is regarded as invalid value
>> + * if duration is unreasonably bigger than file size/average bit rate.
>> + * @return 1 if duration estimeation is valid.
>> + *         0 if duration estimation is not valid.
>> + */
>> +static int valid_duration_estimation(AVFormatContext *ic)
>> +{
>> +    const int64_t duration = ic->duration;
>> +    const int64_t filesize = avio_size(ic->pb);
>> +    const uint8_t limit_bitrate_multiple = 10;
>> +    uint8_t i = 0;
>> +    int bit_rate = 0;
>> +    AVStream *st;
>> +
>> +    for (i=0; i<ic->nb_streams; i++) {
>> +        st = ic->streams[i];
>> +        if (st->codec->bit_rate > 0) {
>> +            switch (st->codec->codec_type) {
>> +            case AVMEDIA_TYPE_VIDEO:
>> +                bit_rate += st->codec->bit_rate;
>> +                break;
>> +            case AVMEDIA_TYPE_AUDIO:
>> +                bit_rate += st->codec->bit_rate;
>> +                break;
>> +            }
>> +           }
>> +    }
>> +    if (ic->bit_rate > 0 &&
>> +        bit_rate > ic->bit_rate*limit_bitrate_multiple) {
>> +        bit_rate = ic->bit_rate;
>> +    }
>> +
>> +       if (duration == AV_NOPTS_VALUE ||
>> +           !check_duration_using_bit_rate(duration, filesize, bit_rate)) {
>> +           ic->bit_rate = bit_rate;
>> +           return 0;
>> +    }
>> +       return 1;
>> +}
>> +
>>  /*******************************************************/
>>
>>  /**
>> @@ -2089,11 +2151,16 @@ static void update_stream_timings(AVFormatContext
>> *ic)
>>              if (ic->nb_programs) {
>>                  for (i=0; i<ic->nb_programs; i++) {
>>                      p = ic->programs[i];
>> -                    if(p->start_time != AV_NOPTS_VALUE && p->end_time >
>> p->start_time)
>> -                        duration = FFMAX(duration, p->end_time -
>> p->start_time);
>> +                    if(p->start_time != AV_NOPTS_VALUE && p->end_time >
>> p->start_time) {
>> +                        if (check_duration_using_bit_rate(p->end_time -
>> p->start_time,
>> +
>> avio_size(ic->pb), ic->bit_rate)) {
>> +                            duration = FFMAX(duration, p->end_time -
>> p->start_time);
>> +                        }
>> +                    }
>>                  }
>> -            } else
>> +            } else {
>>                  duration = FFMAX(duration, end_time - start_time);
>> +            }
>>          }
>>      }
>>      if (duration != INT64_MIN && duration > 0 && ic->duration ==
>> AV_NOPTS_VALUE) {
>> @@ -2265,6 +2332,20 @@ static void estimate_timings(AVFormatContext *ic,
>> int64_t old_offset)
>>          estimate_timings_from_bit_rate(ic);
>>          ic->duration_estimation_method = AVFMT_DURATION_FROM_BITRATE;
>>      }
>> +    if ((ic->duration_estimation_method != AVFMT_DURATION_FROM_BITRATE) &&
>> +        !valid_duration_estimation(ic)) {
>> +        uint8_t i = 0;
>> +        AVStream *st;
>> +        ic->duration = AV_NOPTS_VALUE;
>> +        for (i=0; i<ic->nb_streams; i++) {
>> +            st = ic->streams[i];
>> +            st->duration = AV_NOPTS_VALUE;
>> +        }
>> +        av_log(ic, AV_LOG_WARNING, "Estimating duration from bitrate, this
>> may be inaccurate\n");
>> +        /* less precise: use bitrate info */
>> +        estimate_timings_from_bit_rate(ic);
>> +        ic->duration_estimation_method = AVFMT_DURATION_FROM_BITRATE;
>> +    }
>>      update_stream_timings(ic);
>>
>>      {
>> --
>> 1.7.0.4
>
>
> I Have tried your patch , problem is always.
>
> Program terminated with signal 8, Arithmetic exception.

The way the calculation is written, it will always result in a
division by zero if bitrate is < 10000.
I would recommend to re-order the calcuation into this form:

(filesize * 8) / bitrate * limit_filesize_multiple * 1000000

As an alternative, move one factor 1000 into the filesize
multiplication, to preserve a bit more accuracy in these integer
division.

Personally, i think this check can also easily go wrong. What if only
the audio stream in a file has a determined bitrate, and the video
does not?
It would trigger this, and discard a possibly perfectly valid duration.

- Hendrik


More information about the ffmpeg-devel mailing list