[FFmpeg-devel] [PATCH v3 2/2] avformat/mov: add use_hoov option

[Steven Liu]

> 在 2021年12月25日，19:42，zhilizhao(赵志立) <quinkblack at foxmail.com> 写道：
> 
> 
> 
>> On Dec 25, 2021, at 6:36 PM, Marton Balint <cus at passwd.hu> wrote:
>> 
>> On Sat, 25 Dec 2021, Zhao Zhili wrote:
>> 
>>> Fix #8883.
>>> ---
>>> doc/demuxers.texi  | 6 ++++++
>>> libavformat/isom.h | 1 +
>>> libavformat/mov.c  | 9 +++++++--
>>> 3 files changed, 14 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/doc/demuxers.texi b/doc/demuxers.texi
>>> index cab8a7072c..276b536ac5 100644
>>> --- a/doc/demuxers.texi
>>> +++ b/doc/demuxers.texi
>>> @@ -713,6 +713,12 @@ specify.
>>> 
>>> @item decryption_key
>>> 16-byte key, in hex, to decrypt files encrypted using ISO Common Encryption (CENC/AES-128 CTR; ISO/IEC 23001-7).
>>> +
>>> + at item use_hoov
>>> +Whether to use hoov atom as moov atom to support some broken files. Set it to 0 for disable, 1 for enable,
>>> +-1 for automatic, which means take hoov as moov in the second pass if moov doesn't been found in the first
>>> +pass.
>>> +
>>> @end table
>>> 
>>> @subsection Audible AAX
>>> diff --git a/libavformat/isom.h b/libavformat/isom.h
>>> index ef8f19b18c..7c837811a6 100644
>>> --- a/libavformat/isom.h
>>> +++ b/libavformat/isom.h
>>> @@ -305,6 +305,7 @@ typedef struct MOVContext {
>>>   int32_t movie_display_matrix[3][3]; ///< display matrix from mvhd
>>>   int have_read_mfra_size;
>>>   uint32_t mfra_size;
>>> +    int use_hoov;
>>> } MOVContext;
>>> 
>>> int ff_mp4_read_descr_len(AVIOContext *pb);
>>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>>> index ea2f010aa0..82237f930b 100644
>>> --- a/libavformat/mov.c
>>> +++ b/libavformat/mov.c
>>> @@ -7325,7 +7325,7 @@ static int mov_read_default(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>>>           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.type == MKTAG('h','o','o','v') && c->use_hoov == 1) &&
>>>               a.size >= 8 &&
>>>               c->fc->strict_std_compliance < FF_COMPLIANCE_STRICT) {
>>>               uint32_t type;
>>> @@ -7951,8 +7951,12 @@ static int mov_read_header(AVFormatContext *s)
>>> 
>>>   /* check MOV header */
>>>   do {
>>> -        if (mov->moov_retry)
>>> +        if (mov->moov_retry) {
>>>           avio_seek(pb, 0, SEEK_SET);
>>> +            /* reset use_hoov from auto mode to force mode in second pass */
>>> +            if (mov->use_hoov == -1)
>>> +                mov->use_hoov = 1;
>>> +        }
>>>       if ((err = mov_read_default(mov, pb, atom)) < 0) {
>>>           av_log(s, AV_LOG_ERROR, "error reading header\n");
>>>           return err;
>>> @@ -8593,6 +8597,7 @@ static const AVOption mov_options[] = {
>>>   { "decryption_key", "The media decryption key (hex)", OFFSET(decryption_key), AV_OPT_TYPE_BINARY, .flags = AV_OPT_FLAG_DECODING_PARAM },
>>>   { "enable_drefs", "Enable external track support.", OFFSET(enable_drefs), AV_OPT_TYPE_BOOL,
>>>       {.i64 = 0}, 0, 1, FLAGS },
>>> +    { "use_hoov", "Take hoov as moov atom, 0 for disable, -1 for auto, 1 means force", OFFSET(use_hoov), AV_OPT_TYPE_INT, {.i64 = 1}, -1, 1, FLAGS },
>> 
>> Kind of unusual that the default is 1 and not -1 (automatic). Is it intentional? Does it make sense to find hoov atom first and discard moov? Can't that cause problems for valid files? If it can, then maybe -1 should be the default, no?
> 
> I prefer to set it to -1 and handle it automatically, but it may break those files Derek mentioned:
> 
> http://ffmpeg.org/pipermail/ffmpeg-devel/2021-December/290359.html
> 
> So force taking hoov as moov is for backward compatibility. It’s pain to workaround another workaround.
https://ffmpeg.org/pipermail/ffmpeg-devel/2021-December/290245.html

This can fix both problem, and will not modify any original main workflow, and the simplest way,

I think you have change the main workflow to fix one or two bug, too complex.

> 
>> 
>> Thanks,
>> Marton
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".

Thanks
Steven