[FFmpeg-devel] [PATCH 25/35] avutil/opt: add av_opt_to_string

[Diederick C. Niehorster]

On Thu, Jun 10, 2021 at 12:08 PM Diederick C. Niehorster
<dcnieho at gmail.com> wrote:
>
> On Thu, Jun 10, 2021 at 11:39 AM Diederick C. Niehorster
> <dcnieho at gmail.com> wrote:
> > So in av_opt_get(), I'd do something like this:
> > AVBPrint buf;
> > av_bprint_init(&buf, 0, AV_BPRINT_SIZE_AUTOMATIC);
> > // 1. call internal print function, with &buf
> > // ...
> > // 2. provide output to caller
> > if (!av_bprint_is_complete(&buf))
> > {
> >     av_bprint_finalize(&buf,NULL);
> >     return AVERROR(ENOMEM);
> > }
> > ret = av_bprint_finalize(&buf,out_val);
> > return ret<0 ? ret : 0;
> >
> > I'll experiment with that.
>
> Ok, i have experimented with this, and hit some problems quickly.
> There are at least the following issues:
> 1. how to deal with AV_OPT_ALLOW_NULL in av_opt_get, if its guts use a
> move over to AVBPrint? leaving AVBPrint empty is not the same as a
> null, and does not signal null.
> 2. some cases forward to other functions, most importantly
> AV_OPT_TYPE_DICT, which uses av_dict_get_string. These do not play
> with AVBPrint, but just plain char** or similar. Should i use a local
> buffer in that case and then copy/print its contents into the
> AVBPrint? That seems to not improve the situation.
> This is assuming we'd want to reuse the guts of av_opt_get for this
> new function, which i think we would. Not doing so would mean a lot of
> code duplication.
>
> And there is also that this would be the only av_opt_* function
> returning output through a AVBPrint instead of plain arrays. Is that
> inconsistency in the API worth usually avoiding a dynamic allocation
> in code that'll not be used in a critical path?

I've just sent v2 of the patch set. Given what i have written above, i
have not moved the function to using AVBPrint, so i can reuse
av_opt_get internals and don't introduce inconsistency in the options
API. That said, moving it _all_ over to your better strings API
proposal[1] sounds like a pretty sweet thing to do!

I also hear you on your idea for better options. While the function i
add here allows me to format pixel_format and sample_formats correctly
because they have their own options type, I still have the issue that
another option "codec" is just printed as int, as there is no
AV_OPT_TYPE_CODEC, or otherwise information that it should be turned
into a codec name... Hope that something like your proposal will make
that a thing of the past.

Cheers,
Dee