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

[Marton Balint]

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?

Thanks,
Marton