Re: [FFmpeg-devel] [PATCH 2/3] avformat/mov: skip hoov box if strict >= normal

[Steven Liu <lingjiujianke@gmail.com>]

"zhilizhao(赵志立)" <quinkblack@foxmail.com> 于2021年12月24日周五 18:38写道：
>
>
>
> > On Dec 24, 2021, at 6:19 PM, Steven Liu <lq@chinaffmpeg.org> wrote:
> >
> >> 2021年12月24日 下午5:58，Zhao Zhili <quinkblack@foxmail.com> 写道：
> >>
> >> The samples I have got have hoov and moov both. Unknown boxes
> >> should be skipped according to the spec. So don't treat hoov as
> >> moov in normal mode.
> >>
> >> For backward compatible, a log message has been added to notice
> >> the user to relax the striction if moov doesn't exist.
> >>
> >> Fix #8883.
> >> ---
> >> libavformat/mov.c | 13 +++++++++----
> >> 1 file changed, 9 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/libavformat/mov.c b/libavformat/mov.c
> >> index ea2f010aa0..63483740a0 100644
> >> --- a/libavformat/mov.c
> >> +++ b/libavformat/mov.c
> >> @@ -7324,10 +7324,11 @@ static int mov_read_default(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> >>        if (atom.size >= 8) {
> >>            a.size = avio_rb32(pb);
> >>            a.type = avio_rl32(pb);
> >> -            if (((a.type == MKTAG('f','r','e','e') && c->moov_retry) ||
> >> -                  a.type == MKTAG('h','o','o','v')) &&
> >> -                a.size >= 8 &&
> >> -                c->fc->strict_std_compliance < FF_COMPLIANCE_STRICT) {
> >> +            if (((a.type == MKTAG('f','r','e','e') && c->moov_retry &&
> >> +                        c->fc->strict_std_compliance < FF_COMPLIANCE_STRICT) ||
> >> +                 (a.type == MKTAG('h','o','o','v') &&
> >> +                        c->fc->strict_std_compliance < FF_COMPLIANCE_NORMAL)) &&
> >> +                a.size >= 8) {
> >
> > I prefer add one option ignore_hoov, let user choose ignore it, because it should more clearly than use stirct.
>
> Add an option works for me too, but it should default to ignore hoov, because:
> 1. Truncated/broken files are minority compared to normal files (I hope);
> 2. Support normal files has higher priority than workaround broken files.
>
I think about ignore hoov should not default, because it use hoov
default before.
We should attention users pay attention about this change maybe do it
like deprecate warning. Just waning need change default it ignore hoov
and use moov.
But I’m not sure ignore hoov is correct, because not sure user have
the sample file which only have one hoov and have no moov atom in the
files.

> We can support both by drop fake moov info after found a real moov, but
> it doesn’t deserve the complexity.
>
> I’d like to get more opinions.
>
>
> >>                uint32_t type;
> >>                avio_skip(pb, 4);
> >>                type = avio_rl32(pb);
> >> @@ -7340,6 +7341,10 @@ static int mov_read_default(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> >>                    a.type = MKTAG('m','o','o','v');
> >>                }
> >>            }
> >> +            if (a.type == MKTAG('h','o','o','v') &&
> >> +                c->fc->strict_std_compliance >= FF_COMPLIANCE_NORMAL)
> >> +                av_log(c->fc, AV_LOG_INFO,
> >> +                        "Skip hoov atom, try decrease -strict if moov doesn't exist.\n");
> >>            if (atom.type != MKTAG('r','o','o','t') &&
> >>                atom.type != MKTAG('m','o','o','v')) {
> >>                if (a.type == MKTAG('t','r','a','k') ||
> >> --
> >> 2.31.1
> >>
> >> _______________________________________________
> >> 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
> >
> >> 2021年12月24日 下午5:58，Zhao Zhili <quinkblack@foxmail.com> 写道：
> >>
> >> The samples I have got have hoov and moov both. Unknown boxes
> >> should be skipped according to the spec. So don't treat hoov as
> >> moov in normal mode.
> >>
> >> For backward compatible, a log message has been added to notice
> >> the user to relax the striction if moov doesn't exist.
> >>
> >> Fix #8883.
> >> ---
> >> libavformat/mov.c | 13 +++++++++----
> >> 1 file changed, 9 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/libavformat/mov.c b/libavformat/mov.c
> >> index ea2f010aa0..63483740a0 100644
> >> --- a/libavformat/mov.c
> >> +++ b/libavformat/mov.c
> >> @@ -7324,10 +7324,11 @@ static int mov_read_default(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> >>        if (atom.size >= 8) {
> >>            a.size = avio_rb32(pb);
> >>            a.type = avio_rl32(pb);
> >> -            if (((a.type == MKTAG('f','r','e','e') && c->moov_retry) ||
> >> -                  a.type == MKTAG('h','o','o','v')) &&
> >> -                a.size >= 8 &&
> >> -                c->fc->strict_std_compliance < FF_COMPLIANCE_STRICT) {
> >> +            if (((a.type == MKTAG('f','r','e','e') && c->moov_retry &&
> >> +                        c->fc->strict_std_compliance < FF_COMPLIANCE_STRICT) ||
> >> +                 (a.type == MKTAG('h','o','o','v') &&
> >> +                        c->fc->strict_std_compliance < FF_COMPLIANCE_NORMAL)) &&
> >> +                a.size >= 8) {
> >
> > I prefer add one option ignore_hoov, let user choose ignore it, because it should more clearly than use stirct.
> >>                uint32_t type;
> >>                avio_skip(pb, 4);
> >>                type = avio_rl32(pb);
> >> @@ -7340,6 +7341,10 @@ static int mov_read_default(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> >>                    a.type = MKTAG('m','o','o','v');
> >>                }
> >>            }
> >> +            if (a.type == MKTAG('h','o','o','v') &&
> >> +                c->fc->strict_std_compliance >= FF_COMPLIANCE_NORMAL)
> >> +                av_log(c->fc, AV_LOG_INFO,
> >> +                        "Skip hoov atom, try decrease -strict if moov doesn't exist.\n");
> >>            if (atom.type != MKTAG('r','o','o','t') &&
> >>                atom.type != MKTAG('m','o','o','v')) {
> >>                if (a.type == MKTAG('t','r','a','k') ||
> >> --
> >> 2.31.1
> >>
> >> _______________________________________________
> >> 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".

Thanks
Steven
_______________________________________________
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".