[FFmpeg-devel] [PATCH 4/4] avformat/au: Avoid allocation for metadata string

[Andreas Rheinhardt]

When there are potentially annotation (i.e. metadata) fields to write,
au_get_annotations() is called to produce a string with them. To do so,
it uses an AVBPrint which is finalized to create the string. This is
wasteful, because it always leads to an allocation even if the string
actually fits into the internal buffer of the AVBPrint. This commit
changes this by making au_get_annotations() modify an AVBPrint that
resides on the stack of the caller (i.e. of au_write_header()).

Furthermore, the AVBPrint is now checked for truncation; limiting
the allocations implicit in the AVBPrint allowed to offload the overflow
checks. Notice that these were not correct before: The size parameter of
avio_write() is an int, yet the string in the AVBPrint was allowed to
grow bigger than INT_MAX. And if the length of the string was so near
UINT_MAX that the length + 32 overflowed, the old code would write the
first eight bytes of the string and nothing more, leading to an invalid
file.

Finally, the special case in which the metadata dictionary of the
AVFormatContext is empty (in which case one still has to write eight
binary zeroes) is now no longer treated specially, because this case
no longer incurs any allocation.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
---
 libavformat/au.c | 50 ++++++++++++++++++------------------------------
 1 file changed, 19 insertions(+), 31 deletions(-)

diff --git a/libavformat/au.c b/libavformat/au.c
index a8906a9db7..c09f4da4c9 100644
--- a/libavformat/au.c
+++ b/libavformat/au.c
@@ -35,8 +35,6 @@
 
 /* if we don't know the size in advance */
 #define AU_UNKNOWN_SIZE ((uint32_t)(~0))
-/* the specification requires an annotation field of at least eight bytes */
-#define AU_DEFAULT_HEADER_SIZE (24+8)
 
 static const AVCodecTag codec_au_tags[] = {
     { AV_CODEC_ID_PCM_MULAW,  1 },
@@ -241,7 +239,7 @@ typedef struct AUContext {
 
 #include "rawenc.h"
 
-static int au_get_annotations(AVFormatContext *s, char **buffer)
+static int au_get_annotations(AVFormatContext *s, AVBPrint *annotations)
 {
     static const char keys[][7] = {
         "Title",
@@ -253,21 +251,19 @@ static int au_get_annotations(AVFormatContext *s, char **buffer)
     int cnt = 0;
     AVDictionary *m = s->metadata;
     AVDictionaryEntry *t = NULL;
-    AVBPrint bprint;
-
-    av_bprint_init(&bprint, 64, AV_BPRINT_SIZE_UNLIMITED);
 
     for (int i = 0; i < FF_ARRAY_ELEMS(keys); i++) {
         t = av_dict_get(m, keys[i], NULL, 0);
         if (t != NULL) {
             if (cnt++)
-                av_bprint_chars(&bprint, '\n', 1);
-            av_bprintf(&bprint, "%s=%s", keys[i], t->value);
+                av_bprint_chars(annotations, '\n', 1);
+            av_bprintf(annotations, "%s=%s", keys[i], t->value);
         }
     }
-    /* pad with 0's */
-    av_bprint_chars(&bprint, '\0', 8);
-    return av_bprint_finalize(&bprint, buffer);
+    /* The specification requires the annotation field to be zero-terminated
+     * and its length to be a multiple of eight, so pad with 0's */
+    av_bprint_chars(annotations, '\0', 8);
+    return av_bprint_is_complete(annotations) ? 0 : AVERROR(ENOMEM);
 }
 
 static int au_write_header(AVFormatContext *s)
@@ -276,9 +272,7 @@ static int au_write_header(AVFormatContext *s)
     AUContext *au = s->priv_data;
     AVIOContext *pb = s->pb;
     AVCodecParameters *par = s->streams[0]->codecpar;
-    char *annotations = NULL;
-
-    au->header_size = AU_DEFAULT_HEADER_SIZE;
+    AVBPrint annotations;
 
     if (s->nb_streams != 1) {
         av_log(s, AV_LOG_ERROR, "only one stream is supported\n");
@@ -291,30 +285,24 @@ static int au_write_header(AVFormatContext *s)
         return AVERROR(EINVAL);
     }
 
-    if (av_dict_count(s->metadata) > 0) {
-        ret = au_get_annotations(s, &annotations);
-        if (ret < 0)
-            return ret;
-        if (annotations != NULL) {
-            au->header_size = (24 + strlen(annotations) + 8) & ~7;
-            if (au->header_size < AU_DEFAULT_HEADER_SIZE)
-                au->header_size = AU_DEFAULT_HEADER_SIZE;
-        }
-    }
+    av_bprint_init(&annotations, 0, INT_MAX - 24);
+    ret = au_get_annotations(s, &annotations);
+    if (ret < 0)
+        goto fail;
+    au->header_size = 24 + annotations.len & ~7;
+
     ffio_wfourcc(pb, ".snd");                   /* magic number */
     avio_wb32(pb, au->header_size);             /* header size */
     avio_wb32(pb, AU_UNKNOWN_SIZE);             /* data size */
     avio_wb32(pb, par->codec_tag);              /* codec ID */
     avio_wb32(pb, par->sample_rate);
     avio_wb32(pb, par->channels);
-    if (annotations != NULL) {
-        avio_write(pb, annotations, au->header_size - 24);
-        av_freep(&annotations);
-    } else {
-        avio_wb64(pb, 0); /* annotation field */
-    }
+    avio_write(pb, annotations.str, annotations.len & ~7);
 
-    return 0;
+fail:
+    av_bprint_finalize(&annotations, NULL);
+
+    return ret;
 }
 
 static int au_write_trailer(AVFormatContext *s)
-- 
2.20.1