Re: [FFmpeg-devel] [PATCH 1/1] librtmp: use AVBPrint instead of char *
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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/1] librtmp: use AVBPrint instead of char *
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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/1] librtmp: use AVBPrint instead of char *
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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/1] librtmp: use AVBPrint instead of char *
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 = >rtmp;
RTMP_Close(r);
-av_freep(>temp_filename);
+av_bprint_finalize(>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 = >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(>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(>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);
-
Re: [FFmpeg-devel] [PATCH 1/1] librtmp: use AVBPrint instead of char *
On Wed, Apr 13, 2022 at 3:40 PM 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 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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/1] librtmp: use AVBPrint instead of char *
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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/1] librtmp: use AVBPrint instead of char *
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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/1] librtmp: use AVBPrint instead of char *
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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/1] librtmp: use AVBPrint instead of char *
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 = >rtmp;
RTMP_Close(r);
-av_freep(>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 = >rtmp;
int rc = 0, level;
-char *filename = s->filename;
-int len = strlen(s->filename) + 1;
+av_bprint_init(, 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(, "%s", s->filename);
+if (ctx->app)
+av_bprintf(, " app=%s", ctx->app);
+if (ctx->tcurl)
+av_bprintf(, " tcUrl=%s", ctx->tcurl);
+if (ctx->pageurl)
+av_bprintf(, " pageUrl=%s", ctx->pageurl);
+if (ctx->swfurl)
+av_bprintf(, " swfUrl=%s", ctx->swfurl);
+if (ctx->flashver)
+av_bprintf(, " flashVer=%s", ctx->flashver);
if (ctx->conn) {
char *sep, *p = ctx->conn;
-int options = 0;
-
while (p) {
-options++;
+av_bprintf(, " conn=");
p += strspn(p, " ");
if (!*p)
break;
sep = strchr(p, ' ');
+if (sep)
+*sep = '\0';
+av_bprintf(, "%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(, " playpath=%s", ctx->playpath);
if (ctx->live)
-len += sizeof(" live=1");
+av_bprintf(, " live=1");
if (ctx->subscribe)
-len += strlen(ctx->subscribe) + sizeof(" subscribe=");
-
+av_bprintf(, " subscribe=%s", ctx->subscribe);
if (ctx->client_buffer_time)
-len += strlen(ctx->client_buffer_time) + sizeof(" buffer=");
-
+av_bprintf(, " 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(, " swfUrl=%s swfVfy=1", ctx->swfverify);
else
-len += strlen(ctx->swfurl);
+av_bprintf(, " swfUrl=%s", ctx->swfurl);
}
-if (!(ctx->temp_filename = filename = av_malloc(len)))
+if (!av_bprint_is_complete()) {
+av_bprint_finalize(, 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) {
-
