[FFmpeg-devel] [PATCH 1/3] avformat/avienc: write chained master index only if std_compliance is normal or below

Michael Niedermayer michaelni at gmx.at
Fri Jan 13 18:42:08 EET 2017


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 at 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?


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170113/61fc3aa6/attachment.sig>


More information about the ffmpeg-devel mailing list