[MPlayer-dev-eng] text_font_scale_factor

[Alban Bedel]

On Sun, 8 Jul 2007 23:23:53 +0300
"Anatoli Marinov" <anatoli.marinov at gmail.com> wrote:

> +/// Subtitle scale (RW)
> +static int mp_property_sub_scale(m_option_t * prop, int action, void
> *arg,
> +			      MPContext * mpctx)
> +{
> +    if (!arg)
> +        return M_PROPERTY_ERROR;

This is not ok some actions don't need arguments.

> +    switch (action) {
> +        case M_PROPERTY_SET:
> +            text_font_scale_factor = *(float *) arg;
> +            M_PROPERTY_CLAMP(prop, text_font_scale_factor);
> +            force_load_font = 1;
> +            return M_PROPERTY_OK;
> +        case M_PROPERTY_STEP_UP:
> +        case M_PROPERTY_STEP_DOWN:
> +            text_font_scale_factor += *(float *) arg;

This is not right, it should handle the case w/o argument and reverse
the direction for STEP_DOWN. So something like this is needed:
               text_font_scale_factor += (arg ? *(float *) arg : 0.1)*
                          (action == M_PROPERTY_STEP_UP ? 1.0 : -1.0);

> +            M_PROPERTY_CLAMP(prop, text_font_scale_factor);
> +            force_load_font = 1;
> +            return M_PROPERTY_OK;
> +        default:
> +            return m_property_float_ro(prop, action, arg,
> text_font_scale_factor);
> +    }
> +
> +    return M_PROPERTY_NOT_IMPLEMENTED;

This is useless, it can't be reached. Remove it or replace it with the
default case code (i tend to prefer the latter but this is just taste).

> +}

Otherwise it look ok. It would be nice if you also added the property
to the example menu config (etc/menu.conf).

	Albeu