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

[Michael Niedermayer]

On Sun, Dec 10, 2017 at 05:17:44AM -0600, Rodger Combs wrote:
> 
> 
> > 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?

IIUC we are still within the major bump thing (didnt check but others
have said it in relation to other changes)

a minor bump should probably be done anyway to signal that these
additional flag may be set in the output


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Complexity theory is the science of finding the exact solution to an
approximation. Benchmarking OTOH is finding an approximation of the exact
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20171210/10b2779c/attachment.sig>