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

["\"zhilizhao(赵志立)\"" <quinkblack@foxmail.com>]

> 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.

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".