[FFmpeg-devel,06/10] Revert "avformat/mux: stop delaying writing the header"

Submitted by Rodger Combs on March 14, 2018, 6:24 a.m.

Details

Message ID 20180314062445.89909-6-rodger.combs@gmail.com
State New
Headers show

Commit Message

Rodger Combs March 14, 2018, 6:24 a.m.
This reverts commit d6d605eb05c3ca32f591016c345eb2ad9e81c554.
---
 libavformat/avformat.h         |  2 +-
 libavformat/internal.h         |  6 +++++
 libavformat/mux.c              | 52 ++++++++++++++++++++++++++++++++++--------
 libavformat/options_table.h    |  2 +-
 libavformat/tests/fifo_muxer.c | 52 ++++++++++++++++++++++++++++++++++++++++++
 tests/ref/fate/fifo-muxer-tst  |  1 +
 tests/ref/fate/rgb24-mkv       |  4 ++--
 7 files changed, 105 insertions(+), 14 deletions(-)

Comments

Michael Niedermayer March 15, 2018, 10:12 p.m.
On Wed, Mar 14, 2018 at 01:24:41AM -0500, Rodger Combs wrote:
> This reverts commit d6d605eb05c3ca32f591016c345eb2ad9e81c554.
> ---

The commit message should explain why this change is done

and as this causes fate changes it should explain what changes and why

thx

[...]

Patch hide | download patch | download mbox

diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index a2fe7c6bb2..9e87d6cdac 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -1482,7 +1482,7 @@  typedef struct AVFormatContext {
 #endif
 #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_AUTO_BSF   0x200000 ///< Wait for packet data before writing a header, and add bitstream filters as requested by the muxer
 
     /**
      * Maximum size of the data read from input for determining
diff --git a/libavformat/internal.h b/libavformat/internal.h
index a020b1b417..666e2054a7 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -120,6 +120,12 @@  struct AVFormatInternal {
 
     int avoid_negative_ts_use_pts;
 
+    /**
+     * Whether or not a header has already been written
+     */
+    int header_written;
+    int write_header_ret;
+
     /**
      * Timestamp of the end of the shortest stream.
      */
diff --git a/libavformat/mux.c b/libavformat/mux.c
index a13f0e3a1b..5fdc9275cc 100644
--- a/libavformat/mux.c
+++ b/libavformat/mux.c
@@ -485,6 +485,25 @@  static void flush_if_needed(AVFormatContext *s)
     }
 }
 
