| Message ID | 1483952212-26754-2-git-send-email-t.rapp@noa-archive.com |
|---|---|
| State | Superseded |
| Headers | show |
On Mon, Jan 09, 2017 at 09:56:51AM +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. > > Signed-off-by: Tobias Rapp <t.rapp@noa-archive.com> > --- > libavformat/avi.h | 1 - > libavformat/avienc.c | 36 +++++++++++++++++++++++++++++------- > libavformat/version.h | 2 +- > 3 files changed, 30 insertions(+), 9 deletions(-) iam not against this but as the only way to write strictly spec compiant files >= 256g, i think this is not very user friendly FFmpeg should work out of the box not requireing the user to tune options Here are some random ideas: Enlarge the default index depending on expected bitrate, a high res rawvideo input should by default get a bigger reserved index space Use the input file duration (aka pass it to the muxer) and use it as a guess on duration, enlarge the space accordingly Or, do the hard but correct and just enlarge it As in when you hit some SIZE1 like lets say 1GB leave 1mb space (which at this filesize is a insignificant 0.1%) and when writing the trailer and the file exceeded 256G go back and move the first 1gb forward to make space for a larger master index. (this at that point just moves 0.5% of the file) You can also use the 1mb space for holding a 2nd master index prior to writing the trailer. Either way the file should ideally be partially indexed while it is being written so a 2nd process could read and mostly seek in it [...]
On 10.01.2017 00:15, Michael Niedermayer wrote: > On Mon, Jan 09, 2017 at 09:56:51AM +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. >> >> Signed-off-by: Tobias Rapp <t.rapp@noa-archive.com> >> --- >> libavformat/avi.h | 1 - >> libavformat/avienc.c | 36 +++++++++++++++++++++++++++++------- >> libavformat/version.h | 2 +- >> 3 files changed, 30 insertions(+), 9 deletions(-) > > iam not against this but as the only way to write strictly spec > compiant files >= 256g, i think this is not very user friendly > > FFmpeg should work out of the box not requireing the user to tune > options I took the "reserve_index_space" option of matroskaenc.c as an inspiration. But I see that for Matroska it is "just" a matter of optimization while for AVI it means compliance + optimization. > Here are some random ideas: > > Enlarge the default index depending on expected bitrate, a high res > rawvideo input should by default get a bigger reserved index space Yes, this would be a low-hanging fruit. > Use the input file duration (aka pass it to the muxer) and use it as > a guess on duration, enlarge the space accordingly You mean guessing and setting the AVStream->duration field in ffmpeg.c as a hint to the AVI muxer? Will take a look on how this could be done. > Or, do the hard but correct and just enlarge it > As in when you hit some SIZE1 like lets say 1GB leave 1mb space > (which at this filesize is a insignificant 0.1%) > and when writing the trailer and the file exceeded 256G go back and > move the first 1gb forward to make space for a larger master index. > (this at that point just moves 0.5% of the file) > You can also use the 1mb space for holding a 2nd master index prior > to writing the trailer. Either way the file should ideally be partially > indexed while it is being written so a 2nd process could read and mostly > seek in it Moving the first 1GB forward also means updating each entry within the legacy "idx1" index structure plus the base offset of each ODML standard stream index. Further as the ODML master index of each stream has to be enlarged also parts of the file header need to be moved. In my opinion this would be a complex and fragile process. It seems more safe to either warn the user to re-mux with adapted options (if he/she cares for strict compliance) or fail, as discussed in the other thread of this patch series. Regards, Tobias
On Tue, Jan 10, 2017 at 10:41:04AM +0100, Tobias Rapp wrote: > On 10.01.2017 00:15, Michael Niedermayer wrote: > >On Mon, Jan 09, 2017 at 09:56:51AM +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. > >> > >>Signed-off-by: Tobias Rapp <t.rapp@noa-archive.com> > >>--- > >> libavformat/avi.h | 1 - > >> libavformat/avienc.c | 36 +++++++++++++++++++++++++++++------- > >> libavformat/version.h | 2 +- > >> 3 files changed, 30 insertions(+), 9 deletions(-) > > > >iam not against this but as the only way to write strictly spec > >compiant files >= 256g, i think this is not very user friendly > > > >FFmpeg should work out of the box not requireing the user to tune > >options > > I took the "reserve_index_space" option of matroskaenc.c as an > inspiration. But I see that for Matroska it is "just" a matter of > optimization while for AVI it means compliance + optimization. > > >Here are some random ideas: > > > >Enlarge the default index depending on expected bitrate, a high res > >rawvideo input should by default get a bigger reserved index space > > Yes, this would be a low-hanging fruit. > > >Use the input file duration (aka pass it to the muxer) and use it as > >a guess on duration, enlarge the space accordingly > > You mean guessing and setting the AVStream->duration field in > ffmpeg.c as a hint to the AVI muxer? Will take a look on how this > could be done. yes, that was the idea > > >Or, do the hard but correct and just enlarge it > >As in when you hit some SIZE1 like lets say 1GB leave 1mb space > >(which at this filesize is a insignificant 0.1%) > >and when writing the trailer and the file exceeded 256G go back and > >move the first 1gb forward to make space for a larger master index. > >(this at that point just moves 0.5% of the file) > >You can also use the 1mb space for holding a 2nd master index prior > >to writing the trailer. Either way the file should ideally be partially > >indexed while it is being written so a 2nd process could read and mostly > >seek in it > > Moving the first 1GB forward also means updating each entry within > the legacy "idx1" index structure plus the base offset of each ODML > standard stream index. Further as the ODML master index of each > stream has to be enlarged also parts of the file header need to be > moved. simply writing the whole header again may be cleaner than moving parts of it thx [...]
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 5d5c02a..faa86e1 100644 --- a/libavformat/avienc.c +++ b/libavformat/avienc.c @@ -51,6 +51,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 +#define AVI_MASTER_INDEX_CLUSTER_SIZE (AVI_MASTER_INDEX_PREFIX_SIZE + 256 * AVI_MASTER_INDEX_ENTRY_SIZE) typedef struct AVIIndex { int64_t indx_start; @@ -66,6 +69,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 +139,20 @@ 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; + const int reserve_index_space = (avi->reserve_index_space > 0) ? + avi->reserve_index_space : AVI_MASTER_INDEX_CLUSTER_SIZE; + + avi->master_index_max_size = (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); + 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 0; +} + static int64_t avi_start_new_riff(AVFormatContext *s, AVIOContext *pb, const char *riff_tag, const char *list_tag) { @@ -210,6 +229,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 +249,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); } @@ -576,17 +596,17 @@ static int avi_write_ix(AVFormatContext *s) av_assert0(pb->seekable); - if (avi->riff_id >= AVI_MASTER_INDEX_SIZE && s->strict_std_compliance > FF_COMPLIANCE_NORMAL) { - av_log(s, AV_LOG_ERROR, "Invalid riff index %d >= %d\n", - avi->riff_id, AVI_MASTER_INDEX_SIZE); + if (avi->riff_id >= avi->master_index_max_size && s->strict_std_compliance > FF_COMPLIANCE_NORMAL) { + av_log(s, AV_LOG_ERROR, "Invalid riff index %d >= %d, consider increasing the 'reserve_index_space' option value\n", + avi->riff_id, avi->master_index_max_size); return AVERROR(EINVAL); } 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, AVI_UPDATE_ODML_ENTRY_MASTER); @@ -594,7 +614,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++) { @@ -924,6 +944,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 }, }; @@ -943,6 +964,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, \
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. Signed-off-by: Tobias Rapp <t.rapp@noa-archive.com> --- libavformat/avi.h | 1 - libavformat/avienc.c | 36 +++++++++++++++++++++++++++++------- libavformat/version.h | 2 +- 3 files changed, 30 insertions(+), 9 deletions(-)