Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
To: ffmpeg-devel@ffmpeg.org
Cc: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Subject: [FFmpeg-devel] [PATCH v2 4/6] avformat/mux: Preserve sync even if later packet has negative ts
Date: Wed, 19 Jan 2022 22:29:38 +0100
Message-ID: <AM7PR03MB6660230AFC285D5F32E069798F599@AM7PR03MB6660.eurprd03.prod.outlook.com> (raw)
In-Reply-To: <20220119212940.1071477-1-andreas.rheinhardt@outlook.com>

write_packet() has code to shift the packets timestamps
to make them nonnegative or even make them start at ts zero;
this code inspects every packet that is written and if a packet
with negative timestamp (whether this is dts or pts depends upon
another flag; basically: Matroska uses pts, everyone else dts)
is encountered, this is offset to make the timestamp zero.
All further packets will be offset accordingly (with the offset
converted according to the streams' timebases).

This is based around an assumption, namely that the timestamps
are indeed non-decreasing, so that the first packet with negative
timestamps is the first packet with timestamps. This assumption
is often fulfilled given that the default interleavement function
by default interleaves per dts; yet there are scenarios in which
it may not be fulfilled:
a) av_write_frame() instead of av_interleaved_write_frame() is used.
b) The audio_preload option is used.
c) When the timestamps that are made nonnegative/zero are pts
(i.e. with Matroska), because the packet with the smallest dts
is not necessarily the packet with the smallest pts.
d) Possibly with custom interleavement functions.
In these cases the relative sync of the first few packet(s) is offset
relative to the later packets. This contradicts the documentation
("When shifting is enabled, all output timestamps are shifted by
the same amount").

Therefore this commit changes this: As soon as the first packet
with valid timestamps is output, it is checked and recorded whether
the timestamps need to be shifted. Further packets are no longer
checked for needing to be offset; instead they are simply offset.
In the cases above this leads to packets with negative timestamps
(and the appropriate warnings) instead of desync. This will mostly
be fixed in the next commit.

This commit also factors handling the avoid_negative_ts stuff out
of write_packet() in order to be able to return immediately.

Tickets #4536 and #5784 as well as the matroska-avoid-negative-ts-test
are examples of c); as has been said, some timestamps are now negative,
yet the ref file update does not show it because ffmpeg.c sanitizes
the timestamps (-copyts disables it; ffprobe and mkvinfo also show
the original timestamps).

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavformat/internal.h                    |  23 +++--
 libavformat/mux.c                         | 111 +++++++++++++---------
 libavformat/options.c                     |   1 -
 tests/fate/matroska.mak                   |   4 +-
 tests/ref/fate/matroska-avoid-negative-ts |   6 +-
 5 files changed, 81 insertions(+), 64 deletions(-)

diff --git a/libavformat/internal.h b/libavformat/internal.h
index bffb8e66ff..f24c68703f 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -82,6 +82,17 @@ typedef struct FFFormatContext {
      */
     int nb_interleaved_streams;
 
+    /**
+     * Whether the timestamp shift offset has already been determined.
+     * -1: disabled, 0: not yet determined, 1: determined.
+     */
+    enum {
+        AVOID_NEGATIVE_TS_DISABLED = -1,
+        AVOID_NEGATIVE_TS_UNKNOWN  = 0,
+        AVOID_NEGATIVE_TS_KNOWN    = 1,
+    } avoid_negative_ts_status;
+#define AVOID_NEGATIVE_TS_ENABLED(status) ((status) >= 0)
+
     /**
      * The interleavement function in use. Always set for muxers.
      */
@@ -135,18 +146,6 @@ typedef struct FFFormatContext {
      */
     int raw_packet_buffer_size;
 
-    /**
-     * Offset to remap timestamps to be non-negative.
-     * Expressed in timebase units.
-     * @see AVStream.mux_ts_offset
-     */
-    int64_t offset;
-
-    /**
-     * Timebase for the timestamp offset.
-     */
-    AVRational offset_timebase;
-
 #if FF_API_COMPUTE_PKT_FIELDS2
     int missing_ts_warning;
 #endif
diff --git a/libavformat/mux.c b/libavformat/mux.c
index a1917878a5..0810b674a7 100644
--- a/libavformat/mux.c
+++ b/libavformat/mux.c
@@ -388,6 +388,8 @@ fail:
 
 static int init_pts(AVFormatContext *s)
 {
+    FFFormatContext *const si = ffformatcontext(s);
+
     /* init PTS generation */
     for (unsigned i = 0; i < s->nb_streams; i++) {
         AVStream *const st = s->streams[i];
@@ -418,13 +420,16 @@ static int init_pts(AVFormatContext *s)
         }
     }
 
+    si->avoid_negative_ts_status = AVOID_NEGATIVE_TS_UNKNOWN;
     if (s->avoid_negative_ts < 0) {
         av_assert2(s->avoid_negative_ts == AVFMT_AVOID_NEG_TS_AUTO);
         if (s->oformat->flags & (AVFMT_TS_NEGATIVE | AVFMT_NOTIMESTAMPS)) {
             s->avoid_negative_ts = AVFMT_AVOID_NEG_TS_DISABLED;
+            si->avoid_negative_ts_status = AVOID_NEGATIVE_TS_DISABLED;
         } else
             s->avoid_negative_ts = AVFMT_AVOID_NEG_TS_MAKE_NON_NEGATIVE;
-    }
+    } else if (s->avoid_negative_ts == AVFMT_AVOID_NEG_TS_DISABLED)
+        si->avoid_negative_ts_status = AVOID_NEGATIVE_TS_DISABLED;
 
     return 0;
 }