+static int write_header_internal(AVFormatContext *s)
+{
+    if (!(s->oformat->flags & AVFMT_NOFILE) && s->pb)
+        avio_write_marker(s->pb, AV_NOPTS_VALUE, AVIO_DATA_MARKER_HEADER);
+    if (s->oformat->write_header) {
+        int ret = s->oformat->write_header(s);
+        if (ret >= 0 && s->pb && s->pb->error < 0)
+            ret = s->pb->error;
+        s->internal->write_header_ret = ret;
+        if (ret < 0)
+            return ret;
+        flush_if_needed(s);
+    }
+    s->internal->header_written = 1;
+    if (!(s->oformat->flags & AVFMT_NOFILE) && s->pb)
+        avio_write_marker(s->pb, AV_NOPTS_VALUE, AVIO_DATA_MARKER_UNKNOWN);
+    return 0;
+}
+
 int avformat_init_output(AVFormatContext *s, AVDictionary **options)
 {
     int ret = 0;
@@ -515,18 +534,11 @@  int avformat_write_header(AVFormatContext *s, AVDictionary **options)
         if ((ret = avformat_init_output(s, options)) < 0)
             return ret;
 
-    if (!(s->oformat->flags & AVFMT_NOFILE) && s->pb)
-        avio_write_marker(s->pb, AV_NOPTS_VALUE, AVIO_DATA_MARKER_HEADER);
-    if (s->oformat->write_header) {
-        ret = s->oformat->write_header(s);
-        if (ret >= 0 && s->pb && s->pb->error < 0)
-            ret = s->pb->error;
+    if (!(s->oformat->check_bitstream && s->flags & AVFMT_FLAG_AUTO_BSF)) {
+        ret = write_header_internal(s);
         if (ret < 0)
             goto fail;
-        flush_if_needed(s);
     }
-    if (!(s->oformat->flags & AVFMT_NOFILE) && s->pb)
-        avio_write_marker(s->pb, AV_NOPTS_VALUE, AVIO_DATA_MARKER_UNKNOWN);
 
     if (!s->internal->streams_initialized) {
         if ((ret = init_pts(s)) < 0)
@@ -738,6 +750,12 @@  static int write_packet(AVFormatContext *s, AVPacket *pkt)
         }
     }
 
+    if (!s->internal->header_written) {
+        ret = s->internal->write_header_ret ? s->internal->write_header_ret : write_header_internal(s);
+        if (ret < 0)
+            goto fail;
+    }
+
     if ((pkt->flags & AV_PKT_FLAG_UNCODED_FRAME)) {
         AVFrame *frame = (AVFrame *)pkt->data;
         av_assert0(pkt->size == UNCODED_FRAME_PACKET_SIZE);
@@ -753,6 +771,8 @@  static int write_packet(AVFormatContext *s, AVPacket *pkt)
             ret = s->pb->error;
     }
 
+fail:
+
     if (ret < 0) {
         pkt->pts = pts_backup;
         pkt->dts = dts_backup;
@@ -885,6 +905,11 @@  int av_write_frame(AVFormatContext *s, AVPacket *pkt)
 
     if (!pkt) {
         if (s->oformat->flags & AVFMT_ALLOW_FLUSH) {
+            if (!s->internal->header_written) {
+                ret = s->internal->write_header_ret ? s->internal->write_header_ret : write_header_internal(s);
+                if (ret < 0)
+                    return ret;
+            }
             ret = s->oformat->write_packet(s, NULL);
             flush_if_needed(s);
             if (ret >= 0 && s->pb && s->pb->error < 0)
@@ -1268,8 +1293,14 @@  int av_write_trailer(AVFormatContext *s)
             goto fail;
     }
 
+    if (!s->internal->header_written) {
+        ret = s->internal->write_header_ret ? s->internal->write_header_ret : write_header_internal(s);
+        if (ret < 0)
+            goto fail;
+    }
+
 fail:
-    if (s->oformat->write_trailer) {
+    if (s->internal->header_written && s->oformat->write_trailer) {
         if (!(s->oformat->flags & AVFMT_NOFILE) && s->pb)
             avio_write_marker(s->pb, AV_NOPTS_VALUE, AVIO_DATA_MARKER_TRAILER);
         if (ret >= 0) {
@@ -1282,6 +1313,7 @@  fail:
     if (s->oformat->deinit)
         s->oformat->deinit(s);
 
+    s->internal->header_written =
     s->internal->initialized =
     s->internal->streams_initialized = 0;
 
diff --git a/libavformat/options_table.h b/libavformat/options_table.h
index b8fa47c6fd..b60d031f67 100644
--- a/libavformat/options_table.h
+++ b/libavformat/options_table.h
@@ -57,7 +57,7 @@  static const AVOption avformat_options[] = {
 {"seek2any", "allow seeking to non-keyframes on demuxer level when supported", OFFSET(seek2any), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, D},
 {"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" },
+{"autobsf", "add needed bsfs automatically (delays header until each stream's first packet is written)", 0, AV_OPT_TYPE_CONST, { .i64 = AVFMT_FLAG_AUTO_BSF }, 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/tests/fifo_muxer.c b/libavformat/tests/fifo_muxer.c
index 5127a8aadb..13a55db3a9 100644
--- a/libavformat/tests/fifo_muxer.c
+++ b/libavformat/tests/fifo_muxer.c
@@ -130,6 +130,54 @@  fail:
     return ret;
 }
 
+static int fifo_write_header_err_tst(AVFormatContext *oc, AVDictionary **opts,
+                                     const FailingMuxerPacketData *pkt_data)
+{
+    int ret = 0, i;
+    AVPacket pkt;
+
+    av_init_packet(&pkt);
+
+    ret = avformat_write_header(oc, opts);
+    if (ret) {
+        fprintf(stderr, "Unexpected write_header failure: %s\n",
+                av_err2str(ret));
+        goto fail;
+    }
+
+    for (i = 0; i < MAX_TST_PACKETS; i++ ) {
+        ret = prepare_packet(&pkt, pkt_data, i);
+        if (ret < 0) {
+            fprintf(stderr, "Failed to prepare test packet: %s\n",
+                    av_err2str(ret));
+            goto write_trailer_and_fail;
+        }
+        ret = av_write_frame(oc, &pkt);
+        av_packet_unref(&pkt);
+        if (ret < 0) {
+            break;
+        }
+    }
+
+    if (!ret) {
+        fprintf(stderr, "write_packet not failed when supposed to.\n");
+        goto fail;
+    } else if (ret != -1) {
+        fprintf(stderr, "Unexpected write_packet error: %s\n", av_err2str(ret));
+        goto fail;
+    }
+
+    ret = av_write_trailer(oc);
+    if (ret < 0)
+        fprintf(stderr, "Unexpected write_trailer error: %s\n", av_err2str(ret));
+
+    return ret;
+write_trailer_and_fail:
+    av_write_trailer(oc);
+fail:
+    return ret;
+}
+
 static int fifo_overflow_drop_test(AVFormatContext *oc, AVDictionary **opts,
                                    const FailingMuxerPacketData *data)
 {
@@ -247,6 +295,10 @@  const TestCase tests[] = {
          * exactly what was on input */
         {fifo_basic_test, "nonfail test", NULL,1, 0, 0, {0, 0, 0}},
 
+        /* Test that we receive delayed write_header error from one of the write_packet
+         * calls. */
+        {fifo_write_header_err_tst, "write header error test", NULL, 0, -1, 0, {0, 0, 0}},
+
         /* Each write_packet will fail 3 times before operation is successful. If recovery
          * Since recovery is on, fifo muxer should not return any errors. */
         {fifo_basic_test, "recovery test", "attempt_recovery=1:recovery_wait_time=0",
diff --git a/tests/ref/fate/fifo-muxer-tst b/tests/ref/fate/fifo-muxer-tst
index e1139ee0a8..ca7e294860 100644
--- a/tests/ref/fate/fifo-muxer-tst
+++ b/tests/ref/fate/fifo-muxer-tst
@@ -2,6 +2,7 @@  flush count: 1
 pts seen nr: 15
 pts seen: 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14
 nonfail test: ok
+write header error test: ok
 recovery test: ok
 flush count: 1
 pts seen nr: 15
diff --git a/tests/ref/fate/rgb24-mkv b/tests/ref/fate/rgb24-mkv
index 439b2bc5e9..4c357accaf 100644
--- a/tests/ref/fate/rgb24-mkv
+++ b/tests/ref/fate/rgb24-mkv
@@ -1,5 +1,5 @@ 
-09ee413b2d92a6be5e3b18e9e20a1f74 *tests/data/fate/rgb24-mkv.matroska
-58342 tests/data/fate/rgb24-mkv.matroska
+55270be3b5d393d770a1dfcb19b68271 *tests/data/fate/rgb24-mkv.matroska
+58345 tests/data/fate/rgb24-mkv.matroska
 #tb 0: 1/10
 #media_type 0: video
 #codec_id 0: rawvideo