diff mbox series

[FFmpeg-devel,1/2] lavf/avio: add avio_vprintf()

Message ID 20210418213058.24475-1-stefasab@gmail.com
State New
Headers show
Series [FFmpeg-devel,1/2] lavf/avio: add avio_vprintf() | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Stefano Sabatini April 18, 2021, 9:30 p.m. UTC 
This new function makes it possible to use avio_printf() functionality from
a function taking a variable list of arguments.
---
 doc/APIchanges        |  3 +++
 libavformat/avio.h    |  6 ++++++
 libavformat/aviobuf.c | 17 +++++++++++++----
 libavformat/version.h |  2 +-
 4 files changed, 23 insertions(+), 5 deletions(-)

Comments

Stefano Sabatini April 21, 2021, 9:53 p.m. UTC | #1 
On date Sunday 2021-04-18 23:30:57 +0200, Stefano Sabatini wrote:
> This new function makes it possible to use avio_printf() functionality from
> a function taking a variable list of arguments.
> ---
>  doc/APIchanges        |  3 +++
>  libavformat/avio.h    |  6 ++++++
>  libavformat/aviobuf.c | 17 +++++++++++++----
>  libavformat/version.h |  2 +-
>  4 files changed, 23 insertions(+), 5 deletions(-)

Updated against master.
Stefano Sabatini April 3, 2022, 2:03 p.m. UTC | #2 
On date Wednesday 2021-04-21 23:53:35 +0200, Stefano Sabatini wrote:
> On date Sunday 2021-04-18 23:30:57 +0200, Stefano Sabatini wrote:
> > This new function makes it possible to use avio_printf() functionality from
> > a function taking a variable list of arguments.
> > ---
> >  doc/APIchanges        |  3 +++
> >  libavformat/avio.h    |  6 ++++++
> >  libavformat/aviobuf.c | 17 +++++++++++++----
> >  libavformat/version.h |  2 +-
> >  4 files changed, 23 insertions(+), 5 deletions(-)
> 
> Updated against master.

Updated.
Marton Balint June 9, 2022, 7:03 p.m. UTC | #3 
On Sun, 3 Apr 2022, Stefano Sabatini wrote:

> On date Wednesday 2021-04-21 23:53:35 +0200, Stefano Sabatini wrote:
>> On date Sunday 2021-04-18 23:30:57 +0200, Stefano Sabatini wrote:
>>> This new function makes it possible to use avio_printf() functionality from
>>> a function taking a variable list of arguments.
>>> ---
>>>  doc/APIchanges        |  3 +++
>>>  libavformat/avio.h    |  6 ++++++
>>>  libavformat/aviobuf.c | 17 +++++++++++++----
>>>  libavformat/version.h |  2 +-
>>>  4 files changed, 23 insertions(+), 5 deletions(-)
>>
>> Updated against master.
>
> Updated.
>

Will rebase and apply soon.

Regards,
Marton
Stefano Sabatini June 12, 2022, 3:32 p.m. UTC | #4 
On date Thursday 2022-06-09 21:03:02 +0200, Marton Balint wrote:
> 
> 
> On Sun, 3 Apr 2022, Stefano Sabatini wrote:
> 
> > On date Wednesday 2021-04-21 23:53:35 +0200, Stefano Sabatini wrote:
> > > On date Sunday 2021-04-18 23:30:57 +0200, Stefano Sabatini wrote:
> > > > This new function makes it possible to use avio_printf() functionality from
> > > > a function taking a variable list of arguments.
> > > > ---
> > > >  doc/APIchanges        |  3 +++
> > > >  libavformat/avio.h    |  6 ++++++
> > > >  libavformat/aviobuf.c | 17 +++++++++++++----
> > > >  libavformat/version.h |  2 +-
> > > >  4 files changed, 23 insertions(+), 5 deletions(-)
> > > 
> > > Updated against master.
> > 
> > Updated.
> > 
> 
> Will rebase and apply soon.

Updated.
Nicolas George June 12, 2022, 5 p.m. UTC | #5 
Stefano Sabatini (12022-06-12):
> Updated.

Hi. Since it is somewhat related, I would appreciate if you gave your
opinion on the option of having a good string API in FFmpeg:

https://ffmpeg.org/pipermail/ffmpeg-devel/2021-December/290226.html

Eventually, I would like to have the more generic writers from ffprobe
(JSON, XML) in lavu to be used by diagnostic filters. But I will not be
doing that with pedestrian strings.

Regards,
Soft Works June 16, 2022, 4:23 p.m. UTC | #6 
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Nicolas George
> Sent: Sunday, June 12, 2022 7:01 PM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] lavf/avio: add avio_vprintf()
> 
> Stefano Sabatini (12022-06-12):
> > Updated.
> 
> Hi. Since it is somewhat related, I would appreciate if you gave your
> opinion on the option of having a good string API in FFmpeg:
> 
> https://ffmpeg.org/pipermail/ffmpeg-devel/2021-December/290226.html
> 
> Eventually, I would like to have the more generic writers from
> ffprobe
> (JSON, XML) in lavu to be used by diagnostic filters. But I will not
> be
> doing that with pedestrian strings.

Hi,

as mentioned a while ago, I am quite interested in this and I would be 
glad when it would be possible to find a way for going forward.
My primary interest would be the generalization of ffprobe string
writing code, from which I have two duplicates by now. I had put further
work on hold as I became aware that were plans or proposals for this.

