On Wed, 7 Aug 2019, Alexander Strasser wrote:
Hi Marton!
Not really sure if we need the API, but it definitely looks
handy. Just not sure how often it will used and really result
in clearer or shorter code.
It has better performance than using av_printf because it does not need a
temporary buffer.
Not opposed to it though; no strong opinion on this one.
Some comments follow inline. No in depth review, just what
came to my mind when reading your patch quickly.
On 2019-08-05 23:34 +0200, Marton Balint wrote:
These functions can be used to print a variable number of strings consecutively
to the IO context. Unlike av_bprintf, no temporery buffer is necessary.
Small typo here: temporary
Fixed locally.
Signed-off-by: Marton Balint <c...@passwd.hu>
---
doc/APIchanges | 3 +++
libavformat/avio.h | 16 ++++++++++++++++
libavformat/aviobuf.c | 17 +++++++++++++++++
libavformat/version.h | 2 +-
4 files changed, 37 insertions(+), 1 deletion(-)
diff --git a/doc/APIchanges b/doc/APIchanges
index 6603a8229e..0b19fb067d 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,9 @@ libavutil: 2017-10-21
API changes, most recent first:
+2019-08-xx - xxxxxxxxxx - lavf 58.31.100 - avio.h
+ Add avio_print_n_strings and avio_print.
+
2019-07-27 - xxxxxxxxxx - lavu 56.33.100 - tx.h
Add AV_TX_DOUBLE_FFT and AV_TX_DOUBLE_MDCT
diff --git a/libavformat/avio.h b/libavformat/avio.h
index dcb8dcdf93..ca08907917 100644
--- a/libavformat/avio.h
+++ b/libavformat/avio.h
@@ -574,6 +574,22 @@ int avio_feof(AVIOContext *s);
/** @warning Writes up to 4 KiB per call */
int avio_printf(AVIOContext *s, const char *fmt, ...) av_printf_format(2, 3);
+/**
+ * Write nb_strings number of strings (const char *) to the context.
+ * @return the total number of bytes written
+ */
+int avio_print_n_strings(AVIOContext *s, int nb_strings, ...);
+
+/**
+ * Write strings (const char *) to the context.
+ * This is a macro around avio_print_n_strings but the number of strings to be
+ * written is determined automatically at compile time based on the number of
+ * parameters passed to this macro. For simple string concatenations this
+ * function is more performant than using avio_printf.
+ */
+#define avio_print(s, ...) \
+ avio_print_n_strings(s, sizeof((const char*[]){__VA_ARGS__}) /
sizeof(const char*), __VA_ARGS__)
If we don't have this pattern in the code base already, it would
be better to compile and test on bunch of different compilers.
I tested a few compilers (Clang, GCC, MSVC 2015) on https://godbolt.org/
and it seems to work.
+
/**
* Force flushing of buffered data.
*
diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 2d011027c9..c37b056b64 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -1225,6 +1225,23 @@ int avio_printf(AVIOContext *s, const char *fmt, ...)
return ret;
}
+int avio_print_n_strings(AVIOContext *s, int nb_strings, ...)
+{
+ va_list ap;
+ int ret = 0;
+
+ va_start(ap, nb_strings);
+ for (int i = 0; i < nb_strings; i++) {
+ const char *str = va_arg(ap, const char *);
+ int len = strlen(str);
+ avio_write(s, (const unsigned char *)str, len);
+ ret += len;
I first wanted to say you should compute with the count returned
by avio_write; then after diving into libavformat/avio_buf and
reading a cascade of void functions and seeing signalling via
field error in the context which also is sometimes (mis)used
to store a length(?), I withdraw that comment.
Anyway you might consider using void for this function too,
otherwise I guess you should figure out how to do the error
checking inside of this function because if an error occurs
that call might have been partial and the following writes will
effectively not occur. So I'm feeling rather uncomfortable
with returning a count here. Maybe my stance is to narrow.
It returns int because avio_printf also returns int, but since no error
(other than IO error) can happen, maybe it is better to return void the
same way as avio_write functions do. For IO errors the user should always
check the IO context error flag, that is by design as far as I know.
I'll change it to return void.
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".
