<div dir="ltr">I do not think it breaks atomicity compared to what was already there. The existing comments already note that using two booleans instead of a tristate should be fixed, and checking and setting the toggles was already a two-step process. All I did was add sending messages to the OSD, which (to the best of my knowledge) shouldn't make things any worse than they already are.</div>
<div class="gmail_extra"><br><br><div class="gmail_quote">On Sat, Feb 16, 2013 at 6:13 AM, Rémi Denis-Courmont <span dir="ltr"><<a href="mailto:remi@remlab.net" target="_blank">remi@remlab.net</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">This patch breaks atomicity of toggle.<br>
<br>
Le vendredi 15 février 2013 20:35:27, John French a écrit :<br>
<div class="HOEnZb"><div class="h5">> Resolves ticket #7933<br>
> ---<br>
>  modules/control/hotkeys.c | 31 ++++++++++++++++++++++++++++---<br>
>  1 file changed, 28 insertions(+), 3 deletions(-)<br>
><br>
> diff --git a/modules/control/hotkeys.c b/modules/control/hotkeys.c<br>
> index 42e3ba0..4bb5047 100644<br>
> --- a/modules/control/hotkeys.c<br>
> +++ b/modules/control/hotkeys.c<br>
> @@ -163,22 +163,47 @@ static int PutAction( intf_thread_t *p_intf, int<br>
> i_action )<br>
><br>
>          /* Playlist actions (including audio) */<br>
>          case ACTIONID_LOOP:<br>
> +        {<br>
>              /* Toggle Normal -> Loop -> Repeat -> Normal ... */<br>
> +            char *mode;<br>
>              if( var_GetBool( p_playlist, "repeat" ) )<br>
> +            {<br>
>                  var_SetBool( p_playlist, "repeat", false );<br>
> -            else<br>
> -            if( var_GetBool( p_playlist, "loop" ) )<br>
> +                mode = "Off";<br>
> +            }<br>
> +            else if( var_GetBool( p_playlist, "loop" ) )<br>
>              { /* FIXME: this is not atomic, we should use a real tristate<br>
> */ var_SetBool( p_playlist, "loop", false );<br>
>                  var_SetBool( p_playlist, "repeat", true );<br>
> +                mode = "One";<br>
>              }<br>
>              else<br>
> +            {<br>
>                  var_SetBool( p_playlist, "loop", true );<br>
> +                mode = "All";<br>
> +            }<br>
> +            DisplayMessage( p_vout, SPU_DEFAULT_CHANNEL,<br>
> +                            _("Loop %s"), _(mode) );<br>
>              break;<br>
> +        }<br>
><br>
>          case ACTIONID_RANDOM:<br>
> -            var_ToggleBool( p_playlist, "random" );<br>
> +        {<br>
> +            char *state;<br>
> +            if( var_GetBool( p_playlist, "random" ) )<br>
> +            {<br>
> +                var_SetBool( p_playlist, "random", false );<br>
> +                state = "On";<br>
> +            }<br>
> +            else<br>
> +            {<br>
> +                var_SetBool( p_playlist, "random", true );<br>
> +                state = "Off";<br>
> +            }<br>
> +            DisplayMessage( p_vout, SPU_DEFAULT_CHANNEL,<br>
> +                            _("Random %s"), _(state) );<br>
>              break;<br>
> +        }<br>
><br>
>          case ACTIONID_NEXT:<br>
>              DisplayMessage( p_vout, _("Next") );<br>
<br>
--<br>
</div></div><span class="HOEnZb"><font color="#888888">Rémi Denis-Courmont<br>
<a href="http://www.remlab.net/" target="_blank">http://www.remlab.net/</a><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></span></blockquote></div><br></div>