[FFmpeg-devel] [PATCH 1/1] librtmp: make flashVer case consistent

[FFmpeg-devel] [PATCH 1/1] librtmp: make flashVer case consistent

[Tristan Matthews]

This is basically a cosmetic change (no functional difference).

---
 libavformat/librtmp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/librtmp.c b/libavformat/librtmp.c
index 43013e46e0..b23adb9593 100644
--- a/libavformat/librtmp.c
+++ b/libavformat/librtmp.c
@@ -115,7 +115,7 @@ static int rtmp_open(URLContext *s, const char *uri, int flags)
     if (ctx->app)      len += strlen(ctx->app)      + sizeof(" app=");
     if (ctx->tcurl)    len += strlen(ctx->tcurl)    + sizeof(" tcUrl=");
     if (ctx->pageurl)  len += strlen(ctx->pageurl)  + sizeof(" pageUrl=");
-    if (ctx->flashver) len += strlen(ctx->flashver) + sizeof(" flashver=");
+    if (ctx->flashver) len += strlen(ctx->flashver) + sizeof(" flashVer=");
 
     if (ctx->conn) {
         char *sep, *p = ctx->conn;
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/1] librtmp: make flashVer case consistent

[Marton Balint]

On Wed, 6 Apr 2022, Tristan Matthews wrote:

> This is basically a cosmetic change (no functional difference).
>
> ---
> libavformat/librtmp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavformat/librtmp.c b/libavformat/librtmp.c
> index 43013e46e0..b23adb9593 100644
> --- a/libavformat/librtmp.c
> +++ b/libavformat/librtmp.c
> @@ -115,7 +115,7 @@ static int rtmp_open(URLContext *s, const char *uri, int flags)
>     if (ctx->app)      len += strlen(ctx->app)      + sizeof(" app=");
>     if (ctx->tcurl)    len += strlen(ctx->tcurl)    + sizeof(" tcUrl=");
>     if (ctx->pageurl)  len += strlen(ctx->pageurl)  + sizeof(" pageUrl=");
> -    if (ctx->flashver) len += strlen(ctx->flashver) + sizeof(" flashver=");
> +    if (ctx->flashver) len += strlen(ctx->flashver) + sizeof(" flashVer=");

Actually this whole rtmp_open function should be reworked to use an 
AVBPrint buffer to generate the rtmp URL. The way it works now - 
calculating the length first then creating the actual sting - is 
very ugly.

Regards,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/1] librtmp: make flashVer case consistent

[Tristan Matthews]

On Sat, Apr 9, 2022 at 2:45 PM Marton Balint <cus@passwd.hu> wrote:

