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

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

> On Dec 25, 2021, at 6:36 PM, Marton Balint <cus@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).
>> +
>> +@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.

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