[FFmpeg-devel] AVWriter API

[Nicolas George]

Hi. Thanks for looking into it with an open mind.

Hendrik Leppkes (12020-01-12):
> While this argument sounds sensible on the surface, I can't help but
> wonder if it really makes sense.
> Sure, some returned strings may be able to be wrapped straight into
> whatever structure they use. But what about  const char* strings? Will
> we need to allocate a buffer and copy those in the future? Considering
> I can practically use those inline in logging printfs now, that would
> be an extreme increase in complexity.

I want to apply consistency with pragmatism, not with dogmatism. If what
we have at hand is a static const char *, the of course that is what we
return: it is the simplest and the most efficient. It is especially true
for enum → string functions: av_foobar_get_string(). (Note that you
can't use them really directly in printf, since "%s", NULL can
segfault.)

But in this case, let's also provide the av_foobar_write() variant,
because all it costs is a macro call in the header:
FF_DEFINE_WRITE_FROM_TO_STRING(av_foobar, AVFoobar, "unknown")

But as soon as a function needs to build a string in a buffer, let us
use this, because it is a very simple solution to let the programme
choose what kind of buffer to use.

> Also, what happens to strings that are an input to our libraries? Will
> those no longer be char pointers, possibly malloc'ed (depending on
> where they are used)?

For this, there are too many cases, I have yet to find a unifying
solution. Note that a lot of string implementations can give read-only
access to their internal buffer, which means we can accept const char *
and let the application decide how to allocate it.

> Additionally, any widely used custom string class will have a direct
> conversion from/to char pointers, so is this really such a huge burden
> on application developers? I work on Windows, where everything is
> wchar_t, and I never thought this was a problem, since I have to deal
> with this for every library. In fact the proposed API won't really
> help, since the entire API is still designed around UTF-8, so what I
> would do is wrap the dynbuf writter and just convert to wide-char when
> the string is complete. In short, I do exactly what I do now, just
> more complicated.

I am not sure what your point is about wchar_t, but if you think it
would be useful to provide AVWBufWriter and AVWDynbufWriter,
counterparts to AVBufWriter and AVDynbufWriter that output into wchar_t
buffers, it is entirely possible to do it.

Sure, we can continue doing what we have been doing: allocate a string,
fill it, allocate a new string, convert, allocate yet another string,
copy. This is what Java programmers do all day. But if we use Java
patterns in C, we get the drawbacks of Java patterns plus the fact that
the C allocator is not optimized for that.

My proposal is not only rooted in convenience, it is also rooted in
efficiency. String operations are not part of the inner loops of codecs,
but we still make an awful lot of it. For example,
av_get_channel_layout_string() can be called by verbose code for every
audio frame (cf. af_ashowinfo), that can make thousands of times per
second for low-latency codecs. It's a shame to do a malloc() only to
free it immediately and have the log layer discard the string anyway.

You may not have noticed this, but when you use av_dynbuf_writer(),
which is the functional equivalent of returning a mallocated buffer and
the recommended way, it will first use a buffer on stack, and only use
malloc() if it is not enough. Anton's implementation of
av_channel_layout_describe() always makes a malloc; with AVWriter, it
would only do so if there are more than 200 channels or the caller wants
a permanent copy of the string.

I think this consideration and the factoring of error check are enough
to warrant the little bit of complication for the caller. And the
possibility to output to many kinds of strings comes on top of that.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200114/c1821652/attachment.sig>