Hi,<br><br>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.<br><br>And now I configured the IDE for the vlc project and I can explore the code much better :D<br>
<br>Regards,<br><br>Casian<br><br><br><div class="gmail_quote">On Tue, Mar 23, 2010 at 11:10 PM, Laurent Aimar <span dir="ltr"><<a href="mailto:fenrir@via.ecp.fr">fenrir@via.ecp.fr</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Hi,<br>
<br>
> +static void DisplayRate( input_thread_t *p_input, float f_rate )<br>
> +{<br>
> + char psz_msg[14 + 1];<br>
> + char psz_spd[7 + 1];<br>
> + strncpy( psz_msg, _("Speed: "), sizeof(psz_msg) );<br>
> + snprintf( psz_spd, sizeof(psz_spd), _("%.2fx"), f_rate );<br>
> + strncat( psz_msg, psz_spd, 7 );<br>
Why not using a simple snprintf for the whole ?<br>
It will be simpler and will avoid security risks (strncat/strncpy are<br>
usually really bad as they don't always add the terminal '\0').<br>
<br>
> diff --git a/src/input/input.c b/src/input/input.c<br>
> index f4081cc..75ab43e 100644<br>
> --- a/src/input/input.c<br>
> +++ b/src/input/input.c<br>
> @@ -1773,60 +1773,12 @@ static bool Control( input_thread_t *p_input,<br>
> case INPUT_CONTROL_SET_RATE_SLOWER:<br>
> case INPUT_CONTROL_SET_RATE_FASTER:<br>
As the support for slower/faster are moved to var.c,<br>
INPUT_CONTROL_SET_RATE_SLOWER/FASTER should be removed from here<br>
(and their declarations should also be removed).<br>
<br>
> + /* Moved the implementation of INPUT_CONTROL_SET_RATE_SLOWER/FASTER<br>
> + * in /src/input/var.c */<br>
This comment is not useful (git log/blame in case someone need to find it).<br>
<br>
> #include <vlc_common.h><br>
> +#include <math.h><br>
> #include <stdio.h><br>
> #include <stdlib.h><br>
><br>
> @@ -565,23 +566,66 @@ static int RateCallback( vlc_object_t *p_this, char const *psz_cmd,<br>
> {<br>
> input_thread_t *p_input = (input_thread_t*)p_this;<br>
> VLC_UNUSED(oldval); VLC_UNUSED(p_data);<br>
> +<br>
> + static const int ppi_factor[][2] = {<br>
> + {1,64}, {1,32}, {1,16}, {1,8}, {1,4}, {1,3}, {1,2}, {2,3},<br>
> + {1,1},<br>
> + {3,2}, {2,1}, {3,1}, {4,1}, {8,1}, {16,1}, {32,1}, {64,1},<br>
> + {0,0}<br>
> + };<br>
> +<br>
> + int i;<br>
> + int i_idx;<br>
> + float f_rate = var_GetFloat( p_input, "rate" );<br>
> + float f_error;<br>
> +<br>
> + /* Determine the factor closest to the current rate */<br>
> + f_error = 1E20;<br>
> + i_idx = -1;<br>
> + for( i = 0; ppi_factor[i][0] != 0; i++ )<br>
> + {<br>
> + const float f_test_r = (float)ppi_factor[i][0] / (float)ppi_factor[i][1];<br>
One cast is enough.<br>
> + const float f_test_e = fabs( fabs( f_rate ) - f_test_r );<br>
> + if( f_test_e < f_error )<br>
> + {<br>
> + i_idx = i;<br>
> + f_error = f_test_e;<br>
> + }<br>
> + }<br>
><br>
> - /* Problem with this way: the "rate" variable is updated after the<br>
> - * input thread did the change */<br>
> -<br>
> + if( i_idx < 0 || ppi_factor[i_idx][0] == 0 )<br>
> + return VLC_EBADVAR;<br>
You should keep the original assert, this situation cannot happen.<br>
<br>
> +<br>
> + vlc_value_t val;<br>
> + const float f_rate_min = (float)INPUT_RATE_DEFAULT / INPUT_RATE_MAX;<br>
> + const float f_rate_max = (float)INPUT_RATE_DEFAULT / INPUT_RATE_MIN;<br>
> +<br>
> if( !strcmp( psz_cmd, "rate-slower" ) )<br>
> {<br>
> - input_ControlPush( p_input, INPUT_CONTROL_SET_RATE_SLOWER, NULL );<br>
> + if( ppi_factor[i_idx+1][0] > 0 )<br>
i_idx > 0 is the right test.<br>
> + val.f_float = (float)ppi_factor[i_idx-1][0] / (float)ppi_factor[i_idx-1][1];<br>
One cast.<br>
<br>
> + else<br>
> + val.f_float = f_rate_min;<br>
> +<br>
> + var_SetFloat( p_input, "rate", val.f_float );<br>
You have forgot to apply the sign of the rate.<br>
> }<br>
> else if( !strcmp( psz_cmd, "rate-faster" ) )<br>
> {<br>
> - input_ControlPush( p_input, INPUT_CONTROL_SET_RATE_FASTER, NULL );<br>
> + if( ppi_factor[i_idx+1][0] > 0 )<br>
> + val.f_float = (float)ppi_factor[i_idx+1][0] / (float)ppi_factor[i_idx+1][1];<br>
One cast.<br>
> + else<br>
> + val.f_float = f_rate_max;<br>
> +<br>
> + var_SetFloat( p_input, "rate", val.f_float );<br>
You have forgot to apply the sign of the rate.<br>
> }<br>
> else<br>
> {<br>
> + /* Problem with this way: the "rate" variable is updated after the<br>
> + * input thread did the change */<br>
> newval.i_int = INPUT_RATE_DEFAULT / newval.f_float;<br>
> input_ControlPush( p_input, INPUT_CONTROL_SET_RATE, &newval );<br>
> }<br>
> +<br>
> return VLC_SUCCESS;<br>
> }<br>
<br>
Regards,<br>
<font color="#888888"><br>
--<br>
fenrir<br>
_______________________________________________<br>
vlc-devel mailing list<br>
To unsubscribe or modify your subscription options:<br>
<a href="http://mailman.videolan.org/listinfo/vlc-devel" target="_blank">http://mailman.videolan.org/listinfo/vlc-devel</a><br>
</font></blockquote></div><br>