[vlc-devel] [PATCH] input: set OSD message when changing rate via hotkeys to display the current rate (v3)

[Casian Andrei]

Hi,

I have done the modifications that you pointed out. Thank you for pointing
them out. I was actually thinking what to do on a couple of them. Next time
I won't make the same mistakes again.

And now I configured the IDE for the vlc project and I can explore the code
much better :D

Regards,

Casian


On Tue, Mar 23, 2010 at 11:10 PM, Laurent Aimar <fenrir at via.ecp.fr> wrote:

> Hi,
>
> > +static void DisplayRate( input_thread_t *p_input, float f_rate )
> > +{
> > +    char psz_msg[14 + 1];
> > +    char psz_spd[7 + 1];
> > +    strncpy( psz_msg, _("Speed: "), sizeof(psz_msg) );
> > +    snprintf( psz_spd, sizeof(psz_spd), _("%.2fx"), f_rate );
> > +    strncat( psz_msg, psz_spd, 7 );
>  Why not using a simple snprintf for the whole ?
> It will be simpler and will avoid security risks (strncat/strncpy are
> usually really bad as they don't always add the terminal '\0').
>
> > diff --git a/src/input/input.c b/src/input/input.c
> > index f4081cc..75ab43e 100644
> > --- a/src/input/input.c
> > +++ b/src/input/input.c
> > @@ -1773,60 +1773,12 @@ static bool Control( input_thread_t *p_input,
> >          case INPUT_CONTROL_SET_RATE_SLOWER:
> >          case INPUT_CONTROL_SET_RATE_FASTER:
>  As the support for slower/faster are moved to var.c,
> INPUT_CONTROL_SET_RATE_SLOWER/FASTER should be removed from here
> (and their declarations should also be removed).
>
> > +            /* Moved the implementation of
> INPUT_CONTROL_SET_RATE_SLOWER/FASTER
> > +             * in /src/input/var.c */
>  This comment is not useful (git log/blame in case someone need to find
> it).
>
> >  #include <vlc_common.h>
> > +#include <math.h>
> >  #include <stdio.h>
> >  #include <stdlib.h>
> >
> > @@ -565,23 +566,66 @@ static int RateCallback( vlc_object_t *p_this, char
> const *psz_cmd,
> >  {
> >      input_thread_t *p_input = (input_thread_t*)p_this;
> >      VLC_UNUSED(oldval); VLC_UNUSED(p_data);
> > +
> > +    static const int ppi_factor[][2] = {
> > +        {1,64}, {1,32}, {1,16}, {1,8}, {1,4}, {1,3}, {1,2}, {2,3},
> > +        {1,1},
> > +        {3,2}, {2,1}, {3,1}, {4,1}, {8,1}, {16,1}, {32,1}, {64,1},
> > +        {0,0}
> > +    };
> > +
> > +    int i;
> > +    int i_idx;
> > +    float f_rate = var_GetFloat( p_input, "rate" );
> > +    float f_error;
> > +
> > +    /* Determine the factor closest to the current rate */
> > +    f_error = 1E20;
> > +    i_idx = -1;
> > +    for( i = 0; ppi_factor[i][0] != 0; i++ )
> > +    {
> > +        const float f_test_r = (float)ppi_factor[i][0] /
> (float)ppi_factor[i][1];
>  One cast is enough.
> > +        const float f_test_e = fabs( fabs( f_rate ) - f_test_r );
> > +        if( f_test_e < f_error )
> > +        {
> > +            i_idx = i;
> > +            f_error = f_test_e;
> > +        }
> > +    }
> >
> > -    /* Problem with this way: the "rate" variable is updated after the
> > -     * input thread did the change */
> > -
> > +    if( i_idx < 0 || ppi_factor[i_idx][0] == 0 )
> > +        return VLC_EBADVAR;
>  You should keep the original assert, this situation cannot happen.
>
> > +
> > +    vlc_value_t val;
> > +    const float f_rate_min = (float)INPUT_RATE_DEFAULT / INPUT_RATE_MAX;
> > +    const float f_rate_max = (float)INPUT_RATE_DEFAULT / INPUT_RATE_MIN;
> > +
> >      if( !strcmp( psz_cmd, "rate-slower" ) )
> >      {
> > -        input_ControlPush( p_input, INPUT_CONTROL_SET_RATE_SLOWER, NULL
> );
> > +        if( ppi_factor[i_idx+1][0] > 0 )
> i_idx > 0 is the right test.
> > +            val.f_float = (float)ppi_factor[i_idx-1][0] /
> (float)ppi_factor[i_idx-1][1];
>  One cast.
>
> > +        else
> > +            val.f_float = f_rate_min;
> > +
> > +        var_SetFloat( p_input, "rate", val.f_float );
>  You have forgot to apply the sign of the rate.
> >      }
> >      else if( !strcmp( psz_cmd, "rate-faster" ) )
> >      {
> > -        input_ControlPush( p_input, INPUT_CONTROL_SET_RATE_FASTER, NULL
> );
> > +        if( ppi_factor[i_idx+1][0] > 0 )
> > +            val.f_float = (float)ppi_factor[i_idx+1][0] /
> (float)ppi_factor[i_idx+1][1];
>  One cast.
> > +        else
> > +            val.f_float = f_rate_max;
> > +
> > +        var_SetFloat( p_input, "rate", val.f_float );
>  You have forgot to apply the sign of the rate.
> >      }
> >      else
> >      {
> > +        /* Problem with this way: the "rate" variable is updated after
> the
> > +         * input thread did the change */
> >          newval.i_int = INPUT_RATE_DEFAULT / newval.f_float;
> >          input_ControlPush( p_input, INPUT_CONTROL_SET_RATE, &newval );
> >      }
> > +
> >      return VLC_SUCCESS;
> >  }
>
> Regards,
>
> --
> fenrir
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> http://mailman.videolan.org/listinfo/vlc-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20100324/a4a3e4aa/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-input-set-OSD-message-when-changing-rate-via-hotkeys.patch
Type: text/x-patch
Size: 10943 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20100324/a4a3e4aa/attachment.bin>