Now I've read your "Intro to AVWriter" from April 2021 and I have some 
questions.


> ## Scenario 2: Concatenating the string into a message.
> 
> Let us say you have two foobars to present to the user. Good old C:
> 
>   char msg[2500], foo1_buf[1000], foo2_buf[1000];
>   av_foobar_to_string(foo1_buf, sizeof(foo1_buf), foo1);
>   av_foobar_to_string(foo2_buf, sizeof(foo2_buf), foo2);
>   snprintf(msg, sizeof(msg), "F%d = [ %s, %s ]", num, foo1_buf, foo2_buf);
> 
> But it's ugly. Less ugly, but more complicated:

[skip that version]

> Well, that was the first thing AVWriter was meant to do: allow to build
> strings by concatenating them together. So, the AVWraper version of this
> code:
> 
>   char msg[2500];
>   AVWriter wr = av_buf_writer_array(msg);
>   av_writer_printf(wr, "F%d = [ ", num);
>   av_foobar_write(wr, foo1);
>   av_writer_print(wr, ", ");
>   av_foobar_write(wr, foo2);
>   av_writer_print(wr, " ]");

There are cases where it might be preferable to subsequently append to 
a buffer like that, but it requires many lines of code and it's not as 
easy to get a picture of how the produced string will actually look like.
Maybe you meant it just as one possible example to use AVWriter, but in
this context it looks like as if that's the supposed way to replace the
"Good old C" version?

When you would ask me which code I would _wish_ to be able to write 
in this example case, then it would be this:

  char msg[2500];
  snprintf(msg, sizeof(msg), "F%d = [ %s, %s ]", num, foo1, foo2);


Of course it doesn't need to be snprintf, it could also be av_xprintf().
It would also be fine to have some macro around those params like 
XX(foo1). Or a custom format specifier would also be fine.
Basically I think there are a number of possible ways, but I'm not sure
whether this would be possible with regards to the automatisms you
are planning to checking for a matching string conversion function?

This might be a tough one: it would be cool when this could be used
in av_log() calls for logging, but in this case it would be of course 
required that the string writing will be performed upstream, only when 
the log level condition would be satisfied. Could that even work?


PS: Yes, I have read it to the end; the mentioned av_writer_fmt()
might be related to what I'm asking, but I wasn't sure.

Thanks,
softworkz
diff mbox series

Patch

diff --git a/doc/APIchanges b/doc/APIchanges
index b41dadee8d..08fec7c234 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,9 @@  libavutil:     2017-10-21
 
 API changes, most recent first:
 
+2021-04-18 - xxxxxxxxxx - lavf 58.78.100 - avio.h
+  Add avio_vprintf(), similar to avio_printf().
+
 2021-03-21 - xxxxxxxxxx - lavu 56.72.100 - frame.h
   Deprecated av_get_colorspace_name().
   Use av_color_space_name() instead.
diff --git a/libavformat/avio.h b/libavformat/avio.h
index d022820a6e..24f6839522 100644
--- a/libavformat/avio.h
+++ b/libavformat/avio.h
@@ -571,6 +571,12 @@  int64_t avio_size(AVIOContext *s);
  */
 int avio_feof(AVIOContext *s);
 
+/**
+ * Writes a formatted string to the context taking a va_list.
+ * @return number of bytes written, < 0 on error.
+ */
+int avio_vprintf(AVIOContext *s, const char *fmt, va_list ap);
+
 /**
  * Writes a formatted string to the context.
  * @return number of bytes written, < 0 on error.
diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 518cb11129..289da796c8 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -1196,15 +1196,12 @@  int avio_closep(AVIOContext **s)
     return ret;
 }
 
-int avio_printf(AVIOContext *s, const char *fmt, ...)
+int avio_vprintf(AVIOContext *s, const char *fmt, va_list ap)
 {
-    va_list ap;
     AVBPrint bp;
 
     av_bprint_init(&bp, 0, INT_MAX);
-    va_start(ap, fmt);
     av_vbprintf(&bp, fmt, ap);
-    va_end(ap);
     if (!av_bprint_is_complete(&bp)) {
         av_bprint_finalize(&bp, NULL);
         s->error = AVERROR(ENOMEM);
@@ -1215,6 +1212,18 @@  int avio_printf(AVIOContext *s, const char *fmt, ...)
     return bp.len;
 }
 
+int avio_printf(AVIOContext *s, const char *fmt, ...)
+{
+    va_list ap;
+    int ret;
+
+    va_start(ap, fmt);
+    ret = avio_vprintf(s, fmt, ap);
+    va_end(ap);
+
+    return ret;
+}
+
 void avio_print_string_array(AVIOContext *s, const char *strings[])
 {
     for(; *strings; strings++)
diff --git a/libavformat/version.h b/libavformat/version.h
index ced5600034..b6023f9d2e 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -32,7 +32,7 @@ 
 // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium)
 // Also please add any ticket numbers that you believe might be affected here
 #define LIBAVFORMAT_VERSION_MAJOR  58
-#define LIBAVFORMAT_VERSION_MINOR  77
+#define LIBAVFORMAT_VERSION_MINOR  78
 #define LIBAVFORMAT_VERSION_MICRO 100
 
 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \