diff mbox series

[FFmpeg-devel] avformat/mov: add ignore_hoov option to ignore hoov atom

Message ID 20211223071012.52638-1-lq@chinaffmpeg.org
State New
Headers show
Series [FFmpeg-devel] avformat/mov: add ignore_hoov option to ignore hoov atom | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 fail Make fate failed
andriy/make_ppc success Make finished
andriy/make_fate_ppc fail Make fate failed

Commit Message

Steven Liu Dec. 23, 2021, 7:10 a.m. UTC
Try to get context from the moov atom when the hoov before moov atom,
because the streams info get a possible incorrect when there have both
hoov and moov atom. So add and ignore_hoov option for try to get
moov context by user.

Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
---
 doc/demuxers.texi  |  6 ++++++
 libavformat/isom.h |  2 ++
 libavformat/mov.c  | 14 ++++++++++++--
 3 files changed, 20 insertions(+), 2 deletions(-)

Comments

zhilizhao(赵志立) Dec. 24, 2021, 3:43 a.m. UTC | #1
> On Dec 23, 2021, at 3:10 PM, Steven Liu <lq@chinaffmpeg.org> wrote:
> 
> Try to get context from the moov atom when the hoov before moov atom,
> because the streams info get a possible incorrect when there have both
> hoov and moov atom. So add and ignore_hoov option for try to get
> moov context by user.
> 
> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> ---
> doc/demuxers.texi  |  6 ++++++
> libavformat/isom.h |  2 ++
> libavformat/mov.c  | 14 ++++++++++++--
> 3 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/demuxers.texi b/doc/demuxers.texi
> index cab8a7072c..d23c118210 100644
> --- a/doc/demuxers.texi
> +++ b/doc/demuxers.texi
> @@ -660,6 +660,11 @@ to demuxing linearly from the beginning. Default is true.
> Ignore any edit list atoms. The demuxer, by default, modifies the stream index to reflect the
> timeline described by the edit list. Default is false.
> 
> +@item ignore_hoov
> +Ignore hoov atom. The demuxer, by default, use hoov atom when there have hoov before moov atom.
> +You can try ignore hoov atom try the moov atom.
> +Default is false.
> +
> @item advanced_editlist
> Modify the stream index to reflect the timeline described by the edit list. @code{ignore_editlist}
> must be set to false for this option to be effective.
> diff --git a/libavformat/isom.h b/libavformat/isom.h
> index ef8f19b18c..6aa2b40c94 100644
> --- a/libavformat/isom.h
> +++ b/libavformat/isom.h
> @@ -255,6 +255,7 @@ typedef struct MOVContext {
>     AVFormatContext *fc;
>     int time_scale;
>     int64_t duration;     ///< duration of the longest track
> +    int found_hoov;       ///< 'hoov' atom has been found
>     int found_moov;       ///< 'moov' atom has been found
>     int found_mdat;       ///< 'mdat' atom has been found
>     int found_hdlr_mdta;  ///< 'hdlr' atom with type 'mdta' has been found
> @@ -273,6 +274,7 @@ typedef struct MOVContext {
>     unsigned int nb_chapter_tracks;
>     int use_absolute_path;
>     int ignore_editlist;
> +    int ignore_hoov;
>     int advanced_editlist;
>     int ignore_chapters;
>     int seek_individually;
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 2aed6e80ef..cd775b219f 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -1159,8 +1159,11 @@ static int mov_read_moov(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> 
>     if (c->found_moov) {
>         av_log(c->fc, AV_LOG_WARNING, "Found duplicated MOOV Atom. Skipped it\n");
> -        avio_skip(pb, atom.size);
> -        return 0;
> +        if (!c->found_hoov) {
> +            avio_skip(pb, atom.size);
> +            return 0;
> +        }
> +        c->found_hoov = 0;
>     }
> 
>     if ((ret = mov_read_default(c, pb, atom)) < 0)
> @@ -4239,6 +4242,9 @@ static int mov_read_trak(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>     MOVStreamContext *sc;
>     int ret;
> 
> +    if (c->found_hoov)
> +        return 0;
> +

Does it break files with only hoov boxes?

>     st = avformat_new_stream(c->fc, NULL);
>     if (!st) return AVERROR(ENOMEM);
>     st->id = -1;
> @@ -7329,6 +7335,8 @@ static int mov_read_default(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>                 a.size >= 8 &&
>                 c->fc->strict_std_compliance < FF_COMPLIANCE_STRICT) {
>                 uint32_t type;
> +                if (c->ignore_hoov)
> +                    c->found_hoov = 1;
>                 avio_skip(pb, 4);
>                 type = avio_rl32(pb);
>                 if (avio_feof(pb))
> @@ -8541,6 +8549,8 @@ static const AVOption mov_options[] = {
>         0, 1, FLAGS},
>     {"ignore_editlist", "Ignore the edit list atom.", OFFSET(ignore_editlist), AV_OPT_TYPE_BOOL, {.i64 = 0},
>         0, 1, FLAGS},
> +    {"ignore_hoov", "Ignore the hoov atom.", OFFSET(ignore_hoov), AV_OPT_TYPE_BOOL, {.i64 = 0},
> +        0, 1, FLAGS},

TBH, add an option is too much effort to workaround such broken files. I prefer

a.type == MKTAG('h','o','o','v') && c->fc->strict_std_compliance < FF_COMPLIANCE_UNOFFICIAL

But it will break those samples if user doesn't lower strict_std_compliance.

>     {"advanced_editlist",
>         "Modify the AVIndex according to the editlists. Use this option to decode in the order specified by the edits.",
>         OFFSET(advanced_editlist), AV_OPT_TYPE_BOOL, {.i64 = 1},
> -- 
> 2.25.0
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Steven Liu Dec. 24, 2021, 6:19 a.m. UTC | #2
> 2021年12月24日 上午11:43,zhilizhao(赵志立) <quinkblack@foxmail.com> 写道:
> 
> 
> 
>> On Dec 23, 2021, at 3:10 PM, Steven Liu <lq@chinaffmpeg.org> wrote:
>> 
>> Try to get context from the moov atom when the hoov before moov atom,
>> because the streams info get a possible incorrect when there have both
>> hoov and moov atom. So add and ignore_hoov option for try to get
>> moov context by user.
>> 
>> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
>> ---
>> doc/demuxers.texi  |  6 ++++++
>> libavformat/isom.h |  2 ++
>> libavformat/mov.c  | 14 ++++++++++++--
>> 3 files changed, 20 insertions(+), 2 deletions(-)
>> 
>> diff --git a/doc/demuxers.texi b/doc/demuxers.texi
>> index cab8a7072c..d23c118210 100644
>> --- a/doc/demuxers.texi
>> +++ b/doc/demuxers.texi
>> @@ -660,6 +660,11 @@ to demuxing linearly from the beginning. Default is true.
>> Ignore any edit list atoms. The demuxer, by default, modifies the stream index to reflect the
>> timeline described by the edit list. Default is false.
>> 
>> +@item ignore_hoov
>> +Ignore hoov atom. The demuxer, by default, use hoov atom when there have hoov before moov atom.
>> +You can try ignore hoov atom try the moov atom.
>> +Default is false.
>> +
>> @item advanced_editlist
>> Modify the stream index to reflect the timeline described by the edit list. @code{ignore_editlist}
>> must be set to false for this option to be effective.
>> diff --git a/libavformat/isom.h b/libavformat/isom.h
>> index ef8f19b18c..6aa2b40c94 100644
>> --- a/libavformat/isom.h
>> +++ b/libavformat/isom.h
>> @@ -255,6 +255,7 @@ typedef struct MOVContext {
>>    AVFormatContext *fc;
>>    int time_scale;
>>    int64_t duration;     ///< duration of the longest track
>> +    int found_hoov;       ///< 'hoov' atom has been found
>>    int found_moov;       ///< 'moov' atom has been found
>>    int found_mdat;       ///< 'mdat' atom has been found
>>    int found_hdlr_mdta;  ///< 'hdlr' atom with type 'mdta' has been found
>> @@ -273,6 +274,7 @@ typedef struct MOVContext {
>>    unsigned int nb_chapter_tracks;
>>    int use_absolute_path;
>>    int ignore_editlist;
>> +    int ignore_hoov;
>>    int advanced_editlist;
>>    int ignore_chapters;
>>    int seek_individually;
>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>> index 2aed6e80ef..cd775b219f 100644
>> --- a/libavformat/mov.c
>> +++ b/libavformat/mov.c
>> @@ -1159,8 +1159,11 @@ static int mov_read_moov(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>> 
>>    if (c->found_moov) {
>>        av_log(c->fc, AV_LOG_WARNING, "Found duplicated MOOV Atom. Skipped it\n");
>> -        avio_skip(pb, atom.size);
>> -        return 0;
>> +        if (!c->found_hoov) {
>> +            avio_skip(pb, atom.size);
>> +            return 0;
>> +        }
>> +        c->found_hoov = 0;
>>    }
>> 
>>    if ((ret = mov_read_default(c, pb, atom)) < 0)
>> @@ -4239,6 +4242,9 @@ static int mov_read_trak(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>>    MOVStreamContext *sc;
>>    int ret;
>> 
>> +    if (c->found_hoov)
>> +        return 0;
>> +
> 
> Does it break files with only hoov boxes?
No by default. And Yes if have both hoov and moov.
Yes it should break if user use ignore_hoov option.
Add ignore_hoov can be choosed by users.

>  
>>    st = avformat_new_stream(c->fc, NULL);
>>    if (!st) return AVERROR(ENOMEM);
>>    st->id = -1;
>> @@ -7329,6 +7335,8 @@ static int mov_read_default(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>>                a.size >= 8 &&
>>                c->fc->strict_std_compliance < FF_COMPLIANCE_STRICT) {
>>                uint32_t type;
>> +                if (c->ignore_hoov)
>> +                    c->found_hoov = 1;
>>                avio_skip(pb, 4);
>>                type = avio_rl32(pb);
>>                if (avio_feof(pb))
>> @@ -8541,6 +8549,8 @@ static const AVOption mov_options[] = {
>>        0, 1, FLAGS},
>>    {"ignore_editlist", "Ignore the edit list atom.", OFFSET(ignore_editlist), AV_OPT_TYPE_BOOL, {.i64 = 0},
>>        0, 1, FLAGS},
>> +    {"ignore_hoov", "Ignore the hoov atom.", OFFSET(ignore_hoov), AV_OPT_TYPE_BOOL, {.i64 = 0},
>> +        0, 1, FLAGS},
> 
> TBH, add an option is too much effort to workaround such broken files. I prefer
> 
> a.type == MKTAG('h','o','o','v') && c->fc->strict_std_compliance < FF_COMPLIANCE_UNOFFICIAL
> 
> But it will break those samples if user doesn't lower strict_std_compliance.
> 
>>    {"advanced_editlist",
>>        "Modify the AVIndex according to the editlists. Use this option to decode in the order specified by the edits.",
>>        OFFSET(advanced_editlist), AV_OPT_TYPE_BOOL, {.i64 = 1},
>> -- 
>> 2.25.0
>> 
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Thanks

Steven Liu
Steven Liu Dec. 30, 2021, 7:42 a.m. UTC | #3
Steven Liu <lq@chinaffmpeg.org> 于2021年12月24日周五 14:20写道:
>
>
>
> > 2021年12月24日 上午11:43,zhilizhao(赵志立) <quinkblack@foxmail.com> 写道:
> >
> >
> >
> >> On Dec 23, 2021, at 3:10 PM, Steven Liu <lq@chinaffmpeg.org> wrote:
> >>
> >> Try to get context from the moov atom when the hoov before moov atom,
> >> because the streams info get a possible incorrect when there have both
> >> hoov and moov atom. So add and ignore_hoov option for try to get
> >> moov context by user.
> >>
> >> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> >> ---
> >> doc/demuxers.texi  |  6 ++++++
> >> libavformat/isom.h |  2 ++
> >> libavformat/mov.c  | 14 ++++++++++++--
> >> 3 files changed, 20 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/doc/demuxers.texi b/doc/demuxers.texi
> >> index cab8a7072c..d23c118210 100644
> >> --- a/doc/demuxers.texi
> >> +++ b/doc/demuxers.texi
> >> @@ -660,6 +660,11 @@ to demuxing linearly from the beginning. Default is true.
> >> Ignore any edit list atoms. The demuxer, by default, modifies the stream index to reflect the
> >> timeline described by the edit list. Default is false.
> >>
> >> +@item ignore_hoov
> >> +Ignore hoov atom. The demuxer, by default, use hoov atom when there have hoov before moov atom.
> >> +You can try ignore hoov atom try the moov atom.
> >> +Default is false.
> >> +
> >> @item advanced_editlist
> >> Modify the stream index to reflect the timeline described by the edit list. @code{ignore_editlist}
> >> must be set to false for this option to be effective.
> >> diff --git a/libavformat/isom.h b/libavformat/isom.h
> >> index ef8f19b18c..6aa2b40c94 100644
> >> --- a/libavformat/isom.h
> >> +++ b/libavformat/isom.h
> >> @@ -255,6 +255,7 @@ typedef struct MOVContext {
> >>    AVFormatContext *fc;
> >>    int time_scale;
> >>    int64_t duration;     ///< duration of the longest track
> >> +    int found_hoov;       ///< 'hoov' atom has been found
> >>    int found_moov;       ///< 'moov' atom has been found
> >>    int found_mdat;       ///< 'mdat' atom has been found
> >>    int found_hdlr_mdta;  ///< 'hdlr' atom with type 'mdta' has been found
> >> @@ -273,6 +274,7 @@ typedef struct MOVContext {
> >>    unsigned int nb_chapter_tracks;
> >>    int use_absolute_path;
> >>    int ignore_editlist;
> >> +    int ignore_hoov;
> >>    int advanced_editlist;
> >>    int ignore_chapters;
> >>    int seek_individually;
> >> diff --git a/libavformat/mov.c b/libavformat/mov.c
> >> index 2aed6e80ef..cd775b219f 100644
> >> --- a/libavformat/mov.c
> >> +++ b/libavformat/mov.c
> >> @@ -1159,8 +1159,11 @@ static int mov_read_moov(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> >>
> >>    if (c->found_moov) {
> >>        av_log(c->fc, AV_LOG_WARNING, "Found duplicated MOOV Atom. Skipped it\n");
> >> -        avio_skip(pb, atom.size);
> >> -        return 0;
> >> +        if (!c->found_hoov) {
> >> +            avio_skip(pb, atom.size);
> >> +            return 0;
> >> +        }
> >> +        c->found_hoov = 0;
> >>    }
> >>
> >>    if ((ret = mov_read_default(c, pb, atom)) < 0)
> >> @@ -4239,6 +4242,9 @@ static int mov_read_trak(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> >>    MOVStreamContext *sc;
> >>    int ret;
> >>
> >> +    if (c->found_hoov)
> >> +        return 0;
> >> +
> >
> > Does it break files with only hoov boxes?
> No by default. And Yes if have both hoov and moov.
> Yes it should break if user use ignore_hoov option.
> Add ignore_hoov can be choosed by users.
>
> >
> >>    st = avformat_new_stream(c->fc, NULL);
> >>    if (!st) return AVERROR(ENOMEM);
> >>    st->id = -1;
> >> @@ -7329,6 +7335,8 @@ static int mov_read_default(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> >>                a.size >= 8 &&
> >>                c->fc->strict_std_compliance < FF_COMPLIANCE_STRICT) {
> >>                uint32_t type;
> >> +                if (c->ignore_hoov)
> >> +                    c->found_hoov = 1;
> >>                avio_skip(pb, 4);
> >>                type = avio_rl32(pb);
> >>                if (avio_feof(pb))
> >> @@ -8541,6 +8549,8 @@ static const AVOption mov_options[] = {
> >>        0, 1, FLAGS},
> >>    {"ignore_editlist", "Ignore the edit list atom.", OFFSET(ignore_editlist), AV_OPT_TYPE_BOOL, {.i64 = 0},
> >>        0, 1, FLAGS},
> >> +    {"ignore_hoov", "Ignore the hoov atom.", OFFSET(ignore_hoov), AV_OPT_TYPE_BOOL, {.i64 = 0},
> >> +        0, 1, FLAGS},
> >
> > TBH, add an option is too much effort to workaround such broken files. I prefer
> >
> > a.type == MKTAG('h','o','o','v') && c->fc->strict_std_compliance < FF_COMPLIANCE_UNOFFICIAL
> >
> > But it will break those samples if user doesn't lower strict_std_compliance.
> >
> >>    {"advanced_editlist",
> >>        "Modify the AVIndex according to the editlists. Use this option to decode in the order specified by the edits.",
> >>        OFFSET(advanced_editlist), AV_OPT_TYPE_BOOL, {.i64 = 1},
> >> --
> >> 2.25.0
> >>
> >> _______________________________________________
> >> ffmpeg-devel mailing list
> >> ffmpeg-devel@ffmpeg.org
> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>
> >> To unsubscribe, visit link above, or email
> >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
> Thanks
>
> Steven Liu
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
ping
diff mbox series

Patch

diff --git a/doc/demuxers.texi b/doc/demuxers.texi
index cab8a7072c..d23c118210 100644
--- a/doc/demuxers.texi
+++ b/doc/demuxers.texi
@@ -660,6 +660,11 @@  to demuxing linearly from the beginning. Default is true.
 Ignore any edit list atoms. The demuxer, by default, modifies the stream index to reflect the
 timeline described by the edit list. Default is false.
 
+@item ignore_hoov
+Ignore hoov atom. The demuxer, by default, use hoov atom when there have hoov before moov atom.
+You can try ignore hoov atom try the moov atom.
+Default is false.
+
 @item advanced_editlist
 Modify the stream index to reflect the timeline described by the edit list. @code{ignore_editlist}
 must be set to false for this option to be effective.
diff --git a/libavformat/isom.h b/libavformat/isom.h
index ef8f19b18c..6aa2b40c94 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -255,6 +255,7 @@  typedef struct MOVContext {
     AVFormatContext *fc;
     int time_scale;
     int64_t duration;     ///< duration of the longest track
+    int found_hoov;       ///< 'hoov' atom has been found
     int found_moov;       ///< 'moov' atom has been found
     int found_mdat;       ///< 'mdat' atom has been found
     int found_hdlr_mdta;  ///< 'hdlr' atom with type 'mdta' has been found
@@ -273,6 +274,7 @@  typedef struct MOVContext {
     unsigned int nb_chapter_tracks;
     int use_absolute_path;
     int ignore_editlist;
+    int ignore_hoov;
     int advanced_editlist;
     int ignore_chapters;
     int seek_individually;
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 2aed6e80ef..cd775b219f 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -1159,8 +1159,11 @@  static int mov_read_moov(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 
     if (c->found_moov) {
         av_log(c->fc, AV_LOG_WARNING, "Found duplicated MOOV Atom. Skipped it\n");
-        avio_skip(pb, atom.size);
-        return 0;
+        if (!c->found_hoov) {
+            avio_skip(pb, atom.size);
+            return 0;
+        }
+        c->found_hoov = 0;
     }
 
     if ((ret = mov_read_default(c, pb, atom)) < 0)
@@ -4239,6 +4242,9 @@  static int mov_read_trak(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     MOVStreamContext *sc;
     int ret;
 
+    if (c->found_hoov)
+        return 0;
+
     st = avformat_new_stream(c->fc, NULL);
     if (!st) return AVERROR(ENOMEM);
     st->id = -1;
@@ -7329,6 +7335,8 @@  static int mov_read_default(MOVContext *c, AVIOContext *pb, MOVAtom atom)
                 a.size >= 8 &&
                 c->fc->strict_std_compliance < FF_COMPLIANCE_STRICT) {
                 uint32_t type;
+                if (c->ignore_hoov)
+                    c->found_hoov = 1;
                 avio_skip(pb, 4);
                 type = avio_rl32(pb);
                 if (avio_feof(pb))
@@ -8541,6 +8549,8 @@  static const AVOption mov_options[] = {
         0, 1, FLAGS},
     {"ignore_editlist", "Ignore the edit list atom.", OFFSET(ignore_editlist), AV_OPT_TYPE_BOOL, {.i64 = 0},
         0, 1, FLAGS},
+    {"ignore_hoov", "Ignore the hoov atom.", OFFSET(ignore_hoov), AV_OPT_TYPE_BOOL, {.i64 = 0},
+        0, 1, FLAGS},
     {"advanced_editlist",
         "Modify the AVIndex according to the editlists. Use this option to decode in the order specified by the edits.",
         OFFSET(advanced_editlist), AV_OPT_TYPE_BOOL, {.i64 = 1},