>
>
> On Wed, 6 Apr 2022, Tristan Matthews wrote:
>
> > This is basically a cosmetic change (no functional difference).
> >
> > ---
> > libavformat/librtmp.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavformat/librtmp.c b/libavformat/librtmp.c
> > index 43013e46e0..b23adb9593 100644
> > --- a/libavformat/librtmp.c
> > +++ b/libavformat/librtmp.c
> > @@ -115,7 +115,7 @@ static int rtmp_open(URLContext *s, const char *uri,
> int flags)
> >     if (ctx->app)      len += strlen(ctx->app)      + sizeof(" app=");
> >     if (ctx->tcurl)    len += strlen(ctx->tcurl)    + sizeof(" tcUrl=");
> >     if (ctx->pageurl)  len += strlen(ctx->pageurl)  + sizeof("
> pageUrl=");
> > -    if (ctx->flashver) len += strlen(ctx->flashver) + sizeof("
> flashver=");
> > +    if (ctx->flashver) len += strlen(ctx->flashver) + sizeof("
> flashVer=");
>
> Actually this whole rtmp_open function should be reworked to use an
> AVBPrint buffer to generate the rtmp URL. The way it works now -
> calculating the length first then creating the actual sting - is
> very ugly.
>
>
Oh yeah good call, I will follow up with that instead.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 1/1] librtmp: use AVBPrint instead of char *

[Tristan Matthews]

This avoids having to do one pass to calculate the full length to allocate
followed by a second pass to actually append values.
---
 libavformat/librtmp.c | 123 +++++++++++-------------------------------
 1 file changed, 32 insertions(+), 91 deletions(-)

diff --git a/libavformat/librtmp.c b/libavformat/librtmp.c
index 43013e46e0..7b379e48ee 100644
--- a/libavformat/librtmp.c
+++ b/libavformat/librtmp.c
@@ -25,6 +25,7 @@
  */
 
 #include "libavutil/avstring.h"
+#include "libavutil/bprint.h"
 #include "libavutil/mathematics.h"
 #include "libavutil/opt.h"
 #include "avformat.h"
@@ -50,7 +51,6 @@ typedef struct LibRTMPContext {
     char *pageurl;
     char *client_buffer_time;
     int live;
-    char *temp_filename;
     int buffer_size;
 } LibRTMPContext;
 
@@ -76,7 +76,6 @@ static int rtmp_close(URLContext *s)
     RTMP *r = &ctx->rtmp;
 
     RTMP_Close(r);
-    av_freep(&ctx->temp_filename);
     return 0;
 }
 
@@ -94,11 +93,11 @@ static int rtmp_close(URLContext *s)
  */
 static int rtmp_open(URLContext *s, const char *uri, int flags)
 {
+    AVBPrint filename;
     LibRTMPContext *ctx = s->priv_data;
     RTMP *r = &ctx->rtmp;
     int rc = 0, level;
-    char *filename = s->filename;
-    int len = strlen(s->filename) + 1;
+    av_bprint_init(&filename, 0, AV_BPRINT_SIZE_UNLIMITED);
 
     switch (av_log_get_level()) {
     default:
@@ -112,118 +111,58 @@ static int rtmp_open(URLContext *s, const char *uri, int flags)
     RTMP_LogSetLevel(level);
     RTMP_LogSetCallback(rtmp_log);
 
-    if (ctx->app)      len += strlen(ctx->app)      + sizeof(" app=");
-    if (ctx->tcurl)    len += strlen(ctx->tcurl)    + sizeof(" tcUrl=");
-    if (ctx->pageurl)  len += strlen(ctx->pageurl)  + sizeof(" pageUrl=");
-    if (ctx->flashver) len += strlen(ctx->flashver) + sizeof(" flashver=");
-
+    av_bprintf(&filename, "%s", s->filename);
+    if (ctx->app)
+        av_bprintf(&filename, " app=%s", ctx->app);
+    if (ctx->tcurl)
+        av_bprintf(&filename, " tcUrl=%s", ctx->tcurl);
+    if (ctx->pageurl)
+        av_bprintf(&filename, " pageUrl=%s", ctx->pageurl);
+    if (ctx->swfurl)
+        av_bprintf(&filename, " swfUrl=%s", ctx->swfurl);
+    if (ctx->flashver)
+        av_bprintf(&filename, " flashVer=%s", ctx->flashver);
     if (ctx->conn) {
         char *sep, *p = ctx->conn;
-        int options = 0;
-
         while (p) {
-            options++;
+            av_bprintf(&filename,  " conn=");
             p += strspn(p, " ");
             if (!*p)
                 break;
             sep = strchr(p, ' ');
+            if (sep)
+                *sep = '\0';
+            av_bprintf(&filename, "%s", p);
+
             if (sep)
                 p = sep + 1;
             else
                 break;
         }
-        len += options * sizeof(" conn=");
-        len += strlen(ctx->conn);
     }
-
     if (ctx->playpath)
-        len += strlen(ctx->playpath) + sizeof(" playpath=");
+        av_bprintf(&filename, " playpath=%s", ctx->playpath);
     if (ctx->live)
-        len += sizeof(" live=1");
+        av_bprintf(&filename, " live=1");
     if (ctx->subscribe)
-        len += strlen(ctx->subscribe) + sizeof(" subscribe=");
-
+        av_bprintf(&filename, " subscribe=%s", ctx->subscribe);
     if (ctx->client_buffer_time)
-        len += strlen(ctx->client_buffer_time) + sizeof(" buffer=");
-
+        av_bprintf(&filename, " buffer=%s", ctx->client_buffer_time);
     if (ctx->swfurl || ctx->swfverify) {
-        len += sizeof(" swfUrl=");
-
         if (ctx->swfverify)
-            len += strlen(ctx->swfverify) + sizeof(" swfVfy=1");
+            av_bprintf(&filename, " swfUrl=%s swfVfy=1", ctx->swfverify);
         else
-            len += strlen(ctx->swfurl);
+            av_bprintf(&filename, " swfUrl=%s", ctx->swfurl);
     }
 
-    if (!(ctx->temp_filename = filename = av_malloc(len)))
+    if (!av_bprint_is_complete(&filename)) {
+        av_bprint_finalize(&filename, NULL);
         return AVERROR(ENOMEM);
-
-    av_strlcpy(filename, s->filename, len);
-    if (ctx->app) {
-        av_strlcat(filename, " app=", len);
-        av_strlcat(filename, ctx->app, len);
-    }
-    if (ctx->tcurl) {
-        av_strlcat(filename, " tcUrl=", len);
-        av_strlcat(filename, ctx->tcurl, len);
-    }
-    if (ctx->pageurl) {
-        av_strlcat(filename, " pageUrl=", len);
-        av_strlcat(filename, ctx->pageurl, len);
-    }
-    if (ctx->swfurl) {
-        av_strlcat(filename, " swfUrl=", len);
-        av_strlcat(filename, ctx->swfurl, len);
-    }
-    if (ctx->flashver) {
-        av_strlcat(filename, " flashVer=", len);
-        av_strlcat(filename, ctx->flashver, len);
-    }
-    if (ctx->conn) {
-        char *sep, *p = ctx->conn;
-        while (p) {
-            av_strlcat(filename, " conn=", len);
-            p += strspn(p, " ");
-            if (!*p)
-                break;
-            sep = strchr(p, ' ');
-            if (sep)
-                *sep = '\0';
-            av_strlcat(filename, p, len);
-
-            if (sep)
-                p = sep + 1;
-            else
-                break;
-        }
-    }
-    if (ctx->playpath) {
-        av_strlcat(filename, " playpath=", len);
-        av_strlcat(filename, ctx->playpath, len);
-    }
-    if (ctx->live)
-        av_strlcat(filename, " live=1", len);
-    if (ctx->subscribe) {
-        av_strlcat(filename, " subscribe=", len);
-        av_strlcat(filename, ctx->subscribe, len);
-    }
-    if (ctx->client_buffer_time) {
-        av_strlcat(filename, " buffer=", len);
-        av_strlcat(filename, ctx->client_buffer_time, len);
-    }
-    if (ctx->swfurl || ctx->swfverify) {
-        av_strlcat(filename, " swfUrl=", len);
-
-        if (ctx->swfverify) {
-            av_strlcat(filename, ctx->swfverify, len);
-            av_strlcat(filename, " swfVfy=1", len);
-        } else {
-            av_strlcat(filename, ctx->swfurl, len);
-        }
     }
 
     RTMP_Init(r);
-    if (!RTMP_SetupURL(r, filename)) {
+    /* This will modify filename by null terminating the URL portion */
+    if (!RTMP_SetupURL(r, filename.str)) {
         rc = AVERROR_UNKNOWN;
         goto fail;
     }
@@ -247,9 +186,11 @@ static int rtmp_open(URLContext *s, const char *uri, int flags)
 #endif
 
     s->is_streamed = 1;
+    av_bprint_finalize(&filename, NULL);
     return 0;
 fail:
-    av_freep(&ctx->temp_filename);
+    av_bprint_finalize(&filename, NULL);
+
     if (rc)
         RTMP_Close(r);
 
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/1] librtmp: use AVBPrint instead of char *

[Martin Storsjö]

On Mon, 11 Apr 2022, Tristan Matthews wrote:

> This avoids having to do one pass to calculate the full length to allocate
> followed by a second pass to actually append values.
> ---
> libavformat/librtmp.c | 123 +++++++++++-------------------------------
> 1 file changed, 32 insertions(+), 91 deletions(-)

Thanks, this patch looks good to me. I'll wait for a little while still 
if Marton wants to comment on it, before I go ahead and push it.

// Martin

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/1] librtmp: use AVBPrint instead of char *

[Marton Balint]

On Wed, 13 Apr 2022, Martin Storsjö wrote:

> On Mon, 11 Apr 2022, Tristan Matthews wrote:
>
>>  This avoids having to do one pass to calculate the full length to allocate
>>  followed by a second pass to actually append values.
>>  ---
>>  libavformat/librtmp.c | 123 +++++++++++-------------------------------
>>  1 file changed, 32 insertions(+), 91 deletions(-)
>
> Thanks, this patch looks good to me. I'll wait for a little while still if 
> Marton wants to comment on it, before I go ahead and push it.

According to commit 865461099e062de5a3a109c2a5be98004c11d8bd the buffer 
passed to RTMP_SetupURL has to be kept as long as the RTMP context is 
alive.

Regards,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/1] librtmp: use AVBPrint instead of char *

[Martin Storsjö]

On Wed, 13 Apr 2022, Marton Balint wrote:

>
>
> On Wed, 13 Apr 2022, Martin Storsjö wrote:
>
>> On Mon, 11 Apr 2022, Tristan Matthews wrote:
>>
>>>  This avoids having to do one pass to calculate the full length to 
> allocate
>>>  followed by a second pass to actually append values.
>>>  ---
>>>  libavformat/librtmp.c | 123 +++++++++++-------------------------------
>>>  1 file changed, 32 insertions(+), 91 deletions(-)
>>
>> Thanks, this patch looks good to me. I'll wait for a little while still if 
>> Marton wants to comment on it, before I go ahead and push it.
>
> According to commit 865461099e062de5a3a109c2a5be98004c11d8bd the buffer 
> passed to RTMP_SetupURL has to be kept as long as the RTMP context is 
> alive.

Oh, excellent catch, thanks!

// Martin
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/1] librtmp: use AVBPrint instead of char *

[Tristan Matthews]

On Wed, Apr 13, 2022 at 3:40 PM Marton Balint <cus@passwd.hu> wrote:

>
>
> On Wed, 13 Apr 2022, Martin Storsjö wrote:
>
> > On Mon, 11 Apr 2022, Tristan Matthews wrote:
> >
> >>  This avoids having to do one pass to calculate the full length to
> allocate
> >>  followed by a second pass to actually append values.
> >>  ---
> >>  libavformat/librtmp.c | 123 +++++++++++-------------------------------
> >>  1 file changed, 32 insertions(+), 91 deletions(-)
> >
> > Thanks, this patch looks good to me. I'll wait for a little while still
> if
> > Marton wants to comment on it, before I go ahead and push it.
>
> According to commit 865461099e062de5a3a109c2a5be98004c11d8bd the buffer
> passed to RTMP_SetupURL has to be kept as long as the RTMP context is
> alive.
>
>
Oh good catch, I should've dug deeper as to why ctx->temp_filename existed.

Best,
Tristan



> Regards,
> Marton
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 1/1] librtmp: use AVBPrint instead of char *

