From patchwork Mon Jan 23 10:12:13 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tobias Rapp X-Patchwork-Id: 2293 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.89.21 with SMTP id n21csp1163030vsb; Mon, 23 Jan 2017 02:12:25 -0800 (PST) X-Received: by 10.28.69.28 with SMTP id s28mr10327657wma.40.1485166345362; Mon, 23 Jan 2017 02:12:25 -0800 (PST) Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id 43si18224185wrk.228.2017.01.23.02.12.24; Mon, 23 Jan 2017 02:12:25 -0800 (PST) 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; 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 Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 909C9689FD5; Mon, 23 Jan 2017 12:12:09 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from p1002.netstorage.at (p1002.netstorage.at [89.207.146.186]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 1F690689FC2 for ; Mon, 23 Jan 2017 12:12:02 +0200 (EET) Received: from mailix (noaport.de [46.237.252.213]) by p1002.netstorage.at (Postfix) with ESMTPA id 8F15581526 for ; Mon, 23 Jan 2017 11:12:14 +0100 (CET) Received: from [127.0.0.1] (HSI-KBW-46-237-252-214.hsi.kabel-badenwuerttemberg.de [46.237.252.214]) by mailix with ESMTPSA (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128) ; Mon, 23 Jan 2017 11:12:14 +0100 To: ffmpeg-devel@ffmpeg.org References: <1484731623-21636-1-git-send-email-t.rapp@noa-archive.com> <1484731623-21636-3-git-send-email-t.rapp@noa-archive.com> <20170119173255.GL4698@nb4> <912202d5-e6b3-df2c-d6de-9097c63d7e43@noa-archive.com> <20170120200227.GO4698@nb4> From: Tobias Rapp Organization: NOA GmbH Message-ID: Date: Mon, 23 Jan 2017 11:12:13 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: <20170120200227.GO4698@nb4> X-PPP-Message-ID: <20170123101214.9170.75972@p1002.netstorage.at> X-PPP-Vhost: noa-archive.com Subject: Re: [FFmpeg-devel] [PATCH 2/3] avformat/avienc: add reserve_index_space option 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 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" On 20.01.2017 21:02, Michael Niedermayer wrote: > On Fri, Jan 20, 2017 at 05:06:59PM +0100, Tobias Rapp wrote: >> On 20.01.2017 15:56, Tobias Rapp wrote: >>> On 19.01.2017 18:32, Michael Niedermayer wrote: >>>> On Wed, Jan 18, 2017 at 10:27:02AM +0100, Tobias Rapp wrote: >>>>> Allows the user to reserve space for the ODML master index. A sufficient >>>>> sized master index in the AVI header avoids storing follow-up master >>>>> indexes within the 'movi' data later. >>>>> >>>>> If the option is omitted or zero the index size is estimated from output >>>>> duration and bitrate. A worst-case bitrate for video streams is assumed >>>>> in case it is not available. >>>>> >>>>> Note: fate reference files changed because the video stream had zero >>>>> bitrate before and is guessed now. >>>>> >>>>> Signed-off-by: Tobias Rapp >>>>> --- >>>>> libavformat/avi.h | 1 - >>>>> libavformat/avienc.c | 77 >>>>> ++++++++++++++++++++++++++++++--- >>>>> libavformat/version.h | 2 +- >>>>> tests/ref/fate/mpeg4-bsf-unpack-bframes | 2 +- >>>>> tests/ref/lavf-fate/avi_cram | 2 +- >>>>> 5 files changed, 74 insertions(+), 10 deletions(-) >>>> >>>> this breaks segment: >>>> ./ffmpeg -i lena.pnm -f segment test%d.avi >>>> >>>> possibly related to avi_init() >>> >>> Yes, I can reproduce the problem when going back to Git master and just >>> adding a dummy init (see attached diff). Not sure how to fix this, other >>> muxers also have an init but seem to work fine (mkv) ... >> >> Apparently the codec_tag is cleared in seg_write_header() around line 811: >> >> [...] >> >> See http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavformat/segment.c;hb=HEAD#l811 >> >> If .init is unset, the header is written before the codec_tag is >> cleared. If .init is set, it is written after these lines. > > the return code of your init function is wrong i think Thanks for the hint. Changed the return value to "1" and now avi_write_header() keeps being executed before the codec_tag is cleared in seg_write_header(). Attached an updated version of the patch. > also this patch causes a 1 byte difference in the output of: > ./ffmpeg -i ~/tickets/1116/AVI-RLE_MP3.avi -vcodec copy -an out2.avi > > is that intended ? Yes, it is expected in this case. The video stream has a reported bitrate of "0" which was used as-is for the "dwMaxBytesPerSec" field in the "avih" chunk before. Now the field is changed due to the worst-case bitrate guessing (similar to the FATE reference file changes). A log message hint is printed when adding "-loglevel debug" and maybe it should be made more prominent (AV_LOG_WARNING?) but there is not much a user can do and I think it's unlikely it has a negative effect on players. Regards, Tobias From 4cc70ffdeb7eeb60e7bbb725bd5885dcacf968d2 Mon Sep 17 00:00:00 2001 From: Tobias Rapp Date: Wed, 4 Jan 2017 15:31:29 +0100 Subject: [PATCH 2/3] avformat/avienc: add reserve_index_space option Allows the user to reserve space for the ODML master index. A sufficient sized master index in the AVI header avoids storing follow-up master indexes within the 'movi' data later. If the option is omitted or zero the index size is estimated from output duration and bitrate. A worst-case bitrate for video streams is assumed in case it is not available. Note: fate reference files changed because the video stream had zero bitrate before and is guessed now. Signed-off-by: Tobias Rapp --- libavformat/avi.h | 1 - libavformat/avienc.c | 77 ++++++++++++++++++++++++++++++--- libavformat/version.h | 2 +- tests/ref/fate/mpeg4-bsf-unpack-bframes | 2 +- tests/ref/lavf-fate/avi_cram | 2 +- 5 files changed, 74 insertions(+), 10 deletions(-) diff --git a/libavformat/avi.h b/libavformat/avi.h index af21f2c..b1711f0 100644 --- a/libavformat/avi.h +++ b/libavformat/avi.h @@ -29,7 +29,6 @@ #define AVIF_COPYRIGHTED 0x00020000 #define AVI_MAX_RIFF_SIZE 0x40000000LL -#define AVI_MASTER_INDEX_SIZE 256 #define AVI_MAX_STREAM_COUNT 100 /* stream header flags */ diff --git a/libavformat/avienc.c b/libavformat/avienc.c index fd16fff..5d4ec72 100644 --- a/libavformat/avienc.c +++ b/libavformat/avienc.c @@ -21,6 +21,8 @@ //#define DEBUG +#include + #include "avformat.h" #include "internal.h" #include "avi.h" @@ -29,6 +31,7 @@ #include "mpegts.h" #include "libavformat/avlanguage.h" #include "libavutil/avstring.h" +#include "libavutil/avutil.h" #include "libavutil/internal.h" #include "libavutil/intreadwrite.h" #include "libavutil/dict.h" @@ -51,6 +54,9 @@ typedef struct AVIIentry { } AVIIentry; #define AVI_INDEX_CLUSTER_SIZE 16384 +#define AVI_MASTER_INDEX_PREFIX_SIZE (8+2+1+1+4+8+4+4) +#define AVI_MASTER_INDEX_ENTRY_SIZE 16 /* bytes per entry */ +#define AVI_MASTER_INDEX_SIZE_DEFAULT 256 /* number of entries */ typedef struct AVIIndex { int64_t indx_start; @@ -66,6 +72,8 @@ typedef struct AVIContext { int64_t riff_start, movi_list, odml_list; int64_t frames_hdr_all; int riff_id; + int reserve_index_space; + int master_index_max_size; int write_channel_mask; } AVIContext; @@ -134,6 +142,21 @@ static int avi_add_ientry(AVFormatContext *s, int stream_index, char *tag, return 0; } +static av_cold int avi_init(struct AVFormatContext *s) +{ + AVIContext *avi = s->priv_data; + + if (avi->reserve_index_space > 0) { + avi->master_index_max_size = (avi->reserve_index_space - AVI_MASTER_INDEX_PREFIX_SIZE) / AVI_MASTER_INDEX_ENTRY_SIZE; + avi->master_index_max_size = FFMAX(avi->master_index_max_size, 16); + } else + avi->master_index_max_size = AVI_MASTER_INDEX_SIZE_DEFAULT; + av_log(s, AV_LOG_DEBUG, "reserve_index_space:%d master_index_max_size:%d\n", + avi->reserve_index_space, avi->master_index_max_size); + + return 1; /* stream initialization continues in avi_write_header */ +} + static int64_t avi_start_new_riff(AVFormatContext *s, AVIOContext *pb, const char *riff_tag, const char *list_tag) { @@ -210,6 +233,7 @@ static int avi_write_counters(AVFormatContext *s, int riff_id) static void write_odml_master(AVFormatContext *s, int stream_index) { AVIOContext *pb = s->pb; + AVIContext *avi = s->priv_data; AVStream *st = s->streams[stream_index]; AVCodecParameters *par = st->codecpar; AVIStream *avist = st->priv_data; @@ -229,7 +253,7 @@ static void write_odml_master(AVFormatContext *s, int stream_index) /* dwChunkId */ avio_wl64(pb, 0); /* dwReserved[3] */ avio_wl32(pb, 0); /* Must be 0. */ - for (j = 0; j < AVI_MASTER_INDEX_SIZE * 2; j++) + for (j = 0; j < avi->master_index_max_size * 2; j++) avio_wl64(pb, 0); ff_end_tag(pb, avist->indexes.indx_start); } @@ -239,6 +263,7 @@ static int avi_write_header(AVFormatContext *s) AVIContext *avi = s->priv_data; AVIOContext *pb = s->pb; int bitrate, n, i, nb_frames, au_byterate, au_ssize, au_scale; + int64_t max_stream_duration = 0; AVCodecParameters *video_par; AVStream *video_st = NULL; int64_t list1, list2, strh, strf; @@ -269,13 +294,43 @@ static int avi_write_header(AVFormatContext *s) video_par = NULL; for (n = 0; n < s->nb_streams; n++) { AVCodecParameters *par = s->streams[n]->codecpar; - bitrate += par->bit_rate; + AVStream *st = s->streams[n]; + int64_t stream_bitrate = par->bit_rate; + /* when bitrate is unset, assume uncompressed as a worst-case value */ + if (!stream_bitrate && par->codec_type == AVMEDIA_TYPE_VIDEO && + (st->avg_frame_rate.num > 0) && (st->avg_frame_rate.den > 0)) { + stream_bitrate = (int64_t)par->bits_per_coded_sample * par->width * par->height * + st->avg_frame_rate.num / st->avg_frame_rate.den; + av_log(s, AV_LOG_DEBUG, "Bitrate not available for stream #%d (%s), estimating %"PRId64"kbit/s\n", + n, avcodec_get_name(par->codec_id), stream_bitrate / 1000); + } + bitrate = FFMIN(bitrate + stream_bitrate, INT32_MAX); + if (st->duration > 0) { + int64_t stream_duration = av_rescale_q(st->duration, st->time_base, AV_TIME_BASE_Q); + max_stream_duration = FFMAX(stream_duration, max_stream_duration); + } if (par->codec_type == AVMEDIA_TYPE_VIDEO) { video_par = par; - video_st = s->streams[n]; + video_st = st; } } + /* guess master index size based on bitrate and duration */ + if (!avi->reserve_index_space) { + double duration_est, filesize_est; + if (s->duration > 0) + duration_est = (double)s->duration / AV_TIME_BASE; + else if (max_stream_duration > 0) + duration_est = (double)max_stream_duration / AV_TIME_BASE; + else + duration_est = 10 * 60 * 60; /* default to 10 hours */ + filesize_est = duration_est * (bitrate / 8) * 1.10; /* add 10% safety margin for muxer+bitrate */ + avi->master_index_max_size = FFMAX((int)ceil(filesize_est / AVI_MAX_RIFF_SIZE) + 1, + avi->master_index_max_size); + av_log(s, AV_LOG_DEBUG, "duration_est:%0.3f, filesize_est:%0.1fGiB, master_index_max_size:%d\n", + duration_est, filesize_est / (1024*1024*1024), avi->master_index_max_size); + } + nb_frames = 0; // TODO: should be avg_frame_rate @@ -569,9 +624,9 @@ static int avi_write_ix(AVFormatContext *s) for (i = 0; i < s->nb_streams; i++) { AVIStream *avist = s->streams[i]->priv_data; - if (avi->riff_id - avist->indexes.master_odml_riff_id_base == AVI_MASTER_INDEX_SIZE) { + if (avi->riff_id - avist->indexes.master_odml_riff_id_base == avi->master_index_max_size) { int64_t pos; - int size = 8+2+1+1+4+8+4+4+16*AVI_MASTER_INDEX_SIZE; + int size = AVI_MASTER_INDEX_PREFIX_SIZE + AVI_MASTER_INDEX_ENTRY_SIZE * avi->master_index_max_size; pos = avio_tell(pb); update_odml_entry(s, i, pos, size); @@ -579,7 +634,7 @@ static int avi_write_ix(AVFormatContext *s) av_assert1(avio_tell(pb) - pos == size); avist->indexes.master_odml_riff_id_base = avi->riff_id - 1; } - av_assert0(avi->riff_id - avist->indexes.master_odml_riff_id_base < AVI_MASTER_INDEX_SIZE); + av_assert0(avi->riff_id - avist->indexes.master_odml_riff_id_base < avi->master_index_max_size); } for (i = 0; i < s->nb_streams; i++) { @@ -890,6 +945,14 @@ static int avi_write_trailer(AVFormatContext *s) } } + if (avi->riff_id >= avi->master_index_max_size) { + int index_space = AVI_MASTER_INDEX_PREFIX_SIZE + + AVI_MASTER_INDEX_ENTRY_SIZE * avi->riff_id; + av_log(s, AV_LOG_WARNING, "Output file not strictly OpenDML compliant, " + "consider re-muxing with 'reserve_index_space' option value >= %d\n", + index_space); + } + for (i = 0; i < s->nb_streams; i++) { AVIStream *avist = s->streams[i]->priv_data; for (j = 0; j < avist->indexes.ents_allocated / AVI_INDEX_CLUSTER_SIZE; j++) @@ -908,6 +971,7 @@ static int avi_write_trailer(AVFormatContext *s) #define OFFSET(x) offsetof(AVIContext, x) #define ENC AV_OPT_FLAG_ENCODING_PARAM static const AVOption options[] = { + { "reserve_index_space", "reserve space (in bytes) at the beginning of the file for each stream index", OFFSET(reserve_index_space), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, ENC }, { "write_channel_mask", "write channel mask into wave format header", OFFSET(write_channel_mask), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, ENC }, { NULL }, }; @@ -927,6 +991,7 @@ AVOutputFormat ff_avi_muxer = { .priv_data_size = sizeof(AVIContext), .audio_codec = CONFIG_LIBMP3LAME ? AV_CODEC_ID_MP3 : AV_CODEC_ID_AC3, .video_codec = AV_CODEC_ID_MPEG4, + .init = avi_init, .write_header = avi_write_header, .write_packet = avi_write_packet, .write_trailer = avi_write_trailer, diff --git a/libavformat/version.h b/libavformat/version.h index 21cc8a9..70c8467 100644 --- a/libavformat/version.h +++ b/libavformat/version.h @@ -33,7 +33,7 @@ // Also please add any ticket numbers that you believe might be affected here #define LIBAVFORMAT_VERSION_MAJOR 57 #define LIBAVFORMAT_VERSION_MINOR 62 -#define LIBAVFORMAT_VERSION_MICRO 100 +#define LIBAVFORMAT_VERSION_MICRO 101 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \ LIBAVFORMAT_VERSION_MINOR, \ diff --git a/tests/ref/fate/mpeg4-bsf-unpack-bframes b/tests/ref/fate/mpeg4-bsf-unpack-bframes index 162d436..499f825 100644 --- a/tests/ref/fate/mpeg4-bsf-unpack-bframes +++ b/tests/ref/fate/mpeg4-bsf-unpack-bframes @@ -1 +1 @@ -c9535e459c2ee4ead6d84b93bc7e9f46 +bcb771f8e756290d63d69b6268346ab2 diff --git a/tests/ref/lavf-fate/avi_cram b/tests/ref/lavf-fate/avi_cram index 82882fb..1217bcb 100644 --- a/tests/ref/lavf-fate/avi_cram +++ b/tests/ref/lavf-fate/avi_cram @@ -1,3 +1,3 @@ -6fc88702c23b895c305c5e1f51a0904e *./tests/data/lavf-fate/lavf.avi +87acefc3f163bb2b89de6b055eb721e8 *./tests/data/lavf-fate/lavf.avi 928260 ./tests/data/lavf-fate/lavf.avi ./tests/data/lavf-fate/lavf.avi CRC=0xa4770de2 -- 2.7.4