[FFmpeg-devel] [PATCH 16/17] avformat/matroskadec: Use av_fast_realloc_array()

Andreas Rheinhardt andreas.rheinhardt at gmail.com
Thu Dec 26 12:53:41 EET 2019


instead of av_fast_realloc() for allocating an array. It has the
advantage of doing it's own overflow checks and does not overallocate
unnecessarily: It allocates exactly one element if one element is
desired. This is advantageous for CueTrackPositions: While it is
allowed (and supported) to have more than one of them in a CuePoint,
there is typically only one of them, so that overallocating is a waste.
The memory used for a file with 540000 cues entries therefore decreased
from 69624 KB to 52592 KB.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
---
 libavformat/matroskadec.c | 56 +++++++++++++++++++--------------------
 1 file changed, 27 insertions(+), 29 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 72624dc3f1..fe4a444410 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -109,8 +109,8 @@ typedef const struct EbmlSyntax {
 } EbmlSyntax;
 
 typedef struct EbmlList {
-    int nb_elem;
-    unsigned int alloc_elem_size;
+    unsigned nb_elem;
+    unsigned nb_allocated;
     void *elem;
 } EbmlList;
 
@@ -1226,13 +1226,10 @@ static int ebml_parse(MatroskaDemuxContext *matroska,
         data = (char *) data + syntax->data_offset;
         if (syntax->list_elem_size) {
             EbmlList *list = data;
-            void *newelem;
-
-            if ((unsigned)list->nb_elem + 1 >= UINT_MAX / syntax->list_elem_size)
-                return AVERROR(ENOMEM);
-            newelem = av_fast_realloc(list->elem,
-                                      &list->alloc_elem_size,
-                                      (list->nb_elem + 1) * syntax->list_elem_size);
+            void *newelem = av_fast_realloc_array(list->elem,
+                                                  &list->nb_allocated,
+                                                  list->nb_elem + 1,
+                                                  syntax->list_elem_size);
             if (!newelem)
                 return AVERROR(ENOMEM);
             list->elem = newelem;
@@ -1464,7 +1461,7 @@ level_check:
 
 static void ebml_free(EbmlSyntax *syntax, void *data)
 {
-    int i, j;
+    int i;
     for (i = 0; syntax[i].id; i++) {
         void *data_off = (char *) data + syntax[i].data_offset;
         switch (syntax[i].type) {
@@ -1480,12 +1477,12 @@ static void ebml_free(EbmlSyntax *syntax, void *data)
             if (syntax[i].list_elem_size) {
                 EbmlList *list = data_off;
                 char *ptr = list->elem;
-                for (j = 0; j < list->nb_elem;
+                for (unsigned j = 0; j < list->nb_elem;
                      j++, ptr += syntax[i].list_elem_size)
                     ebml_free(syntax[i].def.n, ptr);
                 av_freep(&list->elem);
                 list->nb_elem = 0;
-                list->alloc_elem_size = 0;
+                list->nb_allocated = 0;
             } else
                 ebml_free(syntax[i].def.n, data_off);
         default:
@@ -1548,7 +1545,7 @@ static MatroskaTrack *matroska_find_track_by_num(MatroskaDemuxContext *matroska,
                                                  int num)
 {
     MatroskaTrack *tracks = matroska->tracks.elem;
-    int i;
+    unsigned i;
 
     for (i = 0; i < matroska->tracks.nb_elem; i++)
         if (tracks[i].num == num)
@@ -1702,7 +1699,7 @@ static void matroska_convert_tag(AVFormatContext *s, EbmlList *list,
 {
     MatroskaTag *tags = list->elem;
     char key[1024];
-    int i;
+    unsigned i;
 
     for (i = 0; i < list->nb_elem; i++) {
         const char *lang = tags[i].lang &&
@@ -1736,7 +1733,7 @@ static void matroska_convert_tags(AVFormatContext *s)
 {
     MatroskaDemuxContext *matroska = s->priv_data;
     MatroskaTags *tags = matroska->tags.elem;
-    int i, j;
+    unsigned i, j;
 
     for (i = 0; i < matroska->tags.nb_elem; i++) {
         if (tags[i].target.attachuid) {
@@ -1752,7 +1749,7 @@ static void matroska_convert_tags(AVFormatContext *s)
             }
             if (!found) {
                 av_log(NULL, AV_LOG_WARNING,
-                       "The tags at index %d refer to a "
+                       "The tags at index %u refer to a "
                        "non-existent attachment %"PRId64".\n",
                        i, tags[i].target.attachuid);
             }
@@ -1769,7 +1766,7 @@ static void matroska_convert_tags(AVFormatContext *s)
             }
             if (!found) {
                 av_log(NULL, AV_LOG_WARNING,
-                       "The tags at index %d refer to a non-existent chapter "
+                       "The tags at index %u refer to a non-existent chapter "
                        "%"PRId64".\n",
                        i, tags[i].target.chapteruid);
             }
@@ -1786,7 +1783,7 @@ static void matroska_convert_tags(AVFormatContext *s)
             }
             if (!found) {
                 av_log(NULL, AV_LOG_WARNING,
-                       "The tags at index %d refer to a non-existent track "
+                       "The tags at index %u refer to a non-existent track "
                        "%"PRId64".\n",
                        i, tags[i].target.trackuid);
             }
@@ -1835,7 +1832,7 @@ static int matroska_parse_seekhead_entry(MatroskaDemuxContext *matroska,
 static void matroska_execute_seekhead(MatroskaDemuxContext *matroska)
 {
     EbmlList *seekhead_list = &matroska->seekhead;
-    int i;
+    unsigned i;
 
     // we should not do any seeking in the streaming case
     if (!(matroska->ctx->pb->seekable & AVIO_SEEKABLE_NORMAL))
@@ -1871,7 +1868,7 @@ static void matroska_add_index_entries(MatroskaDemuxContext *matroska)
     EbmlList *index_list;
     MatroskaIndex *index;
     uint64_t index_scale = 1;
-    int i, j;
+    unsigned i, j;
 
     if (matroska->ctx->flags & AVFMT_FLAG_IGNIDX)
         return;
@@ -2265,8 +2262,8 @@ static int matroska_parse_tracks(AVFormatContext *s)
     MatroskaDemuxContext *matroska = s->priv_data;
     MatroskaTrack *tracks = matroska->tracks.elem;
     AVStream *st;
-    int i, j, ret;
-    int k;
+    unsigned i, j, k;
+    int ret;
 
     for (i = 0; i < matroska->tracks.nb_elem; i++) {
         MatroskaTrack *track = &tracks[i];
@@ -2716,7 +2713,7 @@ static int matroska_parse_tracks(AVFormatContext *s)
                 char buf[32];
                 if (planes[j].type >= MATROSKA_VIDEO_STEREO_PLANE_COUNT)
                     continue;
-                snprintf(buf, sizeof(buf), "%s_%d",
+                snprintf(buf, sizeof(buf), "%s_%u",
                          ff_matroska_video_stereo_plane[planes[j].type], i);
                 for (k=0; k < matroska->tracks.nb_elem; k++)
                     if (planes[j].uid == tracks[k].uid && tracks[k].stream) {
@@ -2791,7 +2788,8 @@ static int matroska_read_header(AVFormatContext *s)
     uint64_t max_start = 0;
     int64_t pos;
     Ebml ebml = { 0 };
-    int i, j, res;
+    unsigned i, j;
+    int res;
 
     matroska->ctx = s;
     matroska->cues_parsing_deferred = 1;
@@ -3708,7 +3706,7 @@ static int matroska_read_seek(AVFormatContext *s, int stream_index,
     MatroskaDemuxContext *matroska = s->priv_data;
     MatroskaTrack *tracks = NULL;
     AVStream *st = s->streams[stream_index];
-    int i, index;
+    int index;
 
     /* Parse the CUES now since we need the index data to seek. */
     if (matroska->cues_parsing_deferred > 0) {
@@ -3734,7 +3732,7 @@ static int matroska_read_seek(AVFormatContext *s, int stream_index,
         goto err;
 
     tracks = matroska->tracks.elem;
-    for (i = 0; i < matroska->tracks.nb_elem; i++) {
+    for (unsigned i = 0; i < matroska->tracks.nb_elem; i++) {
         tracks[i].audio.pkt_cnt        = 0;
         tracks[i].audio.sub_packet_cnt = 0;
         tracks[i].audio.buf_timecode   = AV_NOPTS_VALUE;
@@ -3770,7 +3768,7 @@ static int matroska_read_close(AVFormatContext *s)
 {
     MatroskaDemuxContext *matroska = s->priv_data;
     MatroskaTrack *tracks = matroska->tracks.elem;
-    int n;
+    unsigned n;
 
     matroska_clear_queue(matroska);
 
@@ -4043,7 +4041,7 @@ static int webm_dash_manifest_cues(AVFormatContext *s, int64_t init_range)
     MatroskaSeekhead *seekhead = seekhead_list->elem;
     char *buf;
     int64_t cues_start = -1, cues_end = -1, before_pos, bandwidth;
-    int i;
+    unsigned i;
     int end = 0;
 
     // determine cues start and end positions
@@ -4099,7 +4097,7 @@ static int webm_dash_manifest_cues(AVFormatContext *s, int64_t init_range)
     buf = av_malloc_array(s->streams[0]->nb_index_entries, 20);
     if (!buf) return -1;
     strcpy(buf, "");
-    for (i = 0; i < s->streams[0]->nb_index_entries; i++) {
+    for (int i = 0; i < s->streams[0]->nb_index_entries; i++) {
         int ret = snprintf(buf + end, 20,
                            "%" PRId64"%s", s->streams[0]->index_entries[i].timestamp,
                            i != s->streams[0]->nb_index_entries - 1 ? "," : "");
-- 
2.20.1



More information about the ffmpeg-devel mailing list