[Tristan Matthews]

This avoids having to do one pass to calculate the full length to allocate
followed by a second pass to actually append values.
---
 libavformat/librtmp.c | 124 +++++++++++-------------------------------
 1 file changed, 33 insertions(+), 91 deletions(-)

diff --git a/libavformat/librtmp.c b/libavformat/librtmp.c
index 43013e46e0..b7e9fc81cf 100644
--- a/libavformat/librtmp.c
+++ b/libavformat/librtmp.c
@@ -25,6 +25,7 @@
  */
 
 #include "libavutil/avstring.h"
+#include "libavutil/bprint.h"
 #include "libavutil/mathematics.h"
 #include "libavutil/opt.h"
 #include "avformat.h"
@@ -38,6 +39,7 @@
 
 typedef struct LibRTMPContext {
     const AVClass *class;
+    AVBPrint filename;
     RTMP rtmp;
     char *app;
     char *conn;
@@ -50,7 +52,6 @@ typedef struct LibRTMPContext {
     char *pageurl;
     char *client_buffer_time;
     int live;
-    char *temp_filename;
     int buffer_size;
 } LibRTMPContext;
 
@@ -76,7 +77,7 @@ static int rtmp_close(URLContext *s)
     RTMP *r = &ctx->rtmp;
 
     RTMP_Close(r);
-    av_freep(&ctx->temp_filename);
+    av_bprint_finalize(&ctx->filename, NULL);
     return 0;
 }
 
