[FFmpeg-devel] [PATCH 2/3] lavf/utils: add flag to discard timestamps on corrupted frames

[Rodger Combs]

> On Dec 9, 2017, at 12:19, Michael Niedermayer <michael at niedermayer.cc> wrote:
> 
> On Fri, Dec 08, 2017 at 10:34:39PM -0600, Rodger Combs wrote:
>> 
>> 
>>> On Dec 8, 2017, at 11:06, Michael Niedermayer <michael at niedermayer.cc> wrote:
>>> 
>>> On Thu, Dec 07, 2017 at 10:23:15PM -0600, Rodger Combs wrote:
>>>> ---
>>>> libavformat/avformat.h      | 1 +
>>>> libavformat/options_table.h | 1 +
>>>> libavformat/utils.c         | 8 ++++++++
>>>> 3 files changed, 10 insertions(+)
>>>> 
>>>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>>>> index 4f2798a871..d10d583dff 100644
>>>> --- a/libavformat/avformat.h
>>>> +++ b/libavformat/avformat.h
>>>> @@ -1450,6 +1450,7 @@ typedef struct AVFormatContext {
>>>> #define AVFMT_FLAG_FAST_SEEK   0x80000 ///< Enable fast, but inaccurate seeks for some formats
>>>> #define AVFMT_FLAG_SHORTEST   0x100000 ///< Stop muxing when the shortest stream stops.
>>>> #define AVFMT_FLAG_AUTO_BSF   0x200000 ///< Add bitstream filters as requested by the muxer
>>>> +#define AVFMT_FLAG_DISCARD_CORRUPT_TS 0x400000 ///< Discard timestamps of frames marked corrupt
>>>> 
>>>>    /**
>>>>     * Maximum size of the data read from input for determining
>>>> diff --git a/libavformat/options_table.h b/libavformat/options_table.h
>>>> index b8fa47c6fd..515574d3e0 100644
>>>> --- a/libavformat/options_table.h
>>>> +++ b/libavformat/options_table.h
>>>> @@ -58,6 +58,7 @@ static const AVOption avformat_options[] = {
>>>> {"bitexact", "do not write random/volatile data", 0, AV_OPT_TYPE_CONST, { .i64 = AVFMT_FLAG_BITEXACT }, 0, 0, E, "fflags" },
>>>> {"shortest", "stop muxing with the shortest stream", 0, AV_OPT_TYPE_CONST, { .i64 = AVFMT_FLAG_SHORTEST }, 0, 0, E, "fflags" },
>>>> {"autobsf", "add needed bsfs automatically", 0, AV_OPT_TYPE_CONST, { .i64 = AVFMT_FLAG_AUTO_BSF }, 0, 0, E, "fflags" },
>>>> +{"discardcorruptts", "discard timestamps on corrupted frames", 0, AV_OPT_TYPE_CONST, { .i64 = AVFMT_FLAG_DISCARD_CORRUPT_TS }, 0, 0, E, "fflags" },
>>>> {"analyzeduration", "specify how many microseconds are analyzed to probe the input", OFFSET(max_analyze_duration), AV_OPT_TYPE_INT64, {.i64 = 0 }, 0, INT64_MAX, D},
>>>> {"cryptokey", "decryption key", OFFSET(key), AV_OPT_TYPE_BINARY, {.dbl = 0}, 0, 0, D},
>>>> {"indexmem", "max memory used for timestamp index (per stream)", OFFSET(max_index_size), AV_OPT_TYPE_INT, {.i64 = 1<<20 }, 0, INT_MAX, D},
>>>> diff --git a/libavformat/utils.c b/libavformat/utils.c
>>>> index 84e49208b8..98af941e9f 100644
>>>> --- a/libavformat/utils.c
>>>> +++ b/libavformat/utils.c
>>> 
>>>> @@ -873,6 +873,14 @@ int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
>>>>                st->cur_dts = wrap_timestamp(st, st->cur_dts);
>>>>        }
>>>> 
>>>> +        if ((s->flags & AVFMT_FLAG_DISCARD_CORRUPT_TS) &&
>>>> +            (pkt->flags & AV_PKT_FLAG_CORRUPT)) {
>>>> +            pkt->pts = pkt->dts = AV_NOPTS_VALUE;
>>>> +            av_log(s, AV_LOG_WARNING,
>>>> +                   "Discarded timestamp on corrupted packet (stream = %d)\n",
>>>> +                   pkt->stream_index);
>>>> +        }
>>> 
>>> how many of the cases that set AV_PKT_FLAG_CORRUPT can even be due to
>>> the timestamp ?
>> 
>> Pretty much just the new TEI check, or potentially other CRC checks. I don't _think_ the continuity-counter check failing could indicate an incorrect timestamp. I suppose we could add a second flag just for corruption that could affect timestamps?
> 
> I think thats a great idea, maybe if we already change it we should
> also split the truncated case out as it is very common.
> 
> AV_PKT_FLAG_DATA_CORRUPT                // the packet data may be corrupted
> AV_PKT_FLAG_DATA_TRUNCATED              // the packet size may be corrupted, data available is undamaged unless another flag indicates otherwise. Any headers or sub-packets which are complete are non corrupted-
> AV_PKT_FLAG_TS_CORRUPT                  // the packets timestamps and or duration may be corrupted
> 
> #define AV_PKT_FLAG_CORRUPT (AV_PKT_FLAG_DATA_CORRUPT | AV_PKT_FLAG_DATA_TRUNCATED | AV_PKT_FLAG_TS_CORRUPT)

What kind of API bump would be needed to make this kind of change?

> 
> or some variation of this
> 
> Thanks
> 
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> There will always be a question for which you do not know the correct answer.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org <mailto:ffmpeg-devel at ffmpeg.org>
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>