From patchwork Thu Sep 7 19:17:24 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Almer X-Patchwork-Id: 5040 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.2.15.201 with SMTP id 70csp569059jao; Thu, 7 Sep 2017 12:18:10 -0700 (PDT) X-Received: by 10.223.179.10 with SMTP id j10mr205963wrd.273.1504811890194; Thu, 07 Sep 2017 12:18:10 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1504811890; cv=none; d=google.com; s=arc-20160816; b=dQPCxHia9eUdnE4krVeM5ZsVbxjYyFNWASybaqFM5DCAEUFVydTt0Bl8TcN6BEvk7q lVX0wMjFGZSsqtxxF0W/lM7NKTfMgTQe5030Gt7LJ196REANyFyJyoa8zGcDN+rmFA+i l73yes+sOaq7HTgEzg5R1qImm2lUOvtzm9zouAb7tR8m4kDhzKbUl81/e/j9tVX41g3c 4IAhU2zmv7A+9Zf24AsezZGg+jnxf48iWHWMLCJdB2ZJZkpwLKhzE/XW3yvDZEjU/s56 8AEkFbcHT9TJAQWr7pms/lYKPxuPS9yBRZGKJVp2t68yKe9zhTjf2h91SEaqfDRYPJSr 9DBg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:content-transfer-encoding:mime-version:reply-to :list-subscribe:list-help:list-post:list-archive:list-unsubscribe :list-id:precedence:subject:references:in-reply-to:message-id:date :to:from:dkim-signature:delivered-to:arc-authentication-results; bh=TpRA+i1oky3GKfV8PbsCxxI87fslpScY18PRP8JXnrw=; b=rB+sdq54MYX4cEZDGTO1dOewvlQlxJCzzZ4DrbOb6QWVSkihp/WnxWkzeBXNVovpDa cwWDvIH6B5kDeKlGyv6kYCZDEVJGZDeOawtzABYVzbHtaLf7VdFQgia+ZKd80enZhUqy fQpAAxdfthkEDqWlUeTc5BCyZeH/bsFWFKalmnO6vWpFxQ9awBRzKvMnaPV+zyuaaSsI ln3xe8eozFfNXF92UbucFDDTM4yzXqdYJtgFD9kLND38qb5bW7w+TxvQYfIhuCjJ9Zui BPDR3Fe2DNnvh2zzQ4Og8W3a0Owm8URtLgKpni3rQaKvHb6pJIF6I4c33k/yEGp1wSU7 AbVw== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@gmail.com header.s=20161025 header.b=AIitHEas; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=gmail.com Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id j63si7925wmf.142.2017.09.07.12.18.08; Thu, 07 Sep 2017 12:18:10 -0700 (PDT) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@gmail.com header.s=20161025 header.b=AIitHEas; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=gmail.com Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 2C8AA689E7B; Thu, 7 Sep 2017 22:18:01 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-qt0-f182.google.com (mail-qt0-f182.google.com [209.85.216.182]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 33568689CD6 for ; Thu, 7 Sep 2017 22:17:54 +0300 (EEST) Received: by mail-qt0-f182.google.com with SMTP id b52so1652124qtb.3 for ; Thu, 07 Sep 2017 12:17:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:subject:date:message-id:in-reply-to:references; bh=yLBBSHYQ2pA0QQf7UETYogBVOx2vSZ5S+rPFbKXtwzs=; b=AIitHEas/U53EZ2x1k0aAe3a1PnkZB811CUe0QDK7gmk1J6ylyqOUPRQIQSFpizrIk cL4qdNWXYQpb9FrhWaYmdydUpec5gm1Rk5A/lUpNRCkeVClV8Wge5eLSNeCtV6ewgrl8 RtTO4FAG54DBKWxzWPwGkKy9Jn3/goAdK6RKmWY1aNdB4TGlChUx9av5c6wGDC9NTrL0 /+KLvc3jZTA4PMJ0PG49WwJa+4h7vJXdH5mROzKTS3K1wBSvZRtC+AtkZ8rs+e8CAWOe bCig953994zj22kTNA3u72EX4b+QvW61WJJHXT6psy7TRhAyrUzbgJ/U+Vwj7dyzg+AL 7mLw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=yLBBSHYQ2pA0QQf7UETYogBVOx2vSZ5S+rPFbKXtwzs=; b=LsDIfUJWYjZ8tkiHvNodXFYGxr5yu6bMrwL5/A4lVEzNlyWMGM4eG2cIbsY5q3+eTF XxJM+PZCG+C8Zaj5TfvPqALYg/zPq6eSODu01p1SBGvTo2FQNKsWffNantw7h3Ao8Zsg 7/+LgZEFR5cAJ/DQLk9R2QR6j7i2/IjQ+IlsK2cL+3hERhlA803TtDZUy4vy0LtC0qHD XD5rpAZZgZiZ7LF/WLXP3NIx1p23VIuEvT5a5yGsoV8/8G54R5cgwkItJrDhMSqnjuYg osRr9orOPBFj52FJ+tjmNp96x0ZCaHkKmFS2CL9eHs5e3lZTT0DZxYvlDTqtqvn4tdVO /Agg== X-Gm-Message-State: AHPjjUhRCMx/B6aIZ2aHhkozDpjvcL2nb3y/5GSquwyWYUFOGk2+yc5f 037njr8M8s9UBy8A X-Google-Smtp-Source: AOwi7QDZNG2Ju3hzdukIg7NvK6a6OY0y4KqDfpdVe019N2WHjSOW3tf6ot19Y9f5EBKYMnQPxB/J0Q== X-Received: by 10.237.35.139 with SMTP id j11mr645017qtc.96.1504811876505; Thu, 07 Sep 2017 12:17:56 -0700 (PDT) Received: from localhost.localdomain ([181.231.60.193]) by smtp.gmail.com with ESMTPSA id z192sm18044qka.91.2017.09.07.12.17.54 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 07 Sep 2017 12:17:54 -0700 (PDT) From: James Almer To: ffmpeg-devel@ffmpeg.org Date: Thu, 7 Sep 2017 16:17:24 -0300 Message-Id: <20170907191724.9956-1-jamrial@gmail.com> X-Mailer: git-send-email 2.13.3 In-Reply-To: <20170907181643.GQ7094@nb4> References: <20170907181643.GQ7094@nb4> Subject: [FFmpeg-devel] [PATCH] avformat/mux: stop delaying writing the header X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches MIME-Version: 1.0 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" There's no need to wait for the first packet of every stream now that every bitstream filter behaves as intended. Signed-off-by: James Almer --- What should we do with the AVFMT_FLAG_AUTO_BSF flag? Do we deprecate it and force the automatic insertion of muxer-required bitstream filters now that the generic muxing code will always behave the same, or keep it to give the user a choice to apply said bitstream filters? I ask because some filters, like vp9_superframe, are necessary to avoid creating broken files, so it's not wise to allow the user to disable it. It would also go in line with AVCodec.bsfs, which is essentially a non-optional (for reasons made obvious in fa1749dd34) autobsf at the codec level instead of container level. The change to fate-rgb24-mkv is a regression that can be prevented by committing https://ffmpeg.org/pipermail/ffmpeg-devel/2017-May/211311.html or a similar solution, maybe using avformat_init_output(), to make sure ffmpeg.c sets AVCodecParameters->field_order for the output stream before calling avformat_write_header(). libavformat/internal.h | 6 ----- libavformat/mux.c | 51 ++++++++--------------------------------- libavformat/options_table.h | 2 +- libavformat/tests/fifo_muxer.c | 52 ------------------------------------------ tests/ref/fate/fifo-muxer-tst | 1 - tests/ref/fate/rgb24-mkv | 4 ++-- 6 files changed, 13 insertions(+), 103 deletions(-) diff --git a/libavformat/internal.h b/libavformat/internal.h index d136c79bdd..d46340029b 100644 --- a/libavformat/internal.h +++ b/libavformat/internal.h @@ -121,12 +121,6 @@ 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. */ int64_t shortest_end; diff --git a/libavformat/mux.c b/libavformat/mux.c index 53ad46df42..6a81faf3fb 100644 --- a/libavformat/mux.c +++ b/libavformat/mux.c @@ -478,25 +478,6 @@ 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; @@ -535,11 +516,18 @@ int avformat_write_header(AVFormatContext *s, AVDictionary **options) if ((ret = avformat_init_output(s, options)) < 0) return ret; - if (!(s->oformat->check_bitstream && s->flags & AVFMT_FLAG_AUTO_BSF)) { - ret = write_header_internal(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; 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) @@ -765,12 +753,6 @@ FF_DISABLE_DEPRECATION_WARNINGS FF_ENABLE_DEPRECATION_WARNINGS #endif - 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); @@ -786,7 +768,6 @@ FF_ENABLE_DEPRECATION_WARNINGS ret = s->pb->error; } -fail: #if FF_API_LAVF_MERGE_SD FF_DISABLE_DEPRECATION_WARNINGS if (did_split) @@ -934,11 +915,6 @@ 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) @@ -1322,14 +1298,8 @@ 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->internal->header_written && s->oformat->write_trailer) { + if (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) { @@ -1342,7 +1312,6 @@ 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 118086df66..c62b7d71d6 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 (delays header until each stream's first packet is written)", 0, AV_OPT_TYPE_CONST, { .i64 = AVFMT_FLAG_AUTO_BSF }, 0, 0, E, "fflags" }, +{"autobsf", "add needed bsfs automatically", 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 e20bd6e7b2..4b63df085e 100644 --- a/libavformat/tests/fifo_muxer.c +++ b/libavformat/tests/fifo_muxer.c @@ -238,54 +238,6 @@ 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) { @@ -403,10 +355,6 @@ 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 ca7e294860..e1139ee0a8 100644 --- a/tests/ref/fate/fifo-muxer-tst +++ b/tests/ref/fate/fifo-muxer-tst @@ -2,7 +2,6 @@ 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 4c357accaf..439b2bc5e9 100644 --- a/tests/ref/fate/rgb24-mkv +++ b/tests/ref/fate/rgb24-mkv @@ -1,5 +1,5 @@ -55270be3b5d393d770a1dfcb19b68271 *tests/data/fate/rgb24-mkv.matroska -58345 tests/data/fate/rgb24-mkv.matroska +09ee413b2d92a6be5e3b18e9e20a1f74 *tests/data/fate/rgb24-mkv.matroska +58342 tests/data/fate/rgb24-mkv.matroska #tb 0: 1/10 #media_type 0: video #codec_id 0: rawvideo