| Message ID | CAB0OVGqBnTSiGd6Qbwqk08Fh9HU_cNZR6QyvhTcL18kfD4JLow@mail.gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Mon, Nov 27, 2017 at 05:24:14AM +0100, Carl Eugen Hoyos wrote: > Hi! > > Attached patch avoids allocations >1GB for (short and) invalid mov > files with only reasonable speed impact. > > Please review, Carl Eugen > mov.c | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > 980861e4c47c80c850d4e849043df2510a3d1d32 0001-lavf-mov-Do-not-blindly-allocate-huge-memory-blocks-.patch > From 0d243bad5fdd9850ff41d49a32a06274a3cd9756 Mon Sep 17 00:00:00 2001 > From: Carl Eugen Hoyos <ceffmpeg@gmail.com> > Date: Mon, 27 Nov 2017 05:13:25 +0100 > Subject: [PATCH] lavf/mov: Do not blindly allocate huge memory blocks for > stts entries. > > Fixes large allocations for short files with invalid stts entry. > Fixes bugzilla 1102. > --- > libavformat/mov.c | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/libavformat/mov.c b/libavformat/mov.c > index ddb1e59..9d353bf 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -2838,14 +2838,24 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom) > if (sc->stts_data) > av_log(c->fc, AV_LOG_WARNING, "Duplicated STTS atom\n"); > av_free(sc->stts_data); > - sc->stts_count = 0; > - sc->stts_data = av_malloc_array(entries, sizeof(*sc->stts_data)); > + sc->stts_count = FFMIN(1024 * 1024, entries); > + sc->stts_data = av_realloc_array(NULL, sc->stts_count, sizeof(*sc->stts_data)); > if (!sc->stts_data) > return AVERROR(ENOMEM); i dont know if leaving stts_count random on return is a good idea > > for (i = 0; i < entries && !pb->eof_reached; i++) { > - int sample_duration; > + int sample_duration, ret; > unsigned int sample_count; > + if (i > sc->stts_count) { > + ret = av_reallocp_array(&sc->stts_data, > + FFMIN(sc->stts_count * 2LL, entries), > + sizeof(*sc->stts_data)); this should use a variat of av_fast_realloc [...]
2017-11-28 21:32 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>: > On Mon, Nov 27, 2017 at 05:24:14AM +0100, Carl Eugen Hoyos wrote: >> for (i = 0; i < entries && !pb->eof_reached; i++) { >> - int sample_duration; >> + int sample_duration, ret; >> unsigned int sample_count; >> + if (i > sc->stts_count) { >> + ret = av_reallocp_array(&sc->stts_data, >> + FFMIN(sc->stts_count * 2LL, entries), >> + sizeof(*sc->stts_data)); > > this should use a variant of av_fast_realloc Do you prefer the new patch? The old variant here looks slightly saner to me. Carl Eugen
From 0d243bad5fdd9850ff41d49a32a06274a3cd9756 Mon Sep 17 00:00:00 2001 From: Carl Eugen Hoyos <ceffmpeg@gmail.com> Date: Mon, 27 Nov 2017 05:13:25 +0100 Subject: [PATCH] lavf/mov: Do not blindly allocate huge memory blocks for stts entries. Fixes large allocations for short files with invalid stts entry. Fixes bugzilla 1102. --- libavformat/mov.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/libavformat/mov.c b/libavformat/mov.c index ddb1e59..9d353bf 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -2838,14 +2838,24 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom) if (sc->stts_data) av_log(c->fc, AV_LOG_WARNING, "Duplicated STTS atom\n"); av_free(sc->stts_data); - sc->stts_count = 0; - sc->stts_data = av_malloc_array(entries, sizeof(*sc->stts_data)); + sc->stts_count = FFMIN(1024 * 1024, entries); + sc->stts_data = av_realloc_array(NULL, sc->stts_count, sizeof(*sc->stts_data)); if (!sc->stts_data) return AVERROR(ENOMEM); for (i = 0; i < entries && !pb->eof_reached; i++) { - int sample_duration; + int sample_duration, ret; unsigned int sample_count; + if (i > sc->stts_count) { + ret = av_reallocp_array(&sc->stts_data, + FFMIN(sc->stts_count * 2LL, entries), + sizeof(*sc->stts_data)); + if (ret < 0) { + sc->stts_count = 0; + return ret; + } + sc->stts_count = FFMIN(sc->stts_count * 2, entries); + } sample_count=avio_rb32(pb); sample_duration = avio_rb32(pb); -- 1.7.10.4