Problem after applying a patch to generate random names in ffmpeg

Question

I applied the following patch below, it works as expected, but occasionally transcoding for and in other cases(rare cases, depending on the compiled ffmpeg version) keeps running but instead of creating multiple segments with random names, it keeps storing all in a single segment file.

That is, looking at the patch, is there any inconsistency in it? If so, how could I resolve?

diff --git a/libavformat/utils.c b/libavformat/utils.c
index 17ae300..7ff8c22 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -3809,6 +3809,36 @@ uint64_t ff_ntp_time(void)
     return (av_gettime() / 1000) * 1000 + NTP_OFFSET_US;
 }

+static char *randstring(size_t length) {
+
+    static char charset[] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
+    char *randomString;
+
+    if (length) {
+        randomString = malloc(sizeof(char) * (length +1));
+
+        if (randomString) {
+            for (int n = 0;n < length;n++) {
+                int key = rand() % (int)(sizeof(charset) -1);
+                randomString[n] = charset[key];
+            }
+
+            randomString[length] = '\0';
+        }
+    }
+
+    return randomString;
+}
+
+/**
+ * Random filename usage
+ *
+ * use "%<size>r" at -segment_format. <size> is the length of random string generated, for example: "%20r"
+ *
+ * command example:
+ * ffmpeg -i $1 -r 25 -c:a libfaac -ab:a 128k -ac:a 2 -c:v mpeg2video -s:v 640x360 -aspect:v 16:9 -map 0 -f segment -segment_time 120 -segment_list $2.m3u8 -segment_format mpegts "$2-%20r.ts"
+ */
+
 int av_get_frame_filename(char *buf, int buf_size, const char *path, int number)
 {
     const char *p;
@@ -3829,7 +3859,6 @@ int av_get_frame_filename(char *buf, int buf_size, const char *path, int number)
                     nd = nd * 10 + *p++ - '0';
                 c = *p++;
             } while (av_isdigit(c));
-
             switch (c) {
             case '%':
                 goto addchar;
@@ -3846,6 +3875,17 @@ int av_get_frame_filename(char *buf, int buf_size, const char *path, int number)
                 memcpy(q, buf1, len);
                 q += len;
                 break;
+            case 'r':
+                percentd_found = 1;
+                if (number < 0)
+                    nd += 1;
+                snprintf(buf1, sizeof(buf1), "%s", randstring(nd));
+                len = strlen(buf1);
+                if ((q - buf + len) > buf_size - 1)
+                    goto fail;
+                memcpy(q, buf1, len);
+                q += len;
+                goto addchar;
             default:
                 goto fail;
             }

Answer 1

I don’t know if that answers the question, but it’s gotten too big for a comment.

Right off the bat, I notice randstring(0) returns an uninitialized pointer, which is dangerous. I think, but am not sure, that this could occur if path contain a %0r.

However, there is a serious problem of memory Leak in randstring. This function allocates memory in heap, returns a pointer to av_get_frame_filename and the function av_get_frame_filename flame free nowhere.

So I suggest that the function randstring be the following:

static char *randstring(char *target, size_t length) {

    static char charset[] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";

    for (int n = 0; n < length - 1; n++) {
        int key = rand() % (int)(sizeof(charset) - 1);
        target[n] = charset[key];
    }

    if (length) target[length - 1] = '\0';

    return target;
}

Note that length now is the number of characters to be written including the \0, and no longer excluding \0. And we don’t need to use it anymore either malloc in this capacity.

And then, in your job av_get_frame_filename instead:

snprintf(buf1, sizeof(buf1), "%s", randstring(nd));
len = strlen(buf1);

You put that:

len = sizeof(buf1);
if (len > nd + 1) len = nd + 1;
randstring(buf1, len);

As for the rest of the function av_get_frame_filename, I don’t know, because I haven’t fully understood her yet, although I have more or less understood what she does.