options: Make validation and help possible for all option types

Today, validation is only possible for string type options. But there's
no particular reason why it needs to be restricted in this way, and
there are potential uses, to allow other options to be validated
without forcing the option to have to reimplement parsing from
scratch.

The first part, simply making the validation function an explicit
field instead of overloading priv is simple enough. But if we only do
that, then the validation function still needs to deal with the raw
pre-parsed string. Instead, we want to allow the value to be parsed
before it is validated. That in turn leads to us having validator
functions that should be type aware. Unfortunately, that means we need
to keep the explicit macro like OPT_STRING_VALIDATE() as a way to
enforce the correct typing of the function. Otherwise, we'd have to
have the validator take a void * and hope the implementation can cast
it correctly.

For help, we don't have this problem, as help doesn't look at the
value.

Then, we turn validators that are really help generators into explicit
help functions and where a validator is help + validation, we split
them into two parts.

I have, however, left functions that need to query information for both
help and validation as single functions to avoid code duplication.

In this change, I have not added an other OPT_FOO_VALIDATE() macros as
they are not needed, but I will add some in a separate change to
illustrate the pattern.
This commit is contained in:
Philip Langdale 2021-02-20 16:41:44 -08:00 committed by Jan Ekström
parent 6265724f33
commit 3f006eced4
14 changed files with 163 additions and 110 deletions

View File

