Re: [FFmpeg-devel] [PATCH 1/4 v3] avformat/mov: ignore item boxes for animated heif

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

James Almer:
> Fixes a regression since d9fed9df2a, where the single animated stream would
> be exported twice as two independent streams.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavformat/isom.h |   1 +
>  libavformat/mov.c  | 147 ++++++++++++++++++++++++++++++---------------
>  2 files changed, 99 insertions(+), 49 deletions(-)
> 
> diff --git a/libavformat/isom.h b/libavformat/isom.h
> index 2cf456fee1..21caaac256 100644
> --- a/libavformat/isom.h
> +++ b/libavformat/isom.h
> @@ -280,6 +280,7 @@ typedef struct MOVContext {
>      int64_t duration;     ///< duration of the longest track
>      int found_moov;       ///< 'moov' atom has been found
>      int found_iloc;       ///< 'iloc' atom has been found
> +    int found_iinf;       ///< 'iinf' atom has been found
>      int found_mdat;       ///< 'mdat' atom has been found
>      int found_hdlr_mdta;  ///< 'hdlr' atom with type 'mdta' has been found
>      int trak_index;       ///< Index of the current 'trak'
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index cf931d4594..af95e1f662 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -81,6 +81,7 @@ typedef struct MOVParseTableEntry {
>  
>  static int mov_read_default(MOVContext *c, AVIOContext *pb, MOVAtom atom);
>  static int mov_read_mfra(MOVContext *c, AVIOContext *f);
> +static void mov_free_stream_context(AVFormatContext *s, AVStream *st);
>  static int64_t add_ctts_entry(MOVCtts** ctts_data, unsigned int* ctts_count, unsigned int* allocated_size,
>                                int count, int duration);
>  
> @@ -4644,6 +4645,23 @@ static int mov_read_trak(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>      MOVStreamContext *sc;
>      int ret;
>  
> +    if (c->found_iinf) {
> +        // * For animated heif, if the iinf box showed up before the moov
> +        //   box, we need to clear all the streams read in the former.
> +        for (int i = c->heif_info_size - 1; i >= 0; i--) {
> +            HEIFItem *item = &c->heif_info[i];
> +
> +            if (!item->st)
> +                continue;
> +
> +            mov_free_stream_context(c->fc, item->st);
> +            ff_remove_stream(c->fc, item->st);
> +        }
> +        av_freep(&c->heif_info);
> +        c->heif_info_size = 0;
> +        c->found_iinf = c->found_iloc = 0;
> +    }
> +
>      st = avformat_new_stream(c->fc, NULL);
>      if (!st) return AVERROR(ENOMEM);
>      st->id = -1;
> @@ -7773,8 +7791,9 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>      uint64_t base_offset, extent_offset, extent_length;
>      uint8_t value;
>  
> -    if (c->found_iloc) {
> -        av_log(c->fc, AV_LOG_INFO, "Duplicate iloc box found\n");
> +    if (c->found_moov) {
> +        // * For animated heif, we don't care about the iloc box as all the
> +        //   necessary information can be found in the moov box.
>          return 0;
>      }
>  
> @@ -7896,6 +7915,16 @@ static int mov_read_iinf(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>      int entry_count;
>      int version, ret;
>  
> +    if (c->found_iinf) {
> +        av_log(c->fc, AV_LOG_WARNING, "Duplicate iinf box found\n");
> +        return 0;
> +    }
> +    if (c->found_moov) {
> +        // * For animated heif, we don't care about the iinf box as all the
> +        //   necessary information can be found in the moov box.
> +        return 0;
> +    }
> +
>      version = avio_r8(pb);
>      avio_rb24(pb);  // flags.
>      entry_count = version ? avio_rb32(pb) : avio_rb16(pb);
> @@ -7919,6 +7948,7 @@ static int mov_read_iinf(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>              return ret;
>      }
>  
> +    c->found_iinf = 1;
>      return 0;
>  }
>  
> @@ -7932,6 +7962,13 @@ static int mov_read_iref(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>  static int mov_read_ispe(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>  {
>      uint32_t width, height;
> +
> +    if (c->found_moov) {
> +        // * For animated heif, we don't care about the ispe box as all the
> +        //   necessary information can be found in the moov box.
> +        return 0;
> +    }
> +
>      avio_r8(pb);  /* version */
>      avio_rb24(pb);  /* flags */
>      width  = avio_rb32(pb);
> @@ -7966,6 +8003,12 @@ static int mov_read_iprp(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>      int version, flags;
>      int ret;
>  
> +    if (c->found_moov) {
> +        // * For animated heif, we don't care about the iprp box as all the
> +        //   necessary information can be found in the moov box.
> +        return 0;
> +    }
> +
>      a.size = avio_rb32(pb);
>      a.type = avio_rl32(pb);
>  
> @@ -8587,6 +8630,58 @@ static void mov_free_encryption_index(MOVEncryptionIndex **index) {
>      av_freep(index);
>  }
>  
> +static void mov_free_stream_context(AVFormatContext *s, AVStream *st)
> +{
> +    MOVStreamContext *sc = st->priv_data;
> +
> +    if (!sc)
> +        return;
> +
> +    av_freep(&sc->ctts_data);
> +    for (int i = 0; i < sc->drefs_count; i++) {
> +        av_freep(&sc->drefs[i].path);
> +        av_freep(&sc->drefs[i].dir);
> +    }
> +    av_freep(&sc->drefs);
> +
> +    sc->drefs_count = 0;
> +
> +    if (!sc->pb_is_copied)
> +        ff_format_io_close(s, &sc->pb);
> +
> +    sc->pb = NULL;
> +    av_freep(&sc->chunk_offsets);
> +    av_freep(&sc->stsc_data);
> +    av_freep(&sc->sample_sizes);
> +    av_freep(&sc->keyframes);
> +    av_freep(&sc->stts_data);
> +    av_freep(&sc->sdtp_data);
> +    av_freep(&sc->stps_data);
> +    av_freep(&sc->elst_data);
> +    av_freep(&sc->rap_group);
> +    av_freep(&sc->sync_group);
> +    av_freep(&sc->sgpd_sync);
> +    av_freep(&sc->sample_offsets);
> +    av_freep(&sc->open_key_samples);
> +    av_freep(&sc->display_matrix);
> +    av_freep(&sc->index_ranges);
> +
> +    if (sc->extradata)
> +        for (int i = 0; i < sc->stsd_count; i++)
> +            av_free(sc->extradata[i]);
> +    av_freep(&sc->extradata);
> +    av_freep(&sc->extradata_size);
> +
> +    mov_free_encryption_index(&sc->cenc.encryption_index);
> +    av_encryption_info_free(sc->cenc.default_encrypted_sample);
> +    av_aes_ctr_free(sc->cenc.aes_ctr);
> +
> +    av_freep(&sc->stereo3d);
> +    av_freep(&sc->spherical);
> +    av_freep(&sc->mastering);
> +    av_freep(&sc->coll);
> +}
> +
>  static int mov_read_close(AVFormatContext *s)
>  {
>      MOVContext *mov = s->priv_data;
> @@ -8594,54 +8689,8 @@ static int mov_read_close(AVFormatContext *s)
>  
>      for (i = 0; i < s->nb_streams; i++) {
>          AVStream *st = s->streams[i];
> -        MOVStreamContext *sc = st->priv_data;
> -
> -        if (!sc)
> -            continue;
> -
> -        av_freep(&sc->ctts_data);
> -        for (j = 0; j < sc->drefs_count; j++) {
> -            av_freep(&sc->drefs[j].path);
> -            av_freep(&sc->drefs[j].dir);
> -        }
> -        av_freep(&sc->drefs);
> -
> -        sc->drefs_count = 0;
>  
> -        if (!sc->pb_is_copied)
> -            ff_format_io_close(s, &sc->pb);
> -
> -        sc->pb = NULL;
> -        av_freep(&sc->chunk_offsets);
> -        av_freep(&sc->stsc_data);
> -        av_freep(&sc->sample_sizes);
> -        av_freep(&sc->keyframes);
> -        av_freep(&sc->stts_data);
> -        av_freep(&sc->sdtp_data);
> -        av_freep(&sc->stps_data);
> -        av_freep(&sc->elst_data);
> -        av_freep(&sc->rap_group);
> -        av_freep(&sc->sync_group);
> -        av_freep(&sc->sgpd_sync);
> -        av_freep(&sc->sample_offsets);
> -        av_freep(&sc->open_key_samples);
> -        av_freep(&sc->display_matrix);
> -        av_freep(&sc->index_ranges);
> -
> -        if (sc->extradata)
> -            for (j = 0; j < sc->stsd_count; j++)
> -                av_free(sc->extradata[j]);
> -        av_freep(&sc->extradata);
> -        av_freep(&sc->extradata_size);
> -
> -        mov_free_encryption_index(&sc->cenc.encryption_index);
> -        av_encryption_info_free(sc->cenc.default_encrypted_sample);
> -        av_aes_ctr_free(sc->cenc.aes_ctr);
> -
> -        av_freep(&sc->stereo3d);
> -        av_freep(&sc->spherical);
> -        av_freep(&sc->mastering);
> -        av_freep(&sc->coll);
> +        mov_free_stream_context(s, st);

Factoring this out does not belong in this patch.

>      }
>  
>      av_freep(&mov->dv_demux);

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