diff mbox

[FFmpeg-devel,v2] avutil/avstring: improve av_strreplace implement

Message ID 20170402160817.52617-1-lq@chinaffmpeg.org
State Superseded
Headers show

Commit Message

Steven Liu April 2, 2017, 4:08 p.m. UTC
Use AVBprint to implementing av_strreplace
add av_strreplace TEST_STRREPLACE

Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
---
 libavformat/dashdec.c      |  2 ++
 libavutil/avstring.c       | 74 +++++++---------------------------------------
 libavutil/tests/avstring.c | 16 ++++++++++
 3 files changed, 28 insertions(+), 64 deletions(-)

Comments

Marton Balint April 2, 2017, 7:05 p.m. UTC | #1
On Mon, 3 Apr 2017, Steven Liu wrote:

> Use AVBprint to implementing av_strreplace
> add av_strreplace TEST_STRREPLACE

Thanks for keeping working on this. Agreeing with Nicolas, I also think it 
should be renamed to av_strireplace to reflect its case insensitivity.

Also the patch should be backported to 3.3. as well.

> index 6438dbaede..f86798c69b 100644
> --- a/libavformat/dashdec.c
> +++ b/libavformat/dashdec.c
> @@ -410,8 +410,10 @@ static char *get_content_url(xmlNodePtr *baseurl_nodes, int n_baseurl_nodes, xml
>     }
>     if (rep_bandwidth_val && tmp_str)
>         url = av_strreplace(tmp_str, "$Bandwidth$", (const char*)rep_bandwidth_val);
> +
>     if (tmp_str != url)
>         av_free(tmp_str);
> +
>     return url;
> }

Unrelated whitespace change...

Thanks,
Marton
diff mbox

Patch

diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
index 6438dbaede..f86798c69b 100644
--- a/libavformat/dashdec.c
+++ b/libavformat/dashdec.c
@@ -410,8 +410,10 @@  static char *get_content_url(xmlNodePtr *baseurl_nodes, int n_baseurl_nodes, xml
     }
     if (rep_bandwidth_val && tmp_str)
         url = av_strreplace(tmp_str, "$Bandwidth$", (const char*)rep_bandwidth_val);
+
     if (tmp_str != url)
         av_free(tmp_str);
+
     return url;
 }
 
diff --git a/libavutil/avstring.c b/libavutil/avstring.c
index 52e6e6cd13..6dee687e91 100644
--- a/libavutil/avstring.c
+++ b/libavutil/avstring.c
@@ -233,78 +233,24 @@  int av_strncasecmp(const char *a, const char *b, size_t n)
 
 char *av_strreplace(const char *str, const char *from, const char *to)
 {
-    /* Adjust each of the below values to suit your needs. */
-    /* Increment positions cache size initially by this number. */
-    size_t cache_sz_inc = 16;
-    /* Thereafter, each time capacity needs to be increased,
-     * multiply the increment by this factor. */
-    const size_t cache_sz_inc_factor = 3;
-    /* But never increment capacity by more than this number. */
-    const size_t cache_sz_inc_max = 1048576;
-
-    char *pret, *ret = NULL;
+    char *ret = NULL;
     const char *pstr2, *pstr = str;
-    size_t i, count = 0;
-    uintptr_t *pos_cache_tmp, *pos_cache = NULL;
-    size_t cache_sz = 0;
-    size_t cpylen, orglen, retlen, tolen, fromlen = strlen(from);
+    size_t tolen = strlen(to), fromlen = strlen(from);
+    AVBPrint pbuf;
 
-    /* Find all matches and cache their positions. */
+    av_bprint_init(&pbuf, 1, AV_BPRINT_SIZE_UNLIMITED);
     while ((pstr2 = av_stristr(pstr, from))) {
-        count++;
-        /* Increase the cache size when necessary. */
-        if (cache_sz < count) {
-            cache_sz += cache_sz_inc;
-            pos_cache_tmp = av_realloc(pos_cache, sizeof(*pos_cache) * cache_sz);
-            if (!pos_cache_tmp) {
-                goto end_strreplace;
-            } else pos_cache = pos_cache_tmp;
-            cache_sz_inc *= cache_sz_inc_factor;
-            if (cache_sz_inc > cache_sz_inc_max) {
-                cache_sz_inc = cache_sz_inc_max;
-            }
-        }
-
-        pos_cache[count-1] = pstr2 - str;
+        av_bprint_append_data(&pbuf, pstr, pstr2 - pstr);
         pstr = pstr2 + fromlen;
+        av_bprint_append_data(&pbuf, to, tolen);
     }
-    orglen = pstr - str + strlen(pstr);
-    /* Allocate memory for the post-replacement string. */
-    if (count > 0) {
-        tolen = strlen(to);
-        retlen = orglen + (tolen - fromlen) * count;
-    } else {
-        retlen = orglen;
-    }
-    ret = av_malloc(retlen + 1);
-    if (!ret) {
-        goto end_strreplace;
-    }
-
-    if (!count) {
-        /* If no matches, then just duplicate the string. */
-        av_strlcpy(ret, str, retlen + 1);
+    av_bprint_append_data(&pbuf, pstr, strlen(pstr));
+    if (!av_bprint_is_complete(&pbuf)) {
+        av_bprint_finalize(&pbuf, NULL);
     } else {
-        /* Otherwise, duplicate the string whilst performing
-         * the replacements using the position cache. */
-        pret = ret;
-        memcpy(pret, str, pos_cache[0]);
-        pret += pos_cache[0];
-        for (i = 0; i < count; i++) {
-            memcpy(pret, to, tolen);
-            pret += tolen;
-            pstr = str + pos_cache[i] + fromlen;
-            cpylen = (i == count-1 ? orglen : pos_cache[i+1]) - pos_cache[i] - fromlen;
-            memcpy(pret, pstr, cpylen);
-            pret += cpylen;
-        }
-        ret[retlen] = '\0';
+        av_bprint_finalize(&pbuf, &ret);
     }
 
-end_strreplace:
-    /* Free the cache and return the post-replacement string,
-     * which will be NULL in the event of an error. */
-    av_free(pos_cache);
     return ret;
 }
 
diff --git a/libavutil/tests/avstring.c b/libavutil/tests/avstring.c
index 14bc7ffcea..8660b348a8 100644
--- a/libavutil/tests/avstring.c
+++ b/libavutil/tests/avstring.c
@@ -93,6 +93,22 @@  int main(void)
     TEST_STRNSTR(haystack, needle [2], strlen(haystack), NULL       );
     TEST_STRNSTR(haystack, strings[1], strlen(haystack), haystack   );
 
+    /*Testing av_strreplace()*/
+    #define TEST_STRREPLACE(haystack, needle, expected) \
+        ptr = av_strreplace(haystack, needle, "instead"); \
+        if (ptr == NULL) { \
+            printf("error, received null pointer!\n"); \
+        } else { \
+            if (strcmp(ptr, expected) != 0) \
+                printf( "expected: %s, received: %s\n", expected, ptr); \
+            av_free(ptr); \
+        }
+
+    TEST_STRREPLACE(haystack, needle [0], "Education consists mainly in what we have uninstead");
+    TEST_STRREPLACE(haystack, needle [1], "Education consists mainly in what we have instead");
+    TEST_STRREPLACE(haystack, needle [2], "Education consists mainly in what we have instead.");
+    TEST_STRREPLACE(haystack, needle [1], "Education consists mainly in what we have instead");
+
     /*Testing av_d2str()*/
     #define TEST_D2STR(value, expected) \
         if((ptr = av_d2str(value)) == NULL){ \