diff mbox

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

Message ID 20171207093738.52358-2-rodger.combs@gmail.com
State Superseded
Headers show

Commit Message

Rodger Combs Dec. 7, 2017, 9:37 a.m. UTC
---
 libavformat/avformat.h      |  1 +
 libavformat/internal.h      |  5 +++++
 libavformat/options_table.h |  1 +
 libavformat/utils.c         | 12 ++++++++++++
 4 files changed, 19 insertions(+)

Comments

Michael Niedermayer Dec. 7, 2017, 8:46 p.m. UTC | #1
On Thu, Dec 07, 2017 at 03:37:38AM -0600, Rodger Combs wrote:
> ---
>  libavformat/avformat.h      |  1 +
>  libavformat/internal.h      |  5 +++++
>  libavformat/options_table.h |  1 +
>  libavformat/utils.c         | 12 ++++++++++++
>  4 files changed, 19 insertions(+)
> 
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 4f2798a871..e2d88280a8 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 (replacing with wallclock offset from last non-corrupt frame)

Using wallclock to fill in timestamps feels wrong
if you discard a timestamp it should be set to AV_NOPTS_VALUE or
recomputed based on information from the specification where this is
possible, fps when constant or other hard information.

[...]
Hendrik Leppkes Dec. 7, 2017, 10:53 p.m. UTC | #2
On Thu, Dec 7, 2017 at 9:46 PM, Michael Niedermayer
<michael@niedermayer.cc> wrote:
> On Thu, Dec 07, 2017 at 03:37:38AM -0600, Rodger Combs wrote:
>> ---
>>  libavformat/avformat.h      |  1 +
>>  libavformat/internal.h      |  5 +++++
>>  libavformat/options_table.h |  1 +
>>  libavformat/utils.c         | 12 ++++++++++++
>>  4 files changed, 19 insertions(+)
>>
>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>> index 4f2798a871..e2d88280a8 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 (replacing with wallclock offset from last non-corrupt frame)
>
> Using wallclock to fill in timestamps feels wrong
> if you discard a timestamp it should be set to AV_NOPTS_VALUE or
> recomputed based on information from the specification where this is
> possible, fps when constant or other hard information.
>

I agree, discard should be just that, set it to invalid. Wallclock is
meaningless on anything but a live stream.

- Hendrik
Rodger Combs Dec. 7, 2017, 10:57 p.m. UTC | #3
> On Dec 7, 2017, at 16:53, Hendrik Leppkes <h.leppkes@gmail.com> wrote:
> 
> On Thu, Dec 7, 2017 at 9:46 PM, Michael Niedermayer
> <michael@niedermayer.cc <mailto:michael@niedermayer.cc>> wrote:
>> On Thu, Dec 07, 2017 at 03:37:38AM -0600, Rodger Combs wrote:
>>> ---
>>> libavformat/avformat.h      |  1 +
>>> libavformat/internal.h      |  5 +++++
>>> libavformat/options_table.h |  1 +
>>> libavformat/utils.c         | 12 ++++++++++++
>>> 4 files changed, 19 insertions(+)
>>> 
>>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>>> index 4f2798a871..e2d88280a8 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 (replacing with wallclock offset from last non-corrupt frame)
>> 
>> Using wallclock to fill in timestamps feels wrong
>> if you discard a timestamp it should be set to AV_NOPTS_VALUE or
>> recomputed based on information from the specification where this is
>> possible, fps when constant or other hard information.
>> 
> 
> I agree, discard should be just that, set it to invalid. Wallclock is
> meaningless on anything but a live stream.

My intended use-case is live streams, where wallclock can be more useful than deriving from fps if e.g. entire packets are dropped. I could have one flag to unset timestamps on corrupt packets, and another to fill unset DTS from wallclock?

> 
> - Hendrik
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
diff mbox

Patch

diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 4f2798a871..e2d88280a8 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 (replacing with wallclock offset from last non-corrupt frame)
 
     /**
      * Maximum size of the data read from input for determining
diff --git a/libavformat/internal.h b/libavformat/internal.h
index 36a57214ce..39a69cf630 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -192,6 +192,11 @@  struct AVStreamInternal {
     int need_context_update;
 
     FFFrac *priv_pts;
+
+    /**
+     * The wallclock timestamp of the most recent read packet (if AVFMT_FLAG_DISCARD_CORRUPT_TS is set)
+     */
+    int64_t cur_wallclock_time;
 };
 
 #ifdef __GNUC__
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..1adb007244 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -873,6 +873,18 @@  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)) {
+            int64_t cur_wallclock_time = av_gettime_relative();
+            if (pkt->flags & AV_PKT_FLAG_CORRUPT) {
+                int64_t wallclock_offset = av_rescale_q(st->internal->cur_wallclock_time - cur_wallclock_time, AV_TIME_BASE_Q, st->time_base);
+                pkt->pts = pkt->dts = st->cur_dts + FFMAX(wallclock_offset, 1);
+                av_log(s, AV_LOG_WARNING,
+                       "Discarded timestamp on corrupted packet (stream = %d)\n",
+                       pkt->stream_index);
+            }
+            st->internal->cur_wallclock_time = cur_wallclock_time;
+        }
+
         pkt->dts = wrap_timestamp(st, pkt->dts);
         pkt->pts = wrap_timestamp(st, pkt->pts);