| Message ID | 1483952212-26754-1-git-send-email-t.rapp@noa-archive.com |
|---|---|
| State | Superseded |
| Headers | show |
On Mon, Jan 09, 2017 at 09:56:50AM +0100, Tobias Rapp wrote: > OpenDML specification v1.02 states that entries of a master index chunk > shall point to standard or field index chunks. > > Changed incorrect duration of last master index entry to -1 in case it > points to another master index. > > Propagate error when index write fails. > > Signed-off-by: Tobias Rapp <t.rapp@noa-archive.com> > --- > libavformat/avienc.c | 30 +++++++++++++++++++++++------- > 1 file changed, 23 insertions(+), 7 deletions(-) > > diff --git a/libavformat/avienc.c b/libavformat/avienc.c > index fd16fff..5d5c02a 100644 > --- a/libavformat/avienc.c > +++ b/libavformat/avienc.c > @@ -524,7 +524,13 @@ static int avi_write_header(AVFormatContext *s) > return 0; > } > > -static void update_odml_entry(AVFormatContext *s, int stream_index, int64_t ix, int size) > +/* Index entry points to standard index */ > +#define AVI_UPDATE_ODML_ENTRY_DEFAULT 0x00000000 > + > +/* Index entry points to another master index */ > +#define AVI_UPDATE_ODML_ENTRY_MASTER 0x00000001 > + > +static void update_odml_entry(AVFormatContext *s, int stream_index, int64_t ix, int size, int flags) > { > AVIOContext *pb = s->pb; > AVIContext *avi = s->priv_data; > @@ -544,7 +550,10 @@ static void update_odml_entry(AVFormatContext *s, int stream_index, int64_t ix, > avio_wl64(pb, ix); /* qwOffset */ > avio_wl32(pb, size); /* dwSize */ > ff_parse_specific_params(s->streams[stream_index], &au_byterate, &au_ssize, &au_scale); > - if (s->streams[stream_index]->codecpar->codec_type == AVMEDIA_TYPE_AUDIO && au_ssize > 0) { > + if (flags & AVI_UPDATE_ODML_ENTRY_MASTER) { > + av_assert0(s->strict_std_compliance <= FF_COMPLIANCE_NORMAL); > + avio_wl32(pb, -1); /* dwDuration (undefined) */ > + } else if (s->streams[stream_index]->codecpar->codec_type == AVMEDIA_TYPE_AUDIO && au_ssize > 0) { > uint32_t audio_segm_size = (avist->audio_strm_length - avist->indexes.audio_strm_offset); > if ((audio_segm_size % au_ssize > 0) && !avist->sample_requested) { > avpriv_request_sample(s, "OpenDML index duration for audio packets with partial frames"); > @@ -567,6 +576,12 @@ 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); > + return AVERROR(EINVAL); > + } isnt it better to warn the user and inform him at the end what size of reserved space would have been needed? not saying iam against this, i do see how it is formally correct to fail hard but it feels painfull to me to fail muxing at 256gb hard when we can perfectly fine just continue and the user can just remux the file with bigger reserved master index to fix it [...]
On 10.01.2017 00:19, Michael Niedermayer wrote: > On Mon, Jan 09, 2017 at 09:56:50AM +0100, Tobias Rapp wrote: >> OpenDML specification v1.02 states that entries of a master index chunk >> shall point to standard or field index chunks. >> >> Changed incorrect duration of last master index entry to -1 in case it >> points to another master index. >> >> Propagate error when index write fails. >> >> Signed-off-by: Tobias Rapp <t.rapp@noa-archive.com> >> --- >> libavformat/avienc.c | 30 +++++++++++++++++++++++------- >> 1 file changed, 23 insertions(+), 7 deletions(-) >> >> diff --git a/libavformat/avienc.c b/libavformat/avienc.c >> index fd16fff..5d5c02a 100644 >> --- a/libavformat/avienc.c >> +++ b/libavformat/avienc.c >> @@ -524,7 +524,13 @@ static int avi_write_header(AVFormatContext *s) >> return 0; >> } >> >> -static void update_odml_entry(AVFormatContext *s, int stream_index, int64_t ix, int size) >> +/* Index entry points to standard index */ >> +#define AVI_UPDATE_ODML_ENTRY_DEFAULT 0x00000000 >> + >> +/* Index entry points to another master index */ >> +#define AVI_UPDATE_ODML_ENTRY_MASTER 0x00000001 >> + >> +static void update_odml_entry(AVFormatContext *s, int stream_index, int64_t ix, int size, int flags) >> { >> AVIOContext *pb = s->pb; >> AVIContext *avi = s->priv_data; >> @@ -544,7 +550,10 @@ static void update_odml_entry(AVFormatContext *s, int stream_index, int64_t ix, >> avio_wl64(pb, ix); /* qwOffset */ >> avio_wl32(pb, size); /* dwSize */ >> ff_parse_specific_params(s->streams[stream_index], &au_byterate, &au_ssize, &au_scale); >> - if (s->streams[stream_index]->codecpar->codec_type == AVMEDIA_TYPE_AUDIO && au_ssize > 0) { >> + if (flags & AVI_UPDATE_ODML_ENTRY_MASTER) { >> + av_assert0(s->strict_std_compliance <= FF_COMPLIANCE_NORMAL); >> + avio_wl32(pb, -1); /* dwDuration (undefined) */ >> + } else if (s->streams[stream_index]->codecpar->codec_type == AVMEDIA_TYPE_AUDIO && au_ssize > 0) { >> uint32_t audio_segm_size = (avist->audio_strm_length - avist->indexes.audio_strm_offset); >> if ((audio_segm_size % au_ssize > 0) && !avist->sample_requested) { >> avpriv_request_sample(s, "OpenDML index duration for audio packets with partial frames"); > >> @@ -567,6 +576,12 @@ 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); >> + return AVERROR(EINVAL); >> + } > > isnt it better to warn the user and inform him at the end what size > of reserved space would have been needed? > > not saying iam against this, i do see how it is formally correct to > fail hard but it feels painfull to me to fail > muxing at 256gb hard when we can perfectly fine just continue and > the user can just remux the file with bigger reserved master index > to fix it But then adding "-strict strict" just enables a warning message and a non-compliant file is written still? Or do you mean that additionally a warning message could be written in case std_compliance is <= normal? Background story: I'm working on an application that fetches some AVI file byte range from tape storage and restores the file header. The header restoration requires the ODML master index in the header to be complete. Thus I'd like to ensure that all AVI files put into the archive have a compliant/compatible index structure. For such automated processes failing hard is more safe than continuing with a warning message, checking for log messages in the host application and possibly re-running the transcoding with adapted options. Regards, Tobias
On Tue, Jan 10, 2017 at 09:26:53AM +0100, Tobias Rapp wrote: > On 10.01.2017 00:19, Michael Niedermayer wrote: > >On Mon, Jan 09, 2017 at 09:56:50AM +0100, Tobias Rapp wrote: > >>OpenDML specification v1.02 states that entries of a master index chunk > >>shall point to standard or field index chunks. > >> > >>Changed incorrect duration of last master index entry to -1 in case it > >>points to another master index. > >> > >>Propagate error when index write fails. > >> > >>Signed-off-by: Tobias Rapp <t.rapp@noa-archive.com> > >>--- > >> libavformat/avienc.c | 30 +++++++++++++++++++++++------- > >> 1 file changed, 23 insertions(+), 7 deletions(-) > >> > >>diff --git a/libavformat/avienc.c b/libavformat/avienc.c > >>index fd16fff..5d5c02a 100644 > >>--- a/libavformat/avienc.c > >>+++ b/libavformat/avienc.c > >>@@ -524,7 +524,13 @@ static int avi_write_header(AVFormatContext *s) > >> return 0; > >> } > >> > >>-static void update_odml_entry(AVFormatContext *s, int stream_index, int64_t ix, int size) > >>+/* Index entry points to standard index */ > >>+#define AVI_UPDATE_ODML_ENTRY_DEFAULT 0x00000000 > >>+ > >>+/* Index entry points to another master index */ > >>+#define AVI_UPDATE_ODML_ENTRY_MASTER 0x00000001 > >>+ > >>+static void update_odml_entry(AVFormatContext *s, int stream_index, int64_t ix, int size, int flags) > >> { > >> AVIOContext *pb = s->pb; > >> AVIContext *avi = s->priv_data; > >>@@ -544,7 +550,10 @@ static void update_odml_entry(AVFormatContext *s, int stream_index, int64_t ix, > >> avio_wl64(pb, ix); /* qwOffset */ > >> avio_wl32(pb, size); /* dwSize */ > >> ff_parse_specific_params(s->streams[stream_index], &au_byterate, &au_ssize, &au_scale); > >>- if (s->streams[stream_index]->codecpar->codec_type == AVMEDIA_TYPE_AUDIO && au_ssize > 0) { > >>+ if (flags & AVI_UPDATE_ODML_ENTRY_MASTER) { > >>+ av_assert0(s->strict_std_compliance <= FF_COMPLIANCE_NORMAL); > >>+ avio_wl32(pb, -1); /* dwDuration (undefined) */ > >>+ } else if (s->streams[stream_index]->codecpar->codec_type == AVMEDIA_TYPE_AUDIO && au_ssize > 0) { > >> uint32_t audio_segm_size = (avist->audio_strm_length - avist->indexes.audio_strm_offset); > >> if ((audio_segm_size % au_ssize > 0) && !avist->sample_requested) { > >> avpriv_request_sample(s, "OpenDML index duration for audio packets with partial frames"); > > > >>@@ -567,6 +576,12 @@ 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); > >>+ return AVERROR(EINVAL); > >>+ } > > > >isnt it better to warn the user and inform him at the end what size > >of reserved space would have been needed? > > > >not saying iam against this, i do see how it is formally correct to > >fail hard but it feels painfull to me to fail > >muxing at 256gb hard when we can perfectly fine just continue and > >the user can just remux the file with bigger reserved master index > >to fix it > > But then adding "-strict strict" just enables a warning message and > a non-compliant file is written still? Or do you mean that > additionally a warning message could be written in case > std_compliance is <= normal? > > Background story: I'm working on an application that fetches some > AVI file byte range from tape storage and restores the file header. > The header restoration requires the ODML master index in the header > to be complete. Thus I'd like to ensure that all AVI files put into > the archive have a compliant/compatible index structure. > > For such automated processes failing hard is more safe than > continuing with a warning message, checking for log messages in the > host application and possibly re-running the transcoding with > adapted options. whats your oppinion on an option that selects muxer error behavior ? something that can fail immedeatly on error or warn but continue, return failure after writing the file and trailer that is a option to make it possible for a muxer to continue even if there was an error, maybe even a int max_muxer_error or something else? [...]
Added suggested output stream duration hinting. Uses output stream duration and
bitrate as a hint for better filesize guessing. If stream bitrate is unknown a
worst-case value is assumed.
Sidenote: Noticed during tests that most loss-less encoders don't set bitrate
and thus the options_table.h default value of 200k is used in avi_write_header.
Not sure whether to fix this per encoder or just clear bitrate somewhere in
avcodec_open2 based on AV_CODEC_PROP_LOSSLESS.
Tobias Rapp (4):
ffmpeg: pass output stream duration as a hint to the muxer
avformat/avienc: write chained master index only if std_compliance is
normal or below
avformat/avienc: add reserve_index_space option
doc/muxers: add AVI muxer documentation
doc/muxers.texi | 33 ++++++++++
ffmpeg.c | 9 +++
libavformat/avformat.h | 3 +
libavformat/avi.h | 1 -
libavformat/avienc.c | 108 ++++++++++++++++++++++++++++----
libavformat/version.h | 2 +-
tests/ref/fate/mpeg4-bsf-unpack-bframes | 2 +-
tests/ref/lavf-fate/avi_cram | 2 +-
8 files changed, 143 insertions(+), 17 deletions(-)
On 13.01.2017 17:42, Michael Niedermayer wrote: > On Tue, Jan 10, 2017 at 09:26:53AM +0100, Tobias Rapp wrote: >> On 10.01.2017 00:19, Michael Niedermayer wrote: >>> [...] >>> isnt it better to warn the user and inform him at the end what size >>> of reserved space would have been needed? >>> >>> not saying iam against this, i do see how it is formally correct to >>> fail hard but it feels painfull to me to fail >>> muxing at 256gb hard when we can perfectly fine just continue and >>> the user can just remux the file with bigger reserved master index >>> to fix it >> >> But then adding "-strict strict" just enables a warning message and >> a non-compliant file is written still? Or do you mean that >> additionally a warning message could be written in case >> std_compliance is <= normal? >> >> Background story: I'm working on an application that fetches some >> AVI file byte range from tape storage and restores the file header. >> The header restoration requires the ODML master index in the header >> to be complete. Thus I'd like to ensure that all AVI files put into >> the archive have a compliant/compatible index structure. >> >> For such automated processes failing hard is more safe than >> continuing with a warning message, checking for log messages in the >> host application and possibly re-running the transcoding with >> adapted options. > > whats your oppinion on an option that selects muxer error behavior ? > something that can > fail immedeatly on error or > warn but continue, return failure after writing the file and trailer > > that is a option to make it possible for a muxer to continue even > if there was an error, maybe even a int max_muxer_error or something > else? You mean something like extending the current use of error_recognition and AV_EF_EXPLODE to encoders/muxers? In general it might be a good idea but I don't see how it helps in the current case. When all entries of the master index are exhausted and another entry is to be added the basic options are: a) write a chained master index (currently implemented) b) enlarge the master index (currently not implemented and I won't volunteer for doing it) c) fail (a) slightly violates the specs but (b+c) doesn't. One could see (c) as a placeholder until (b) is done. In that case '-strict 1' would perform (b) and '-strict -1' might perform (a) because it is faster. Version 2 of the patch series now adds a compliance warning with proper reserve_index_space value to (a). Regards, Tobias
On Tue, Jan 17, 2017 at 02:44:28PM +0100, Tobias Rapp wrote: > On 13.01.2017 17:42, Michael Niedermayer wrote: > >On Tue, Jan 10, 2017 at 09:26:53AM +0100, Tobias Rapp wrote: > >>On 10.01.2017 00:19, Michael Niedermayer wrote: > >>> [...] > >>>isnt it better to warn the user and inform him at the end what size > >>>of reserved space would have been needed? > >>> > >>>not saying iam against this, i do see how it is formally correct to > >>>fail hard but it feels painfull to me to fail > >>>muxing at 256gb hard when we can perfectly fine just continue and > >>>the user can just remux the file with bigger reserved master index > >>>to fix it > >> > >>But then adding "-strict strict" just enables a warning message and > >>a non-compliant file is written still? Or do you mean that > >>additionally a warning message could be written in case > >>std_compliance is <= normal? > >> > >>Background story: I'm working on an application that fetches some > >>AVI file byte range from tape storage and restores the file header. > >>The header restoration requires the ODML master index in the header > >>to be complete. Thus I'd like to ensure that all AVI files put into > >>the archive have a compliant/compatible index structure. > >> > >>For such automated processes failing hard is more safe than > >>continuing with a warning message, checking for log messages in the > >>host application and possibly re-running the transcoding with > >>adapted options. > > > >whats your oppinion on an option that selects muxer error behavior ? > >something that can > >fail immedeatly on error or > >warn but continue, return failure after writing the file and trailer > > > >that is a option to make it possible for a muxer to continue even > >if there was an error, maybe even a int max_muxer_error or something > >else? > > You mean something like extending the current use of > error_recognition and AV_EF_EXPLODE to encoders/muxers? In general no, the semantic meaning is different recogniing errors in the input material and recognizing errors in the muxer "implementation" in relation to the used input or other are not the same thing. They may have different values, different defaults, and would have different explanation in the docs > it might be a good idea but I don't see how it helps in the current > case. well, it gives the user the option d) write a working file with chained master index and fail at trailer writing time to indicate that its not spec compliant but loose no data. The use case is anything that has non recoverable input, anything that cannot be re-run with a larger reserved space > > When all entries of the master index are exhausted and another entry > is to be added the basic options are: > a) write a chained master index (currently implemented) > b) enlarge the master index (currently not implemented and I won't > volunteer for doing it) > c) fail > [...]
On 17.01.2017 19:10, Michael Niedermayer wrote: > On Tue, Jan 17, 2017 at 02:44:28PM +0100, Tobias Rapp wrote: >> On 13.01.2017 17:42, Michael Niedermayer wrote: >>> On Tue, Jan 10, 2017 at 09:26:53AM +0100, Tobias Rapp wrote: >>>> On 10.01.2017 00:19, Michael Niedermayer wrote: >>>>> [...] >>>>> isnt it better to warn the user and inform him at the end what size >>>>> of reserved space would have been needed? >>>>> >>>>> not saying iam against this, i do see how it is formally correct to >>>>> fail hard but it feels painfull to me to fail >>>>> muxing at 256gb hard when we can perfectly fine just continue and >>>>> the user can just remux the file with bigger reserved master index >>>>> to fix it >>>> >>>> But then adding "-strict strict" just enables a warning message and >>>> a non-compliant file is written still? Or do you mean that >>>> additionally a warning message could be written in case >>>> std_compliance is <= normal? >>>> >>>> Background story: I'm working on an application that fetches some >>>> AVI file byte range from tape storage and restores the file header. >>>> The header restoration requires the ODML master index in the header >>>> to be complete. Thus I'd like to ensure that all AVI files put into >>>> the archive have a compliant/compatible index structure. >>>> >>>> For such automated processes failing hard is more safe than >>>> continuing with a warning message, checking for log messages in the >>>> host application and possibly re-running the transcoding with >>>> adapted options. >>> >>> whats your oppinion on an option that selects muxer error behavior ? >>> something that can >>> fail immedeatly on error or >>> warn but continue, return failure after writing the file and trailer >>> >>> that is a option to make it possible for a muxer to continue even >>> if there was an error, maybe even a int max_muxer_error or something >>> else? >> > >> You mean something like extending the current use of >> error_recognition and AV_EF_EXPLODE to encoders/muxers? In general > > no, the semantic meaning is different > recogniing errors in the input material and recognizing errors in the > muxer "implementation" in relation to the used input or other are not > the same thing. > They may have different values, different defaults, and would have > different explanation in the docs > >> it might be a good idea but I don't see how it helps in the current >> case. > > well, it gives the user the option > d) write a working file with chained master index and fail at trailer > writing time to indicate that its not spec compliant but loose no > data. The use case is anything that has non recoverable input, > anything that cannot be re-run with a larger reserved space I agree this would be better e.g. when recording live data coming from an input device. Would the indication be done by some special return code "AV_PROBLEM_BUT_CONTINUE" on API level? Or maybe a separate error reporting field/buffer within the AVFormatContext struct? Trying to understand how it would integrate into the host application (ffmpeg/other) ... Anyway, will send version 3 of the patch series with this patch excluded. > [...] Regards, Tobias
On Wed, Jan 18, 2017 at 10:18:36AM +0100, Tobias Rapp wrote: > On 17.01.2017 19:10, Michael Niedermayer wrote: > >On Tue, Jan 17, 2017 at 02:44:28PM +0100, Tobias Rapp wrote: > >>On 13.01.2017 17:42, Michael Niedermayer wrote: > >>>On Tue, Jan 10, 2017 at 09:26:53AM +0100, Tobias Rapp wrote: > >>>>On 10.01.2017 00:19, Michael Niedermayer wrote: > >>>>>[...] > >>>>>isnt it better to warn the user and inform him at the end what size > >>>>>of reserved space would have been needed? > >>>>> > >>>>>not saying iam against this, i do see how it is formally correct to > >>>>>fail hard but it feels painfull to me to fail > >>>>>muxing at 256gb hard when we can perfectly fine just continue and > >>>>>the user can just remux the file with bigger reserved master index > >>>>>to fix it > >>>> > >>>>But then adding "-strict strict" just enables a warning message and > >>>>a non-compliant file is written still? Or do you mean that > >>>>additionally a warning message could be written in case > >>>>std_compliance is <= normal? > >>>> > >>>>Background story: I'm working on an application that fetches some > >>>>AVI file byte range from tape storage and restores the file header. > >>>>The header restoration requires the ODML master index in the header > >>>>to be complete. Thus I'd like to ensure that all AVI files put into > >>>>the archive have a compliant/compatible index structure. > >>>> > >>>>For such automated processes failing hard is more safe than > >>>>continuing with a warning message, checking for log messages in the > >>>>host application and possibly re-running the transcoding with > >>>>adapted options. > >>> > >>>whats your oppinion on an option that selects muxer error behavior ? > >>>something that can > >>>fail immedeatly on error or > >>>warn but continue, return failure after writing the file and trailer > >>> > >>>that is a option to make it possible for a muxer to continue even > >>>if there was an error, maybe even a int max_muxer_error or something > >>>else? > >> > > > >>You mean something like extending the current use of > >>error_recognition and AV_EF_EXPLODE to encoders/muxers? In general > > > >no, the semantic meaning is different > >recogniing errors in the input material and recognizing errors in the > >muxer "implementation" in relation to the used input or other are not > >the same thing. > >They may have different values, different defaults, and would have > >different explanation in the docs > > > >>it might be a good idea but I don't see how it helps in the current > >>case. > > > >well, it gives the user the option > >d) write a working file with chained master index and fail at trailer > >writing time to indicate that its not spec compliant but loose no > >data. The use case is anything that has non recoverable input, > >anything that cannot be re-run with a larger reserved space > > I agree this would be better e.g. when recording live data coming > from an input device. Would the indication be done by some special > return code "AV_PROBLEM_BUT_CONTINUE" on API level? Or maybe a > separate error reporting field/buffer within the AVFormatContext > struct? Trying to understand how it would integrate into the host > application (ffmpeg/other) ... what i was thinking about would when enabled not return an error before the end, but a AV_PROBLEM_BUT_CONTINUE would be an option too [...]
diff --git a/libavformat/avienc.c b/libavformat/avienc.c index fd16fff..5d5c02a 100644 --- a/libavformat/avienc.c +++ b/libavformat/avienc.c @@ -524,7 +524,13 @@ static int avi_write_header(AVFormatContext *s) return 0; } -static void update_odml_entry(AVFormatContext *s, int stream_index, int64_t ix, int size) +/* Index entry points to standard index */ +#define AVI_UPDATE_ODML_ENTRY_DEFAULT 0x00000000 + +/* Index entry points to another master index */ +#define AVI_UPDATE_ODML_ENTRY_MASTER 0x00000001 + +static void update_odml_entry(AVFormatContext *s, int stream_index, int64_t ix, int size, int flags) { AVIOContext *pb = s->pb; AVIContext *avi = s->priv_data; @@ -544,7 +550,10 @@ static void update_odml_entry(AVFormatContext *s, int stream_index, int64_t ix, avio_wl64(pb, ix); /* qwOffset */ avio_wl32(pb, size); /* dwSize */ ff_parse_specific_params(s->streams[stream_index], &au_byterate, &au_ssize, &au_scale); - if (s->streams[stream_index]->codecpar->codec_type == AVMEDIA_TYPE_AUDIO && au_ssize > 0) { + if (flags & AVI_UPDATE_ODML_ENTRY_MASTER) { + av_assert0(s->strict_std_compliance <= FF_COMPLIANCE_NORMAL); + avio_wl32(pb, -1); /* dwDuration (undefined) */ + } else if (s->streams[stream_index]->codecpar->codec_type == AVMEDIA_TYPE_AUDIO && au_ssize > 0) { uint32_t audio_segm_size = (avist->audio_strm_length - avist->indexes.audio_strm_offset); if ((audio_segm_size % au_ssize > 0) && !avist->sample_requested) { avpriv_request_sample(s, "OpenDML index duration for audio packets with partial frames"); @@ -567,6 +576,12 @@ 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); + 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) { @@ -574,7 +589,7 @@ static int avi_write_ix(AVFormatContext *s) int size = 8+2+1+1+4+8+4+4+16*AVI_MASTER_INDEX_SIZE; pos = avio_tell(pb); - update_odml_entry(s, i, pos, size); + update_odml_entry(s, i, pos, size, AVI_UPDATE_ODML_ENTRY_MASTER); write_odml_master(s, i); av_assert1(avio_tell(pb) - pos == size); avist->indexes.master_odml_riff_id_base = avi->riff_id - 1; @@ -610,7 +625,7 @@ static int avi_write_ix(AVFormatContext *s) (ie->flags & 0x10 ? 0 : 0x80000000)); } - update_odml_entry(s, i, ix, avio_tell(pb) - ix); + update_odml_entry(s, i, ix, avio_tell(pb) - ix, AVI_UPDATE_ODML_ENTRY_DEFAULT); } return 0; } @@ -801,6 +816,7 @@ static int avi_write_packet_internal(AVFormatContext *s, AVPacket *pkt) AVIOContext *pb = s->pb; AVIStream *avist = s->streams[stream_index]->priv_data; AVCodecParameters *par = s->streams[stream_index]->codecpar; + int ret; if (pkt->dts != AV_NOPTS_VALUE) avist->last_dts = pkt->dts + pkt->duration; @@ -810,7 +826,8 @@ static int avi_write_packet_internal(AVFormatContext *s, AVPacket *pkt) // Make sure to put an OpenDML chunk when the file size exceeds the limits if (pb->seekable && (avio_tell(pb) - avi->riff_start > AVI_MAX_RIFF_SIZE)) { - avi_write_ix(s); + if ((ret = avi_write_ix(s)) < 0) + return ret; ff_end_tag(pb, avi->movi_list); if (avi->riff_id == 1) @@ -827,7 +844,6 @@ static int avi_write_packet_internal(AVFormatContext *s, AVPacket *pkt) avist->audio_strm_length += size; if (s->pb->seekable) { - int ret; ret = avi_add_ientry(s, stream_index, NULL, flags, size); if (ret < 0) return ret; @@ -861,7 +877,7 @@ static int avi_write_trailer(AVFormatContext *s) res = avi_write_idx1(s); ff_end_tag(pb, avi->riff_start); } else { - avi_write_ix(s); + res = avi_write_ix(s); ff_end_tag(pb, avi->movi_list); ff_end_tag(pb, avi->riff_start);
OpenDML specification v1.02 states that entries of a master index chunk shall point to standard or field index chunks. Changed incorrect duration of last master index entry to -1 in case it points to another master index. Propagate error when index write fails. Signed-off-by: Tobias Rapp <t.rapp@noa-archive.com> --- libavformat/avienc.c | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-)