James Almer:
> No point using av_fast_realloc() in a loop when we want to allocate all
> entries to begin with, and any duplicate stts/ctts will just replace the
> old arrays.
> Furthermore, these are temporary arrays that will be merged into tts_data
> when building the index.
>
> Signed-off-by: James Almer <jamr...@gmail.com>
> ---
> libavformat/isom.h | 2 --
> libavformat/mov.c | 40 ++++++----------------------------------
> 2 files changed, 6 insertions(+), 36 deletions(-)
>
> diff --git a/libavformat/isom.h b/libavformat/isom.h
> index ccdead7192..16981dc918 100644
> --- a/libavformat/isom.h
> +++ b/libavformat/isom.h
> @@ -183,12 +183,10 @@ typedef struct MOVStreamContext {
> unsigned int tts_allocated_size;
> MOVTimeToSample *tts_data;
> unsigned int stts_count;
> - unsigned int stts_allocated_size;
> MOVStts *stts_data;
> unsigned int sdtp_count;
> uint8_t *sdtp_data;
> unsigned int ctts_count;
> - unsigned int ctts_allocated_size;
> MOVCtts *ctts_data;
> unsigned int stsc_count;
> MOVStsc *stsc_data;
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 138120488a..f310cb8d49 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -3498,22 +3498,14 @@ static int mov_read_stts(MOVContext *c, AVIOContext
> *pb, MOVAtom atom)
> av_log(c->fc, AV_LOG_WARNING, "Duplicated STTS atom\n");
> av_freep(&sc->stts_data);
> sc->stts_count = 0;
> - if (entries >= INT_MAX / sizeof(*sc->stts_data))
> +
> + sc->stts_data = av_malloc_array(entries, sizeof(*sc->stts_data));
> + if (!sc->stts_data)
> return AVERROR(ENOMEM);
>
> for (i = 0; i < entries && !pb->eof_reached; i++) {
> unsigned int sample_duration;
> unsigned int sample_count;
> - unsigned int min_entries = FFMIN(FFMAX(i + 1, 1024 * 1024), entries);
> - MOVStts *stts_data = av_fast_realloc(sc->stts_data,
> &sc->stts_allocated_size,
> - min_entries *
> sizeof(*sc->stts_data));
> - if (!stts_data) {
> - av_freep(&sc->stts_data);
> - sc->stts_count = 0;
> - return AVERROR(ENOMEM);
> - }
> - sc->stts_count = min_entries;
> - sc->stts_data = stts_data;
>
> sample_count = avio_rb32(pb);
> sample_duration = avio_rb32(pb);
> @@ -3656,20 +3648,12 @@ static int mov_read_ctts(MOVContext *c, AVIOContext
> *pb, MOVAtom atom)
>
> if (!entries)
> return 0;
> - if (entries >= UINT_MAX / sizeof(*sc->ctts_data))
> - return AVERROR_INVALIDDATA;
> av_freep(&sc->ctts_data);
> - sc->ctts_data = av_fast_realloc(NULL, &sc->ctts_allocated_size, entries
> * sizeof(*sc->ctts_data));
> + sc->ctts_data = av_malloc_array(entries, sizeof(*sc->ctts_data));
> if (!sc->ctts_data)
> return AVERROR(ENOMEM);
>
> for (i = 0; i < entries && !pb->eof_reached; i++) {
> - MOVCtts *ctts_data;
> - const size_t min_size_needed = (ctts_count + 1) * sizeof(MOVCtts);
> - const size_t requested_size =
> - min_size_needed > sc->ctts_allocated_size ?
> - FFMAX(min_size_needed, 2 * sc->ctts_allocated_size) :
> - min_size_needed;
> int count = avio_rb32(pb);
> int duration = avio_rb32(pb);
>
> @@ -3680,18 +3664,8 @@ static int mov_read_ctts(MOVContext *c, AVIOContext
> *pb, MOVAtom atom)
> continue;
> }
>
> - if (ctts_count >= UINT_MAX / sizeof(MOVCtts) - 1)
> - return AVERROR(ENOMEM);
> -
> - ctts_data = av_fast_realloc(sc->ctts_data, &sc->ctts_allocated_size,
> requested_size);
> -
> - if (!ctts_data)
> - return AVERROR(ENOMEM);
> -
> - sc->ctts_data = ctts_data;
> -
> - ctts_data[ctts_count].count = count;
> - ctts_data[ctts_count].offset = duration;
> + sc->ctts_data[ctts_count].count = count;
> + sc->ctts_data[ctts_count].offset = duration;
> ctts_count++;
>
> av_log(c->fc, AV_LOG_TRACE, "count=%d, duration=%d\n",
> @@ -4585,7 +4559,6 @@ static int mov_merge_tts_data(MOVContext *mov, AVStream
> *st, int flags)
> } else
> sc->ctts_count = 0;
> av_freep(&sc->ctts_data);
> - sc->ctts_allocated_size = 0;
>
> idx = 0;
> if (stts) {
> @@ -4610,7 +4583,6 @@ static int mov_merge_tts_data(MOVContext *mov, AVStream
> *st, int flags)
> } else
> sc->stts_count = 0;
> av_freep(&sc->stts_data);
> - sc->stts_allocated_size = 0;
>
> return 0;
> }
I always thought that this pattern is used because it avoids having to
allocate a potentially huge buffer just because a counter says that many
elements are present even when the actual file doesn't contain that many
entries because it is very small (this might cause timeout issues in the
fuzzer).
- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
