From 86ed6efd8acbf86c7f85d69e51c40d08f421a1ca Mon Sep 17 00:00:00 2001 From: wm4 Date: Tue, 18 Sep 2012 17:05:11 +0200 Subject: commands: handle property clamping in m_option Instead of clamping property values to the valid range in each property implementation, handle it in the property layer. The functionality to handle clamping for each type is in m_option.c. It's not really clear whether this is really needed. Normally, the raw values for M_PROPERTY_SET come only from m_option_type.parse (setting properties as string) or from m_option_parse.add (using the "switch" input command). However, since this was already done before, and since we _really_ want to be sure only to write valid values, add this code anyway. The newly added warnings/error messages should never actually be printed during normal operation and are for debugging (if they happen, we definitely want to see them). --- command.c | 16 ---------- m_option.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- m_option.h | 7 +++++ m_property.c | 20 ++++++++++-- m_property.h | 8 ----- 5 files changed, 123 insertions(+), 27 deletions(-) diff --git a/command.c b/command.c index b5b101ce79..c48962c28a 100644 --- a/command.c +++ b/command.c @@ -145,7 +145,6 @@ static int mp_property_playback_speed(m_option_t *prop, int action, switch (action) { case M_PROPERTY_SET: opts->playback_speed = *(float *) arg; - M_PROPERTY_CLAMP(prop, opts->playback_speed); // Adjust time until next frame flip for nosound mode mpctx->time_frame *= orig_speed / opts->playback_speed; reinit_audio_chain(mpctx); @@ -196,7 +195,6 @@ static int mp_property_stream_pos(m_option_t *prop, int action, void *arg, *(int64_t *) arg = stream_tell(stream); return M_PROPERTY_OK; case M_PROPERTY_SET: - M_PROPERTY_CLAMP(prop, *(int64_t *) arg); stream_seek(stream, *(int64_t *) arg); return M_PROPERTY_OK; } @@ -286,7 +284,6 @@ static int mp_property_percent_pos(m_option_t *prop, int action, switch (action) { case M_PROPERTY_SET: - M_PROPERTY_CLAMP(prop, *(int *)arg); pos = *(int *)arg; break; default: @@ -306,7 +303,6 @@ static int mp_property_time_pos(m_option_t *prop, int action, switch (action) { case M_PROPERTY_SET: - M_PROPERTY_CLAMP(prop, *(double *)arg); queue_seek(mpctx, MPSEEK_ABSOLUTE, *(double *)arg, 0); return M_PROPERTY_OK; } @@ -337,7 +333,6 @@ static int mp_property_chapter(m_option_t *prop, int action, void *arg, return M_PROPERTY_OK; } case M_PROPERTY_SET: - M_PROPERTY_CLAMP(prop, *(int *)arg); step_all = *(int *)arg - chapter; chapter += step_all; break; @@ -380,7 +375,6 @@ static int mp_property_edition(m_option_t *prop, int action, void *arg, case M_PROPERTY_PRINT: return m_property_int_ro(prop, action, arg, edition); case M_PROPERTY_SET: - M_PROPERTY_CLAMP(prop, *(int *)arg); edition = *(int *)arg; break; case M_PROPERTY_SWITCH: { @@ -464,7 +458,6 @@ static int mp_property_angle(m_option_t *prop, int action, void *arg, } case M_PROPERTY_SET: angle = *(int *)arg; - M_PROPERTY_CLAMP(prop, angle); break; case M_PROPERTY_SWITCH: // NOTE: should cycle @@ -566,7 +559,6 @@ static int mp_property_volume(m_option_t *prop, int action, void *arg, switch (action) { case M_PROPERTY_SET: - M_PROPERTY_CLAMP(prop, *(float *) arg); mixer_setvolume(&mpctx->mixer, *(float *) arg, *(float *) arg); return M_PROPERTY_OK; case M_PROPERTY_SWITCH: @@ -721,7 +713,6 @@ static int mp_property_balance(m_option_t *prop, int action, void *arg, return M_PROPERTY_OK; } case M_PROPERTY_SET: - M_PROPERTY_CLAMP(prop, *(float *)arg); mixer_setbalance(&mpctx->mixer, *(float *)arg); return M_PROPERTY_OK; } @@ -850,7 +841,6 @@ static int mp_property_fullscreen(m_option_t *prop, int action, void *arg, switch (action) { case M_PROPERTY_SET: - M_PROPERTY_CLAMP(prop, *(int *) arg); if (vo_fs == !!*(int *) arg) return M_PROPERTY_OK; if (mpctx->video_out->config_ok) @@ -874,7 +864,6 @@ static int mp_property_deinterlace(m_option_t *prop, int action, vf->control(vf, VFCTRL_GET_DEINTERLACE, arg); return M_PROPERTY_OK; case M_PROPERTY_SET: - M_PROPERTY_CLAMP(prop, *(int *) arg); vf->control(vf, VFCTRL_SET_DEINTERLACE, arg); return M_PROPERTY_OK; } @@ -997,7 +986,6 @@ static int mp_property_panscan(m_option_t *prop, int action, void *arg, switch (action) { case M_PROPERTY_SET: - M_PROPERTY_CLAMP(prop, *(float *) arg); vo_panscan = *(float *) arg; vo_control(mpctx->video_out, VOCTRL_SET_PANSCAN, NULL); return M_PROPERTY_OK; @@ -1018,7 +1006,6 @@ static int mp_property_vo_flag(m_option_t *prop, int action, void *arg, switch (action) { case M_PROPERTY_SET: - M_PROPERTY_CLAMP(prop, *(int *) arg); if (*vo_var == !!*(int *) arg) return M_PROPERTY_OK; if (mpctx->video_out->config_ok) @@ -1079,7 +1066,6 @@ static int mp_property_gamma(m_option_t *prop, int action, void *arg, switch (action) { case M_PROPERTY_SET: - M_PROPERTY_CLAMP(prop, *(int *) arg); *gamma = *(int *) arg; r = set_video_colors(mpctx->sh_video, prop->name, *gamma); if (r <= 0) @@ -1323,7 +1309,6 @@ static int mp_property_sub_scale(m_option_t *prop, int action, void *arg, switch (action) { case M_PROPERTY_SET: - M_PROPERTY_CLAMP(prop, *(float *) arg); if (opts->ass_enabled) opts->ass_font_scale = *(float *) arg; else @@ -1358,7 +1343,6 @@ static int mp_property_tv_color(m_option_t *prop, int action, void *arg, switch (action) { case M_PROPERTY_SET: - M_PROPERTY_CLAMP(prop, *(int *) arg); return tv_set_color_options(tvh, prop->offset, *(int *) arg); case M_PROPERTY_GET: return tv_get_color_options(tvh, prop->offset, arg); diff --git a/m_option.c b/m_option.c index 48539b3a4c..d48d4016b8 100644 --- a/m_option.c +++ b/m_option.c @@ -90,6 +90,14 @@ static void copy_opt(const m_option_t *opt, void *dst, const void *src) #define VAL(x) (*(int *)(x)) +static int clamp_flag(const m_option_t *opt, void *val) +{ + if (VAL(val) == opt->min || VAL(val) == opt->max) + return 0; + VAL(val) = opt->min; + return M_OPT_OUT_OF_RANGE; +} + static int parse_flag(const m_option_t *opt, struct bstr name, struct bstr param, void *dst) { @@ -141,12 +149,29 @@ const m_option_type_t m_option_type_flag = { .print = print_flag, .copy = copy_opt, .add = add_flag, + .clamp = clamp_flag, }; // Integer #undef VAL +static int clamp_longlong(const m_option_t *opt, void *val) +{ + long long v = *(long long *)val; + int r = 0; + if ((opt->flags & M_OPT_MAX) && (v > opt->max)) { + v = opt->max; + r = M_OPT_OUT_OF_RANGE; + } + if ((opt->flags & M_OPT_MIN) && (v < opt->min)) { + v = opt->min; + r = M_OPT_OUT_OF_RANGE; + } + *(long long *)val = v; + return r; +} + static int parse_longlong(const m_option_t *opt, struct bstr name, struct bstr param, void *dst) { @@ -184,6 +209,22 @@ static int parse_longlong(const m_option_t *opt, struct bstr name, return 1; } +static int clamp_int(const m_option_t *opt, void *val) +{ + long long tmp = *(int *)val; + int r = clamp_longlong(opt, &tmp); + *(int *)val = tmp; + return r; +} + +static int clamp_int64(const m_option_t *opt, void *val) +{ + long long tmp = *(int64_t *)val; + int r = clamp_longlong(opt, &tmp); + *(int64_t *)val = tmp; + return r; +} + static int parse_int(const m_option_t *opt, struct bstr name, struct bstr param, void *dst) { @@ -204,7 +245,6 @@ static int parse_int64(const m_option_t *opt, struct bstr name, return r; } - static char *print_int(const m_option_t *opt, const void *val) { if (opt->type->size == sizeof(int64_t)) @@ -247,6 +287,7 @@ const m_option_type_t m_option_type_int = { .print = print_int, .copy = copy_opt, .add = add_int, + .clamp = clamp_int, }; const m_option_type_t m_option_type_int64 = { @@ -256,6 +297,7 @@ const m_option_type_t m_option_type_int64 = { .print = print_int, .copy = copy_opt, .add = add_int64, + .clamp = clamp_int64, }; static int parse_intpair(const struct m_option *opt, struct bstr name, @@ -301,6 +343,21 @@ const struct m_option_type m_option_type_intpair = { .copy = copy_opt, }; +static int clamp_choice(const m_option_t *opt, void *val) +{ + int v = *(int *)val; + if ((opt->flags & M_OPT_MIN) && (opt->flags & M_OPT_MAX)) { + if (v >= opt->min && v <= opt->max) + return 0; + } + ; + for (struct m_opt_choice_alternatives *alt = opt->priv; alt->name; alt++) { + if (alt->value == v) + return 0; + } + return M_OPT_INVALID; +} + static int parse_choice(const struct m_option *opt, struct bstr name, struct bstr param, void *dst) { @@ -416,6 +473,7 @@ const struct m_option_type m_option_type_choice = { .print = print_choice, .copy = copy_opt, .add = add_choice, + .clamp = clamp_choice, }; // Float @@ -423,6 +481,22 @@ const struct m_option_type m_option_type_choice = { #undef VAL #define VAL(x) (*(double *)(x)) +static int clamp_double(const m_option_t *opt, void *val) +{ + double v = VAL(val); + int r = 0; + if ((opt->flags & M_OPT_MAX) && (v > opt->max)) { + v = opt->max; + r = M_OPT_OUT_OF_RANGE; + } + if ((opt->flags & M_OPT_MIN) && (v < opt->min)) { + v = opt->min; + r = M_OPT_OUT_OF_RANGE; + } + VAL(val) = v; + return r; +} + static int parse_double(const m_option_t *opt, struct bstr name, struct bstr param, void *dst) { @@ -514,11 +588,20 @@ const m_option_type_t m_option_type_double = { .print = print_double, .pretty_print = print_double_f2, .copy = copy_opt, + .clamp = clamp_double, }; #undef VAL #define VAL(x) (*(float *)(x)) +static int clamp_float(const m_option_t *opt, void *val) +{ + double tmp = VAL(val); + int r = clamp_double(opt, &tmp); + VAL(val) = tmp; + return r; +} + static int parse_float(const m_option_t *opt, struct bstr name, struct bstr param, void *dst) { @@ -555,6 +638,7 @@ const m_option_type_t m_option_type_float = { .pretty_print = print_float_f2, .copy = copy_opt, .add = add_float, + .clamp = clamp_float, }; ///////////// String @@ -562,6 +646,17 @@ const m_option_type_t m_option_type_float = { #undef VAL #define VAL(x) (*(char **)(x)) +static int clamp_str(const m_option_t *opt, void *val) +{ + char *v = VAL(val); + int len = v ? strlen(v) : 0; + if ((opt->flags & M_OPT_MIN) && (len < opt->min)) + return M_OPT_OUT_OF_RANGE; + if ((opt->flags & M_OPT_MAX) && (len > opt->max)) + return M_OPT_OUT_OF_RANGE; + return 0; +} + static int parse_str(const m_option_t *opt, struct bstr name, struct bstr param, void *dst) { @@ -620,6 +715,7 @@ const m_option_type_t m_option_type_string = { .print = print_str, .copy = copy_str, .free = free_str, + .clamp = clamp_str, }; //////////// String list @@ -1150,6 +1246,7 @@ const m_option_type_t m_option_type_time = { .pretty_print = pretty_print_time, .copy = copy_opt, .add = add_double, + .clamp = clamp_double, }; diff --git a/m_option.h b/m_option.h index 11e9e620fb..166e38e999 100644 --- a/m_option.h +++ b/m_option.h @@ -251,6 +251,13 @@ struct m_option_type { // add gives merely the direction. The wrap parameter determines whether // the value is clipped, or wraps around to the opposite max/min. void (*add)(const m_option_t *opt, void *val, double add, bool wrap); + + // Clamp the value in val to the option's valid value range. + // Return values: + // M_OPT_OUT_OF_RANGE: val was invalid, and modified (clamped) to be valid + // M_OPT_INVALID: val was invalid, and can't be made valid + // 0: val was already valid and is unchanged + int (*clamp)(const m_option_t *opt, void *val); }; // Option description diff --git a/m_property.c b/m_property.c index cb19ff08cb..af5d51dd21 100644 --- a/m_property.c +++ b/m_property.c @@ -129,6 +129,24 @@ int m_property_do(const m_option_t *prop_list, const char *name, r = do_action(prop_list, name, M_PROPERTY_SET, &val, ctx); m_option_free(opt, &val); return r; + case M_PROPERTY_SET: + if ((r = + do_action(prop_list, name, M_PROPERTY_GET_TYPE, &opt, ctx)) <= 0) + return r; + if (!opt->type->clamp) { + mp_msg(MSGT_CPLAYER, MSGL_WARN, "Property '%s' without clamp().\n", + name); + } else { + m_option_copy(opt, &val, arg); + r = opt->type->clamp(opt, arg); + m_option_free(opt, &val); + if (r != 0) { + mp_msg(MSGT_CPLAYER, MSGL_ERR, + "Property '%s': invalid value.\n", name); + return M_PROPERTY_ERROR; + } + } + return do_action(prop_list, name, M_PROPERTY_SET, arg, ctx); } return do_action(prop_list, name, action, arg, ctx); } @@ -270,7 +288,6 @@ int m_property_int_range(const m_option_t *prop, int action, { switch (action) { case M_PROPERTY_SET: - M_PROPERTY_CLAMP(prop, *(int *)arg); *var = *(int *)arg; return 1; } @@ -305,7 +322,6 @@ int m_property_float_range(const m_option_t *prop, int action, { switch (action) { case M_PROPERTY_SET: - M_PROPERTY_CLAMP(prop, *(float *)arg); *var = *(float *)arg; return 1; } diff --git a/m_property.h b/m_property.h index 462dad57fc..eba61a1619 100644 --- a/m_property.h +++ b/m_property.h @@ -159,14 +159,6 @@ char* mp_property_print(const char *name, void* ctx); /// \brief Helper functions for common property types. ///@{ -/// Clamp a value according to \ref m_option::min and \ref m_option::max. -#define M_PROPERTY_CLAMP(prop,val) do { \ - if(((prop)->flags & M_OPT_MIN) && (val) < (prop)->min) \ - (val) = (prop)->min; \ - else if(((prop)->flags & M_OPT_MAX) && (val) > (prop)->max) \ - (val) = (prop)->max; \ - } while(0) - /// Implement get. int m_property_int_ro(const m_option_t* prop,int action, void* arg,int var); -- cgit v1.2.3