@@ -638,6 +643,64 @@ static void guess_pkt_duration(AVFormatContext *s, AVStream *st, AVPacket *pkt)
     }
 }
 
+static void handle_avoid_negative_ts(FFFormatContext *si, FFStream *sti,
+                                     AVPacket *pkt)
+{
+    AVFormatContext *const s = &si->pub;
+    int64_t offset;
+
+    if (!AVOID_NEGATIVE_TS_ENABLED(si->avoid_negative_ts_status))
+        return;
+
+    if (si->avoid_negative_ts_status == AVOID_NEGATIVE_TS_UNKNOWN) {
+        int use_pts = si->avoid_negative_ts_use_pts;
+        int64_t ts = use_pts ? pkt->pts : pkt->dts;
+
+        if (ts == AV_NOPTS_VALUE)
+            return;
+        if (ts < 0 ||
+            ts > 0 && s->avoid_negative_ts == AVFMT_AVOID_NEG_TS_MAKE_ZERO) {
+            for (unsigned i = 0; i < s->nb_streams; i++) {
+                AVStream *const st2  = s->streams[i];
+                FFStream *const sti2 = ffstream(st2);
+                sti2->mux_ts_offset = av_rescale_q_rnd(-ts,
+                                                       sti->pub.time_base,
+                                                       st2->time_base,
+                                                       AV_ROUND_UP);
+            }
+        }
+        si->avoid_negative_ts_status = AVOID_NEGATIVE_TS_KNOWN;
+    }
+
+    offset = sti->mux_ts_offset;
+
+    if (pkt->dts != AV_NOPTS_VALUE)
+        pkt->dts += offset;
+    if (pkt->pts != AV_NOPTS_VALUE)
+        pkt->pts += offset;
+
+    if (si->avoid_negative_ts_use_pts) {
+        if (pkt->pts != AV_NOPTS_VALUE && pkt->pts < 0) {
+            av_log(s, AV_LOG_WARNING, "failed to avoid negative "
+                   "pts %s in stream %d.\n"
+                   "Try -avoid_negative_ts 1 as a possible workaround.\n",
+                   av_ts2str(pkt->pts),
+                   pkt->stream_index
+            );
+        }
+    } else {
+        if (pkt->dts != AV_NOPTS_VALUE && pkt->dts < 0) {
+            av_log(s, AV_LOG_WARNING,
+                   "Packets poorly interleaved, failed to avoid negative "
+                   "timestamp %s in stream %d.\n"
+                   "Try -max_interleave_delta 0 as a possible workaround.\n",
+                   av_ts2str(pkt->dts),
+                   pkt->stream_index
+            );
+        }
+    }
+}
+
 /**
  * Shift timestamps and call muxer; the original pts/dts are not kept.
  *
@@ -663,51 +726,7 @@ static int write_packet(AVFormatContext *s, AVPacket *pkt)
         if (pkt->pts != AV_NOPTS_VALUE)
             pkt->pts += offset;
     }
-
-    if (s->avoid_negative_ts > 0) {
-        int64_t offset = sti->mux_ts_offset;
-        int64_t ts = si->avoid_negative_ts_use_pts ? pkt->pts : pkt->dts;
-
-        if (si->offset == AV_NOPTS_VALUE && ts != AV_NOPTS_VALUE &&
-            (ts < 0 || s->avoid_negative_ts == AVFMT_AVOID_NEG_TS_MAKE_ZERO)) {
-            si->offset = -ts;
-            si->offset_timebase = st->time_base;
-        }
-
-        if (si->offset != AV_NOPTS_VALUE && !offset) {
-            offset = sti->mux_ts_offset =
-                av_rescale_q_rnd(si->offset,
-                                 si->offset_timebase,
-                                 st->time_base,
-                                 AV_ROUND_UP);
-        }
-
-        if (pkt->dts != AV_NOPTS_VALUE)
-            pkt->dts += offset;
-        if (pkt->pts != AV_NOPTS_VALUE)
-            pkt->pts += offset;
-
-        if (si->avoid_negative_ts_use_pts) {
-            if (pkt->pts != AV_NOPTS_VALUE && pkt->pts < 0) {
-                av_log(s, AV_LOG_WARNING, "failed to avoid negative "
-                    "pts %s in stream %d.\n"
-                    "Try -avoid_negative_ts 1 as a possible workaround.\n",
-                    av_ts2str(pkt->pts),
-                    pkt->stream_index
-                );
-            }
-        } else {
-            if (pkt->dts != AV_NOPTS_VALUE && pkt->dts < 0) {
-                av_log(s, AV_LOG_WARNING,
-                    "Packets poorly interleaved, failed to avoid negative "
-                    "timestamp %s in stream %d.\n"
-                    "Try -max_interleave_delta 0 as a possible workaround.\n",
-                    av_ts2str(pkt->dts),
-                    pkt->stream_index
-                );
-            }
-        }
-    }
+    handle_avoid_negative_ts(si, sti, pkt);
 
     if ((pkt->flags & AV_PKT_FLAG_UNCODED_FRAME)) {
         AVFrame **frame = (AVFrame **)pkt->data;
diff --git a/libavformat/options.c b/libavformat/options.c
index 1634388acb..2d55d3ad6e 100644
--- a/libavformat/options.c
+++ b/libavformat/options.c
@@ -174,7 +174,6 @@ AVFormatContext *avformat_alloc_context(void)
         return NULL;
     }
 
-    si->offset = AV_NOPTS_VALUE;
     si->shortest_end = AV_NOPTS_VALUE;
 
     return s;
diff --git a/tests/fate/matroska.mak b/tests/fate/matroska.mak
index b65a76411b..da1fdbd5ea 100644
--- a/tests/fate/matroska.mak
+++ b/tests/fate/matroska.mak
@@ -94,8 +94,8 @@ fate-matroska-dovi-write-config7: CMD = transcode mov $(TARGET_SAMPLES)/mov/dovi
 # the first packet (with the overall lowest dts) is a video packet,
 # whereas an audio packet to be muxed later has the overall lowest pts
 # which happens to be negative and therefore needs to be shifted.
-# This is currently buggy (the timestamps of the video frames muxed
-# before the first audio frame are not shifted).
+# This is currently buggy (the timestamps are not shifted properly:
+# the first audio packet has negative pts).
 # (-ss 1.09 ensures that a video frame has the lowest dts of all packets;
 # yet there is an audio packet with the overall lowest pts. output_ts_offset
 # makes the pts of the audio packet, but not the leading video packet negative
diff --git a/tests/ref/fate/matroska-avoid-negative-ts b/tests/ref/fate/matroska-avoid-negative-ts
index a687c8f63c..1b9b2f2786 100644
--- a/tests/ref/fate/matroska-avoid-negative-ts
+++ b/tests/ref/fate/matroska-avoid-negative-ts
@@ -1,4 +1,4 @@
-3349536550047c5c553215003ba2acb7 *tests/data/fate/matroska-avoid-negative-ts.matroska
+90cf5a330659140d47ec11208f525908 *tests/data/fate/matroska-avoid-negative-ts.matroska
 973070 tests/data/fate/matroska-avoid-negative-ts.matroska
 #extradata 0:       22, 0x2885037c
 #tb 0: 1/1000
@@ -12,11 +12,11 @@
 #sample_rate 1: 44100
 #channel_layout 1: 4
 #channel_layout_name 1: mono
-0,        -37,         24,       40,     9156, 0xe5bd034a, S=1,       40
+0,        -37,         43,       40,     9156, 0xe5bd034a, S=1,       40
 1,          0,          0,       26,      417, 0x7198c15e
 0,          3,          3,       40,     1740, 0x29ac4480, F=0x0
-0,         24,        123,       40,     3672, 0x98652013, F=0x0
 1,         26,         26,       26,      417, 0x3c67c32d
+0,         43,        123,       40,     3672, 0x98652013, F=0x0
 1,         52,         52,       26,      417, 0x8c24b1ca
 1,         78,         78,       26,      417, 0x6ee576b7
 0,         83,         83,       40,     2532, 0xa2c42769, F=0x0
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

  parent reply	other threads:[~2022-01-19 21:30 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-19 21:19 [FFmpeg-devel] [PATCH v2 1/6] avformat/mux: Remove assert based on faulty assumptions Andreas Rheinhardt
2022-01-19 21:29 ` [FFmpeg-devel] [PATCH v2 2/6] fate/matroska: Add test for avoiding negative timestamps Andreas Rheinhardt
     [not found] ` <20220119212940.1071477-1-andreas.rheinhardt@outlook.com>
2022-01-19 21:29   ` [FFmpeg-devel] [PATCH v2 3/6] avformat/avformat: Add AVFMT_AVOID_NEG_TS_DISABLED Andreas Rheinhardt
2022-01-19 21:29   ` Andreas Rheinhardt [this message]
2022-01-19 21:29   ` [FFmpeg-devel] [PATCH v2 5/6] avformat/mux: Peek into the muxing queue for avoid_negative_ts Andreas Rheinhardt
2022-01-19 21:29   ` [FFmpeg-devel] [PATCH v2 6/6] avformat/hls: Remove redundant cast Andreas Rheinhardt
2022-01-21 12:14 ` [FFmpeg-devel] [PATCH v2 1/6] avformat/mux: Remove assert based on faulty assumptions Andreas Rheinhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AM7PR03MB6660230AFC285D5F32E069798F599@AM7PR03MB6660.eurprd03.prod.outlook.com \
    --to=andreas.rheinhardt@outlook.com \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git