@@ -97,8 +98,8 @@ static int rtmp_open(URLContext *s, const char *uri, int flags)
     LibRTMPContext *ctx = s->priv_data;
     RTMP *r = &ctx->rtmp;
     int rc = 0, level;
-    char *filename = s->filename;
-    int len = strlen(s->filename) + 1;
+    /* This needs to stay allocated for as long as the RTMP context exists. */
+    av_bprint_init(&ctx->filename, 0, AV_BPRINT_SIZE_UNLIMITED);
 
     switch (av_log_get_level()) {
     default:
@@ -112,118 +113,58 @@ static int rtmp_open(URLContext *s, const char *uri, int flags)
     RTMP_LogSetLevel(level);
     RTMP_LogSetCallback(rtmp_log);
 
-    if (ctx->app)      len += strlen(ctx->app)      + sizeof(" app=");
-    if (ctx->tcurl)    len += strlen(ctx->tcurl)    + sizeof(" tcUrl=");
-    if (ctx->pageurl)  len += strlen(ctx->pageurl)  + sizeof(" pageUrl=");
-    if (ctx->flashver) len += strlen(ctx->flashver) + sizeof(" flashver=");
-
+    av_bprintf(&ctx->filename, "%s", s->filename);
+    if (ctx->app)
+        av_bprintf(&ctx->filename, " app=%s", ctx->app);
+    if (ctx->tcurl)
+        av_bprintf(&ctx->filename, " tcUrl=%s", ctx->tcurl);
+    if (ctx->pageurl)
+        av_bprintf(&ctx->filename, " pageUrl=%s", ctx->pageurl);
+    if (ctx->swfurl)
+        av_bprintf(&ctx->filename, " swfUrl=%s", ctx->swfurl);
+    if (ctx->flashver)
+        av_bprintf(&ctx->filename, " flashVer=%s", ctx->flashver);
     if (ctx->conn) {
         char *sep, *p = ctx->conn;
-        int options = 0;
-
         while (p) {
-            options++;
+            av_bprintf(&ctx->filename,  " conn=");
             p += strspn(p, " ");
             if (!*p)
                 break;
             sep = strchr(p, ' ');
+            if (sep)
+                *sep = '\0';
+            av_bprintf(&ctx->filename, "%s", p);
+
             if (sep)
                 p = sep + 1;
             else
                 break;
         }
-        len += options * sizeof(" conn=");
-        len += strlen(ctx->conn);
     }
-
     if (ctx->playpath)
-        len += strlen(ctx->playpath) + sizeof(" playpath=");
+        av_bprintf(&ctx->filename, " playpath=%s", ctx->playpath);
     if (ctx->live)
-        len += sizeof(" live=1");
+        av_bprintf(&ctx->filename, " live=1");
     if (ctx->subscribe)
-        len += strlen(ctx->subscribe) + sizeof(" subscribe=");
-
+        av_bprintf(&ctx->filename, " subscribe=%s", ctx->subscribe);
     if (ctx->client_buffer_time)
-        len += strlen(ctx->client_buffer_time) + sizeof(" buffer=");
-
+        av_bprintf(&ctx->filename, " buffer=%s", ctx->client_buffer_time);
     if (ctx->swfurl || ctx->swfverify) {
-        len += sizeof(" swfUrl=");
-
         if (ctx->swfverify)
-            len += strlen(ctx->swfverify) + sizeof(" swfVfy=1");
+            av_bprintf(&ctx->filename, " swfUrl=%s swfVfy=1", ctx->swfverify);
         else
-            len += strlen(ctx->swfurl);
+            av_bprintf(&ctx->filename, " swfUrl=%s", ctx->swfurl);
     }
 
-    if (!(ctx->temp_filename = filename = av_malloc(len)))
+    if (!av_bprint_is_complete(&ctx->filename)) {
+        av_bprint_finalize(&ctx->filename, NULL);
         return AVERROR(ENOMEM);
-
-    av_strlcpy(filename, s->filename, len);
-    if (ctx->app) {
-        av_strlcat(filename, " app=", len);
-        av_strlcat(filename, ctx->app, len);
-    }
-    if (ctx->tcurl) {
-        av_strlcat(filename, " tcUrl=", len);
-        av_strlcat(filename, ctx->tcurl, len);
-    }
-    if (ctx->pageurl) {
-        av_strlcat(filename, " pageUrl=", len);
-        av_strlcat(filename, ctx->pageurl, len);
-    }
-    if (ctx->swfurl) {
-        av_strlcat(filename, " swfUrl=", len);
-        av_strlcat(filename, ctx->swfurl, len);
-    }
-    if (ctx->flashver) {
-        av_strlcat(filename, " flashVer=", len);
-        av_strlcat(filename, ctx->flashver, len);
-    }
-    if (ctx->conn) {
-        char *sep, *p = ctx->conn;
-        while (p) {
-            av_strlcat(filename, " conn=", len);
-            p += strspn(p, " ");
-            if (!*p)
-                break;
-            sep = strchr(p, ' ');
-            if (sep)
-                *sep = '\0';
-            av_strlcat(filename, p, len);
-
-            if (sep)
-                p = sep + 1;
-            else
-                break;
-        }
-    }
-    if (ctx->playpath) {
-        av_strlcat(filename, " playpath=", len);
-        av_strlcat(filename, ctx->playpath, len);
-    }
-    if (ctx->live)
-        av_strlcat(filename, " live=1", len);
-    if (ctx->subscribe) {
-        av_strlcat(filename, " subscribe=", len);
-        av_strlcat(filename, ctx->subscribe, len);
-    }
-    if (ctx->client_buffer_time) {
-        av_strlcat(filename, " buffer=", len);
-        av_strlcat(filename, ctx->client_buffer_time, len);
-    }
-    if (ctx->swfurl || ctx->swfverify) {
-        av_strlcat(filename, " swfUrl=", len);
-
-        if (ctx->swfverify) {
-            av_strlcat(filename, ctx->swfverify, len);
-            av_strlcat(filename, " swfVfy=1", len);
-        } else {
-            av_strlcat(filename, ctx->swfurl, len);
-        }
     }
 
     RTMP_Init(r);
-    if (!RTMP_SetupURL(r, filename)) {
+    /* This will modify filename by null terminating the URL portion */
+    if (!RTMP_SetupURL(r, ctx->filename.str)) {
         rc = AVERROR_UNKNOWN;
         goto fail;
     }
@@ -249,9 +190,10 @@ static int rtmp_open(URLContext *s, const char *uri, int flags)
     s->is_streamed = 1;
     return 0;
 fail:
-    av_freep(&ctx->temp_filename);
+
     if (rc)
         RTMP_Close(r);
+    av_bprint_finalize(&ctx->filename, NULL);
 
     return rc;
 }
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/1] librtmp: use AVBPrint instead of char *

[Martin Storsjö]

On Fri, 15 Apr 2022, Tristan Matthews wrote:

> This avoids having to do one pass to calculate the full length to allocate
> followed by a second pass to actually append values.
> ---
> libavformat/librtmp.c | 124 +++++++++++-------------------------------
> 1 file changed, 33 insertions(+), 91 deletions(-)
>
> diff --git a/libavformat/librtmp.c b/libavformat/librtmp.c
> index 43013e46e0..b7e9fc81cf 100644
> --- a/libavformat/librtmp.c
> +++ b/libavformat/librtmp.c
> @@ -25,6 +25,7 @@
>  */
>
> #include "libavutil/avstring.h"
> +#include "libavutil/bprint.h"
> #include "libavutil/mathematics.h"
> #include "libavutil/opt.h"
> #include "avformat.h"
> @@ -38,6 +39,7 @@
>
> typedef struct LibRTMPContext {
>     const AVClass *class;
> +    AVBPrint filename;
>     RTMP rtmp;
>     char *app;
>     char *conn;
> @@ -50,7 +52,6 @@ typedef struct LibRTMPContext {
>     char *pageurl;
>     char *client_buffer_time;
>     int live;
> -    char *temp_filename;
>     int buffer_size;
> } LibRTMPContext;

This looks ok to me.

(I guess it also could be considered viable to not store the AVBprint in 
the context but detach an allocated string instead; that would decrease 
the persistent allocation size a little, at the cost of some more copying, 
but the difference is probably negligible, so this probably is sensible as 
is.)

Let's still wait for Marton's review too.

// Martin

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/1] librtmp: use AVBPrint instead of char *

[Marton Balint]

On Sat, 16 Apr 2022, Martin Storsjö wrote:

> On Fri, 15 Apr 2022, Tristan Matthews wrote:
>
>>  This avoids having to do one pass to calculate the full length to allocate
>>  followed by a second pass to actually append values.
>>  ---
>>  libavformat/librtmp.c | 124 +++++++++++-------------------------------
>>  1 file changed, 33 insertions(+), 91 deletions(-)
>>
>>  diff --git a/libavformat/librtmp.c b/libavformat/librtmp.c
>>  index 43013e46e0..b7e9fc81cf 100644
>>  --- a/libavformat/librtmp.c
>>  +++ b/libavformat/librtmp.c
>>  @@ -25,6 +25,7 @@
>>   */
>>
>>  #include "libavutil/avstring.h"
>>  +#include "libavutil/bprint.h"
>>  #include "libavutil/mathematics.h"
>>  #include "libavutil/opt.h"
>>  #include "avformat.h"
>>  @@ -38,6 +39,7 @@
>>
>>  typedef struct LibRTMPContext {
>>      const AVClass *class;
>>  +    AVBPrint filename;
>>      RTMP rtmp;
>>      char *app;
>>      char *conn;
>>  @@ -50,7 +52,6 @@ typedef struct LibRTMPContext {
>>      char *pageurl;
>>      char *client_buffer_time;
>>      int live;
>>  -    char *temp_filename;
>>      int buffer_size;
>>  } LibRTMPContext;
>
> This looks ok to me.
>
> (I guess it also could be considered viable to not store the AVBprint in the 
> context but detach an allocated string instead; that would decrease the 
> persistent allocation size a little, at the cost of some more copying, but 
> the difference is probably negligible, so this probably is sensible as is.)

Agreed.

>
> Let's still wait for Marton's review too.

LGTM as well. Thanks.

Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/1] librtmp: use AVBPrint instead of char *

[Martin Storsjö]

On Tue, 19 Apr 2022, Marton Balint wrote:

>
>
> On Sat, 16 Apr 2022, Martin Storsjö wrote:
>
>> On Fri, 15 Apr 2022, Tristan Matthews wrote:
>>
>>>  This avoids having to do one pass to calculate the full length to 
> allocate
>>>  followed by a second pass to actually append values.
>>>  ---
>>>  libavformat/librtmp.c | 124 +++++++++++-------------------------------
>>>  1 file changed, 33 insertions(+), 91 deletions(-)
>>>
>>>  diff --git a/libavformat/librtmp.c b/libavformat/librtmp.c
>>>  index 43013e46e0..b7e9fc81cf 100644
>>>  --- a/libavformat/librtmp.c
>>>  +++ b/libavformat/librtmp.c
>>>  @@ -25,6 +25,7 @@
>>>   */
>>>
>>>  #include "libavutil/avstring.h"
>>>  +#include "libavutil/bprint.h"
>>>  #include "libavutil/mathematics.h"
>>>  #include "libavutil/opt.h"
>>>  #include "avformat.h"
>>>  @@ -38,6 +39,7 @@
>>>
>>>  typedef struct LibRTMPContext {
>>>      const AVClass *class;
>>>  +    AVBPrint filename;
>>>      RTMP rtmp;
>>>      char *app;
>>>      char *conn;
>>>  @@ -50,7 +52,6 @@ typedef struct LibRTMPContext {
>>>      char *pageurl;
>>>      char *client_buffer_time;
>>>      int live;
>>>  -    char *temp_filename;
>>>      int buffer_size;
>>>  } LibRTMPContext;
>>
>> This looks ok to me.
>>
>> (I guess it also could be considered viable to not store the AVBprint in 
> the 
>> context but detach an allocated string instead; that would decrease the 
>> persistent allocation size a little, at the cost of some more copying, but 
>> the difference is probably negligible, so this probably is sensible as is.)
>
> Agreed.
>
>>
>> Let's still wait for Marton's review too.
>
> LGTM as well. Thanks.

Pushed now then.

// Martin
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".