@ -110,16 +110,19 @@ struct dec_wrapper_opts {
int64_t audio_reverse_size;
};
static int decoder_list_opt(struct mp_log *log, const m_option_t *opt,
struct bstr name, struct bstr param);
static int decoder_list_help(struct mp_log *log, const m_option_t *opt,
struct bstr name);
const struct m_sub_options dec_wrapper_conf = {
.opts = (const struct m_option[]){
{"correct-pts", OPT_FLAG(correct_pts)},
{"fps", OPT_DOUBLE(force_fps), M_RANGE(0, DBL_MAX)},
{"ad", OPT_STRING_VALIDATE(audio_decoders, decoder_list_opt)},
{"vd", OPT_STRING_VALIDATE(video_decoders, decoder_list_opt)},
{"audio-spdif", OPT_STRING_VALIDATE(audio_spdif, decoder_list_opt)},
{"ad", OPT_STRING(audio_decoders),
.help = decoder_list_help},
{"vd", OPT_STRING(video_decoders),
.help = decoder_list_help},
{"audio-spdif", OPT_STRING(audio_spdif),
.help = decoder_list_help},
{"video-rotate", OPT_CHOICE(video_rotate, {"no", -1}),
.flags = UPDATE_IMGPAR, M_RANGE(0, 359)},
{"video-aspect-override", OPT_ASPECT(movie_aspect),
@ -232,11 +235,9 @@ struct priv {
int dropped_frames; // total frames _probably_ dropped
};
static int decoder_list_opt(struct mp_log *log, const m_option_t *opt,
struct bstr name, struct bstr param)
static int decoder_list_help(struct mp_log *log, const m_option_t *opt,
struct bstr name)
{
if (!bstr_equals0(param, "help"))
return 1;
if (strcmp(opt->name, "ad") == 0) {
struct mp_decoder_list *list = audio_decoder_list();
mp_print_decoders(log, MSGL_INFO, "Audio decoders:", list);

View File

@ -61,6 +61,31 @@ const char m_option_path_separator = OPTION_PATH_SEPARATOR;
#define OPT_INT_MAX(opt, T, Tm) ((opt)->min < (opt)->max \
? ((opt)->max >= (double)(Tm) ? (Tm) : (T)((opt)->max)) : (Tm))
int m_option_parse(struct mp_log *log, const m_option_t *opt,
struct bstr name, struct bstr param, void *dst)
{
int r = M_OPT_INVALID;
if (bstr_equals0(param, "help") && opt->help) {
r = opt->help(log, opt, name);
if (r < 0)
return r;
}
r = opt->type->parse(log, opt, name, param, dst);
if (r < 0)
return r;
if (opt->validate) {
r = opt->validate(log, opt, name, dst);
if (r < 0) {
if (opt->type->free)
opt->type->free(dst);
return r;
}
}
return 1;
}
char *m_option_strerror(int code)
{
switch (code) {
@ -1192,13 +1217,6 @@ const m_option_type_t m_option_type_aspect = {
static int parse_str(struct mp_log *log, const m_option_t *opt,
struct bstr name, struct bstr param, void *dst)
{
m_opt_string_validate_fn validate = opt->priv;
if (validate) {
int r = validate(log, opt, name, param);
if (r < 0)
return r;
}
if (dst) {
talloc_free(VAL(dst));
VAL(dst) = bstrdup0(NULL, param);

View File

@ -189,9 +189,12 @@ struct m_opt_choice_alternatives {
const char *m_opt_choice_str(const struct m_opt_choice_alternatives *choices,
int value);
// For OPT_STRING_VALIDATE(). Behaves like m_option_type.parse().
// Validator function signatures. Required to properly type the param value.
typedef int (*m_opt_generic_validate_fn)(struct mp_log *log, const m_option_t *opt,
struct bstr name, void *value);
typedef int (*m_opt_string_validate_fn)(struct mp_log *log, const m_option_t *opt,
struct bstr name, struct bstr param);
struct bstr name, const char **value);
// m_option.priv points to this if OPT_SUBSTRUCT is used
struct m_sub_options {
@ -270,6 +273,9 @@ struct m_option_type {
// Parse the data from a string.
/** It is the only required function, all others can be NULL.
* Generally should not be called directly outside of the options module,
* but instead through \ref m_option_parse which calls additional option
* specific callbacks during the process.
*
* \param log for outputting parser error or help messages
* \param opt The option that is parsed.
@ -384,6 +390,12 @@ struct m_option {
// Print a warning when this option is used (for options with no direct
// replacement.)
const char *deprecation_message;
// Optional function that validates a param value for this option.
m_opt_generic_validate_fn validate;
// Optional function that displays help. Will replace type-specific help.
int (*help)(struct mp_log *log, const m_option_t *opt, struct bstr name);
};
char *format_file_size(int64_t size);
@ -490,12 +502,13 @@ char *format_file_size(int64_t size);
char *m_option_strerror(int code);
// Helper to parse options, see \ref m_option_type::parse.
static inline int m_option_parse(struct mp_log *log, const m_option_t *opt,
struct bstr name, struct bstr param, void *dst)
{
return opt->type->parse(log, opt, name, param, dst);
}
// Base function to parse options. Includes calling help and validation
// callbacks. Only when this functionality is for some reason required to not
// happen should the parse function pointer be utilized by itself.
//
// See \ref m_option_type::parse.
int m_option_parse(struct mp_log *log, const m_option_t *opt,
struct bstr name, struct bstr param, void *dst);
// Helper to print options, see \ref m_option_type::print.
static inline char *m_option_print(const m_option_t *opt, const void *val_ptr)
@ -663,7 +676,8 @@ extern const char m_option_path_separator;
#define OPT_STRING_VALIDATE(field, validate_fn) \
OPT_TYPED_FIELD(m_option_type_string, char*, field), \
.priv = MP_EXPECT_TYPE(m_opt_string_validate_fn, validate_fn)
.validate = (m_opt_generic_validate_fn) \
MP_EXPECT_TYPE(m_opt_string_validate_fn, validate_fn)
#define M_CHOICES(...) \
.priv = (void *)&(const struct m_opt_choice_alternatives[]){ __VA_ARGS__, {0}}

View File

@ -63,8 +63,8 @@ static void uninit_avctx(struct mp_filter *vd);
static int get_buffer2_direct(AVCodecContext *avctx, AVFrame *pic, int flags);
static enum AVPixelFormat get_format_hwdec(struct AVCodecContext *avctx,
const enum AVPixelFormat *pix_fmt);
static int hwdec_validate_opt(struct mp_log *log, const m_option_t *opt,
struct bstr name, struct bstr param);
static int hwdec_opt_help(struct mp_log *log, const m_option_t *opt,
struct bstr name);
#define HWDEC_DELAY_QUEUE_COUNT 2
@ -117,7 +117,8 @@ const struct m_sub_options vd_lavc_conf = {
{"no", INT_MAX}, {"yes", 1}), M_RANGE(1, INT_MAX)},
{"vd-lavc-o", OPT_KEYVALUELIST(avopts)},
{"vd-lavc-dr", OPT_FLAG(dr)},
{"hwdec", OPT_STRING_VALIDATE(hwdec_api, hwdec_validate_opt),
{"hwdec", OPT_STRING(hwdec_api),
.help = hwdec_opt_help,
.flags = M_OPT_OPTIONAL_PARAM | UPDATE_HWDEC},
{"hwdec-codecs", OPT_STRING(hwdec_codecs)},
{"hwdec-image-format", OPT_IMAGEFORMAT(hwdec_image_format)},
@ -533,33 +534,30 @@ static void select_and_set_hwdec(struct mp_filter *vd)
}
}
static int hwdec_validate_opt(struct mp_log *log, const m_option_t *opt,
struct bstr name, struct bstr param)
static int hwdec_opt_help(struct mp_log *log, const m_option_t *opt,
struct bstr name)
{
if (bstr_equals0(param, "help")) {
struct hwdec_info *hwdecs = NULL;
int num_hwdecs = 0;
add_all_hwdec_methods(&hwdecs, &num_hwdecs);
struct hwdec_info *hwdecs = NULL;
int num_hwdecs = 0;
add_all_hwdec_methods(&hwdecs, &num_hwdecs);
mp_info(log, "Valid values (with alternative full names):\n");
mp_info(log, "Valid values (with alternative full names):\n");
for (int n = 0; n < num_hwdecs; n++) {
struct hwdec_info *hwdec = &hwdecs[n];
for (int n = 0; n < num_hwdecs; n++) {
struct hwdec_info *hwdec = &hwdecs[n];
mp_info(log, " %s (%s)\n", hwdec->method_name, hwdec->name);
}
talloc_free(hwdecs);
mp_info(log, " auto (yes '')\n");
mp_info(log, " no\n");
mp_info(log, " auto-safe\n");
mp_info(log, " auto-copy\n");
mp_info(log, " auto-copy-safe\n");
return M_OPT_EXIT;
mp_info(log, " %s (%s)\n", hwdec->method_name, hwdec->name);
}
return 0;
talloc_free(hwdecs);
mp_info(log, " auto (yes '')\n");
mp_info(log, " no\n");
mp_info(log, " auto-safe\n");
mp_info(log, " auto-copy\n");
mp_info(log, " auto-copy-safe\n");
return M_OPT_EXIT;
}
static void force_fallback(struct mp_filter *vd)

View File

@ -28,7 +28,7 @@
static int d3d11_validate_adapter(struct mp_log *log,
const struct m_option *opt,
struct bstr name, struct bstr param);
struct bstr name, const char **value);
struct d3d11_opts {
int feature_level;
@ -61,7 +61,7 @@ const struct m_sub_options d3d11_conf = {
{"d3d11-flip", OPT_FLAG(flip)},
{"d3d11-sync-interval", OPT_INT(sync_interval), M_RANGE(0, 4)},
{"d3d11-adapter", OPT_STRING_VALIDATE(adapter_name,
d3d11_validate_adapter)},
d3d11_validate_adapter)},
{"d3d11-output-format", OPT_CHOICE(output_format,
{"auto", DXGI_FORMAT_UNKNOWN},
{"rgba8", DXGI_FORMAT_R8G8B8A8_UNORM},
@ -111,8 +111,9 @@ struct priv {
static int d3d11_validate_adapter(struct mp_log *log,
const struct m_option *opt,
struct bstr name, struct bstr param)
struct bstr name, const char **value)
{
struct bstr param = bstr0(*value);
bool help = bstr_equals0(param, "help");
bool adapter_matched = false;
struct bstr listing = { 0 };

View File

@ -54,13 +54,15 @@
static int vt_switcher_pipe[2];
static int drm_validate_connector_opt(
struct mp_log *log, const struct m_option *opt, struct bstr name,
struct bstr param);
static int drm_connector_opt_help(
struct mp_log *log, const struct m_option *opt, struct bstr name);
static int drm_mode_opt_help(
struct mp_log *log, const struct m_option *opt, struct bstr name);
static int drm_validate_mode_opt(
struct mp_log *log, const struct m_option *opt, struct bstr name,
struct bstr param);
const char **value);
static void kms_show_available_modes(
struct mp_log *log, const drmModeConnector *connector);
@ -71,10 +73,10 @@ static double mode_get_Hz(const drmModeModeInfo *mode);
#define OPT_BASE_STRUCT struct drm_opts
const struct m_sub_options drm_conf = {
.opts = (const struct m_option[]) {
{"drm-connector", OPT_STRING_VALIDATE(drm_connector_spec,
drm_validate_connector_opt)},
{"drm-mode", OPT_STRING_VALIDATE(drm_mode_spec,
drm_validate_mode_opt)},
{"drm-connector", OPT_STRING(drm_connector_spec),
.help = drm_connector_opt_help},
{"drm-mode", OPT_STRING_VALIDATE(drm_mode_spec, drm_validate_mode_opt),
.help = drm_mode_opt_help},
{"drm-atomic", OPT_CHOICE(drm_atomic, {"no", 0}, {"auto", 1})},
{"drm-draw-plane", OPT_CHOICE(drm_draw_plane,
{"primary", DRM_OPTS_PRIMARY_PLANE},
@ -747,31 +749,28 @@ double kms_get_display_fps(const struct kms *kms)
return mode_get_Hz(&kms->mode.mode);
}
static int drm_validate_connector_opt(struct mp_log *log, const struct m_option *opt,
struct bstr name, struct bstr param)
static int drm_connector_opt_help(struct mp_log *log, const struct m_option *opt,
struct bstr name)
{
if (bstr_equals0(param, "help")) {
kms_show_available_cards_and_connectors(log);
return M_OPT_EXIT;
}
return 1;
kms_show_available_cards_and_connectors(log);
return M_OPT_EXIT;
}
static int drm_mode_opt_help(struct mp_log *log, const struct m_option *opt,
struct bstr name)
{
kms_show_available_cards_connectors_and_modes(log);
return M_OPT_EXIT;
}
static int drm_validate_mode_opt(struct mp_log *log, const struct m_option *opt,
struct bstr name, struct bstr param)
struct bstr name, const char **value)
{
if (bstr_equals0(param, "help")) {
kms_show_available_cards_connectors_and_modes(log);
return M_OPT_EXIT;
}
char *spec = bstrto0(NULL, param);
if (!parse_mode_spec(spec, NULL)) {
const char *param = *value;
if (!parse_mode_spec(param, NULL)) {
mp_fatal(log, "Invalid value for option drm-mode. Must be a positive number, a string of the format WxH[@R] or 'help'\n");
talloc_free(spec);
return M_OPT_INVALID;
}
talloc_free(spec);
return 1;
}

View File

@ -110,16 +110,20 @@ static const struct ra_ctx_fns *contexts[] = {
#endif
};
int ra_ctx_validate_api(struct mp_log *log, const struct m_option *opt,
struct bstr name, struct bstr param)
int ra_ctx_api_help(struct mp_log *log, const struct m_option *opt,
struct bstr name)
{
if (bstr_equals0(param, "help")) {
mp_info(log, "GPU APIs (contexts):\n");
mp_info(log, " auto (autodetect)\n");
for (int n = 0; n < MP_ARRAY_SIZE(contexts); n++)
mp_info(log, " %s (%s)\n", contexts[n]->type, contexts[n]->name);
return M_OPT_EXIT;
}
mp_info(log, "GPU APIs (contexts):\n");
mp_info(log, " auto (autodetect)\n");
for (int n = 0; n < MP_ARRAY_SIZE(contexts); n++)
mp_info(log, " %s (%s)\n", contexts[n]->type, contexts[n]->name);
return M_OPT_EXIT;
}
int ra_ctx_validate_api(struct mp_log *log, const struct m_option *opt,
struct bstr name, const char **value)
{
struct bstr param = bstr0(*value);
if (bstr_equals0(param, "auto"))
return 1;
for (int i = 0; i < MP_ARRAY_SIZE(contexts); i++) {
@ -129,16 +133,20 @@ int ra_ctx_validate_api(struct mp_log *log, const struct m_option *opt,
return M_OPT_INVALID;
}
int ra_ctx_validate_context(struct mp_log *log, const struct m_option *opt,
struct bstr name, struct bstr param)
int ra_ctx_context_help(struct mp_log *log, const struct m_option *opt,
struct bstr name)
{
if (bstr_equals0(param, "help")) {
mp_info(log, "GPU contexts (APIs):\n");
mp_info(log, " auto (autodetect)\n");
for (int n = 0; n < MP_ARRAY_SIZE(contexts); n++)
mp_info(log, " %s (%s)\n", contexts[n]->name, contexts[n]->type);
return M_OPT_EXIT;
}
mp_info(log, "GPU contexts (APIs):\n");
mp_info(log, " auto (autodetect)\n");
for (int n = 0; n < MP_ARRAY_SIZE(contexts); n++)
mp_info(log, " %s (%s)\n", contexts[n]->name, contexts[n]->type);
return M_OPT_EXIT;
}
int ra_ctx_validate_context(struct mp_log *log, const struct m_option *opt,
struct bstr name, const char **value)
{
struct bstr param = bstr0(*value);
if (bstr_equals0(param, "auto"))
return 1;
for (int i = 0; i < MP_ARRAY_SIZE(contexts); i++) {

View File

@ -100,7 +100,11 @@ struct ra_ctx *ra_ctx_create(struct vo *vo, const char *context_type,
void ra_ctx_destroy(struct ra_ctx **ctx);
struct m_option;
int ra_ctx_api_help(struct mp_log *log, const struct m_option *opt,
struct bstr name);
int ra_ctx_validate_api(struct mp_log *log, const struct m_option *opt,
struct bstr name, struct bstr param);
struct bstr name, const char **value);
int ra_ctx_context_help(struct mp_log *log, const struct m_option *opt,
struct bstr name);
int ra_ctx_validate_context(struct mp_log *log, const struct m_option *opt,
struct bstr name, struct bstr param);
struct bstr name, const char **value);

View File

@ -106,8 +106,9 @@ struct ra_hwdec *ra_hwdec_load_driver(struct ra *ra, struct mp_log *log,
}
int ra_hwdec_validate_opt(struct mp_log *log, const m_option_t *opt,
struct bstr name, struct bstr param)
struct bstr name, const char **value)
{
struct bstr param = bstr0(*value);
bool help = bstr_equals0(param, "help");
if (help)
mp_info(log, "Available hwdecs:\n");

View File

@ -109,7 +109,7 @@ struct ra_hwdec *ra_hwdec_load_driver(struct ra *ra, struct mp_log *log,
bool is_auto);
int ra_hwdec_validate_opt(struct mp_log *log, const m_option_t *opt,
struct bstr name, struct bstr param);
struct bstr name, const char **value);
void ra_hwdec_uninit(struct ra_hwdec *hwdec);

View File

@ -67,8 +67,9 @@ static bool parse_3dlut_size(const char *arg, int *p1, int *p2, int *p3)
}
static int validate_3dlut_size_opt(struct mp_log *log, const m_option_t *opt,
struct bstr name, struct bstr param)
struct bstr name, const char **value)
{
struct bstr param = bstr0(*value);
int p1, p2, p3;
char s[20];
snprintf(s, sizeof(s), "%.*s", BSTR_P(param));

View File

@ -337,13 +337,13 @@ static const struct gl_video_opts gl_video_opts_def = {
};
static int validate_scaler_opt(struct mp_log *log, const m_option_t *opt,
struct bstr name, struct bstr param);
struct bstr name, const char **value);
static int validate_window_opt(struct mp_log *log, const m_option_t *opt,
struct bstr name, struct bstr param);
struct bstr name, const char **value);
static int validate_error_diffusion_opt(struct mp_log *log, const m_option_t *opt,
struct bstr name, struct bstr param);
struct bstr name, const char **value);
#define OPT_BASE_STRUCT struct gl_video_opts
@ -4089,8 +4089,9 @@ void gl_video_configure_queue(struct gl_video *p, struct vo *vo)
}
static int validate_scaler_opt(struct mp_log *log, const m_option_t *opt,
struct bstr name, struct bstr param)
struct bstr name, const char **value)
{
struct bstr param = bstr0(*value);
char s[20] = {0};
int r = 1;
bool tscale = bstr_equals0(name, "tscale");
@ -4121,8 +4122,9 @@ static int validate_scaler_opt(struct mp_log *log, const m_option_t *opt,
}
static int validate_window_opt(struct mp_log *log, const m_option_t *opt,
struct bstr name, struct bstr param)
struct bstr name, const char **value)
{
struct bstr param = bstr0(*value);
char s[20] = {0};
int r = 1;
if (bstr_equals0(param, "help")) {
@ -4146,8 +4148,9 @@ static int validate_window_opt(struct mp_log *log, const m_option_t *opt,
}
static int validate_error_diffusion_opt(struct mp_log *log, const m_option_t *opt,
struct bstr name, struct bstr param)
struct bstr name, const char **value)
{
struct bstr param = bstr0(*value);
char s[20] = {0};
int r = 1;
if (bstr_equals0(param, "help")) {

View File

@ -321,8 +321,12 @@ err_out:
#define OPT_BASE_STRUCT struct gpu_priv
static const m_option_t options[] = {
{"gpu-context", OPT_STRING_VALIDATE(context_name, ra_ctx_validate_context)},
{"gpu-api", OPT_STRING_VALIDATE(context_type, ra_ctx_validate_api)},
{"gpu-context",
OPT_STRING_VALIDATE(context_name, ra_ctx_validate_context),
.help = ra_ctx_context_help},
{"gpu-api",
OPT_STRING_VALIDATE(context_type, ra_ctx_validate_api),
.help = ra_ctx_api_help},
{"gpu-debug", OPT_FLAG(opts.debug)},
{"gpu-sw", OPT_FLAG(opts.allow_sw)},
{0}

View File

@ -31,8 +31,9 @@ struct vulkan_opts {
};
static int vk_validate_dev(struct mp_log *log, const struct m_option *opt,
struct bstr name, struct bstr param)
struct bstr name, const char **value)
{
struct bstr param = bstr0(*value);
int ret = M_OPT_INVALID;
VkResult res;