[FFmpeg-devel] [RFC] opt.h cleanup?

[Stefano Sabatini]

I'm having a look at the opt.h file, and I'm tempted to clean it up a
bit, and while at it try to spot possible design problems.

A list of possible changes follows:

const AVOption *av_find_opt(void *obj, const char *name, const char *unit, int mask, int flags);
=>
const AVOption *av_opt_find(void *obj, const char *name, const char *unit, int mask, int flags);

not very clear the meaning of unit though.

int av_set_string3   (void *obj, const char *name, const char *val, int alloc, const AVOption **o_out);
=>
int av_opt_set_string(void *obj, const char *name, const char *val, int alloc, const AVOption **o_out);

or maybe av_opt_set() since this is really a general setter, not sure
the alloc param is a good idea.

Maybe having a distinct av_opt_set_string() (for setting a *string*
value) and an av_opt_set() (for setting a string representing a value)
may help.

const AVOption *av_set_double(void *obj, const char *name, double n);
const AVOption *av_set_q     (void *obj, const char *name, AVRational n);
const AVOption *av_set_int   (void *obj, const char *name, int64_t n);
=>
const AVOption *av_opt_set_double(void *obj, const char *name, double n);
const AVOption *av_opt_set_q     (void *obj, const char *name, AVRational n);
const AVOption *av_opt_set_int   (void *obj, const char *name, int64_t n);

the interface seems basically fine to me.

double     av_get_double(void *obj, const char *name, const AVOption **o_out);
AVRational av_get_q     (void *obj, const char *name, const AVOption **o_out);
int64_t    av_get_int   (void *obj, const char *name, const AVOption **o_out);
=>
double     av_opt_get_double(void *obj, const char *name, const AVOption **o_out);
AVRational av_opt_get_q     (void *obj, const char *name, const AVOption **o_out);
int64_t    av_opt_get_int   (void *obj, const char *name, const AVOption **o_out);

the interface looks basically good

const char *av_get_string    (void *obj, const char *name, const AVOption **o_out, char *buf, int buf_len);
=>
const char *av_opt_get_string(void *obj, const char *name, const AVOption **o_out, char *buf, int buf_len);

maybe buf_len should be replaced by buf_size, not very strong
preference though
...

const AVOption *av_next_option(void *obj, const AVOption *last);

looks consistent enouhg with other similar functions (av_codec_next(),
av_hwaccell_next(), av_filter_next(), ...), maybe av_option_next()
would be even better but this is not very important.

int av_opt_show2(void *obj, void *av_log_obj, int req_flags, int rej_flags);
could became:
void av_opt_show3(...), and doing nothing when obj is NULL

void av_opt_set_defaults(void *s);
void av_opt_set_defaults2(void *s, int mask, int flags);

maybe returning an int may help to check wrong defaults?

int av_set_options_string(void *ctx, const char *opts,
                          const char *key_val_sep, const char *pairs_sep);
=>
int av_opt_set_options(...)

...

Other nits: *obj may be consistently renamed *ctx.

Comments are welcome.

I have no hurry for this, but I'd like to start to work on it after
the 0.7/0.8 release for which we already have enough API changes (and
focus on bugfixing in the meaningwhile).
-- 
FFmpeg = Fundamentalist Freak Murdering Portable Ecumenical Guide