On 1/13/2025 7:35 PM, Andreas Rheinhardt wrote:
That only applied to stts, because ctts had a realloc call for all entries before the loop.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 <[email protected]> --- 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).
If you think this approach is better, then i can remove that call.
OpenPGP_signature.asc
Description: OpenPGP digital signature
_______________________________________________ ffmpeg-devel mailing list [email protected] https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email [email protected] with subject "unsubscribe".
