[vlc-devel] [PATCH v2 08/11] oldrc: use vlc_actions_do

Rémi Denis-Courmont remi at remlab.net
Thu Aug 10 10:37:15 CEST 2017


Le 10 août 2017 11:19:08 GMT+03:00, Jean-Baptiste Kempf <jb at videolan.org> a écrit :
>Hello,
>
>On Thu, 10 Aug 2017, at 10:15, Rémi Denis-Courmont wrote:
>> Le 10 août 2017 10:05:39 GMT+03:00, "Hugo Beauzée-Luyssen"
>> <hugo at beauzee.fr> a écrit :>> On Wed, Aug 9, 2017, at 09:48 PM, Rémi
>Denis-Courmont wrote:
>>> 
>>>>  Le keskiviikkona 9. elokuuta 2017, 18.16.32 EEST Hugo Beauzée-
>>>>  Luyssen a
>>>>>>>  écrit :
>>>> 
>>>>>  From: Thomas Guillem <thomas at gllm.fr>
>>>>> 
>>>>>  
>>>>> 
>>>>>  ---
>>>>> 
>>>>>   modules/control/oldrc.c | 23 +++++++++++------------
>>>>> 
>>>>>   1 file changed, 11 insertions(+), 12 deletions(-)
>>>>> 
>>>>>  
>>>>> 
>>>>>  diff --git a/modules/control/oldrc.c b/modules/control/oldrc.c
>>>>>>>>>  index 66402293a5..f71f0278af 100644
>>>>> 
>>>>>  --- a/modules/control/oldrc.c
>>>>> 
>>>>>  +++ b/modules/control/oldrc.c
>>>>> 
>>>>>  @@ -714,8 +714,7 @@ static void *Run( void *data )
>>>>> 
>>>>>           }
>>>>> 
>>>>>           else if( !strcmp( psz_cmd, "key" ) || !strcmp( psz_cmd,
>>>>>           "hotkey" )
>>>>>>>>>  ) {
>>>>> 
>>>>>  -            var_SetInteger( p_intf->obj.libvlc, "key-action",
>>>>>>>>>  -                            vlc_actions_get_id( psz_arg ) );
>>>>>>>>>  +            vlc_actions_do( p_intf, vlc_actions_get_id(
>psz_arg
>>>>>               ), true, 0
>>>>>>>>>  ); }
>>>>> 
>>>>>           else switch( psz_cmd[0] )
>>>>> 
>>>>>           {
>>>>> 
>>>>>  @@ -959,7 +958,7 @@ static int Input( vlc_object_t *p_this, char
>>>>>  const
>>>>>>>>>  *psz_cmd, /* Parse commands that only require an input */
>>>>> 
>>>>>       if( !strcmp( psz_cmd, "pause" ) )
>>>>> 
>>>>>       {
>>>>> 
>>>>>  -        playlist_TogglePause( p_intf->p_sys->p_playlist );
>>>>>>>>>  +        vlc_actions_do( p_intf, ACTIONID_PLAY_PAUSE, true, 0
>);
>>>>>>>>  
>>>> 
>>>>  Why oh why? What is the point in doing this?
>>>> 
>>>>  
>>>> 
>>>>>           i_error = VLC_SUCCESS;
>>>>> 
>>>>>       }
>>>>> 
>>>>>       else if( !strcmp( psz_cmd, "seek" ) )
>>>>> 
>>>>>  @@ -1001,18 +1000,18 @@ static int Input( vlc_object_t *p_this,
>>>>>  char const
>>>>>>>>>  *psz_cmd, }
>>>>> 
>>>>>           else
>>>>> 
>>>>>           {
>>>>> 
>>>>>  -            var_SetInteger( p_intf->obj.libvlc, "key-action",
>>>>>>>>>  ACTIONID_JUMP_BACKWARD_EXTRASHORT ); +           
>vlc_actions_do(
>>>>>  p_intf,
>>>>>>>>>  ACTIONID_JUMP_BACKWARD_EXTRASHORT, true, 0 ); }
>>>>> 
>>>>>           i_error = VLC_SUCCESS;
>>>>> 
>>>>>       }
>>>>> 
>>>>>       else if ( !strcmp( psz_cmd, "faster" ) )
>>>>> 
>>>>>       {
>>>>> 
>>>>>  -        var_TriggerCallback( p_intf->p_sys->p_playlist, "rate-
>>>>>           faster" );
>>>>>>>>>  +        vlc_actions_do( p_intf, ACTIONID_FASTER, true, 0 );
>>>>>>>>  
>>>> 
>>>>  Ditto.
>>>> 
>>>>  
>>>> 
>>>>>           i_error = VLC_SUCCESS;
>>>>> 
>>>>>       }
>>>>> 
>>>>>       else if ( !strcmp( psz_cmd, "slower" ) )
>>>>> 
>>>>>       {
>>>>> 
>>>>>  -        var_TriggerCallback( p_intf->p_sys->p_playlist, "rate-
>>>>>           slower" );
>>>>>>>>>  +        vlc_actions_do( p_intf, ACTIONID_SLOWER, true, 0 );
>>>>>>>>  
>>>> 
>>>>  Ditto.
>>>> 
>>>>  
>>>> 
>>>>>           i_error = VLC_SUCCESS;
>>>>> 
>>>>>       }
>>>>> 
>>>>>       else if ( !strcmp( psz_cmd, "normal" ) )
>>>>> 
>>>>>  @@ -1022,7 +1021,7 @@ static int Input( vlc_object_t *p_this,
>char
>>>>>  const
>>>>>>>>>  *psz_cmd, }
>>>>> 
>>>>>       else if ( !strcmp( psz_cmd, "frame" ) )
>>>>> 
>>>>>       {
>>>>> 
>>>>>  -        var_TriggerCallback( p_input, "frame-next" );
>>>>> 
>>>>>  +        vlc_actions_do( p_intf, ACTIONID_RATE_NORMAL, true, 0 );
>>>>>>>>  
>>>> 
>>>>  Ditto.
>>>> 
>>>>  
>>>> 
>>>>>           i_error = VLC_SUCCESS;
>>>>> 
>>>>>       }
>>>>> 
>>>>>       else if( !strcmp( psz_cmd, "chapter" ) ||
>>>>> 
>>>>>  @@ -1046,9 +1045,9 @@ static int Input( vlc_object_t *p_this,
>char
>>>>>  const
>>>>>>>>>  *psz_cmd, }
>>>>> 
>>>>>           }
>>>>> 
>>>>>           else if( !strcmp( psz_cmd, "chapter_n" ) )
>>>>> 
>>>>>  -            var_TriggerCallback( p_input, "next-chapter" );
>>>>>>>>>  +            vlc_actions_do( p_intf, ACTIONID_CHAPTER_NEXT,
>true,
>>>>>               1, p_input
>>>>>>>>>  ); else if( !strcmp( psz_cmd, "chapter_p" ) )
>>>>> 
>>>>  
>>>> 
>>>>  Ditto.
>>>> 
>>>>  
>>>> 
>>>>>  -            var_TriggerCallback( p_input, "prev-chapter" );
>>>>>>>>>  +            vlc_actions_do( p_intf, ACTIONID_CHAPTER_PREV,
>true,
>>>>>               1, p_input
>>>>>>>>>  ); i_error = VLC_SUCCESS;
>>>>> 
>>>>  
>>>> 
>>>>  Ditto.
>>>> 
>>>>  
>>>> 
>>>>>       }
>>>>> 
>>>>>       else if( !strcmp( psz_cmd, "title" ) ||
>>>>> 
>>>>>  @@ -1070,9 +1069,9 @@ static int Input( vlc_object_t *p_this,
>char
>>>>>  const
>>>>>>>>>  *psz_cmd, }
>>>>> 
>>>>>           }
>>>>> 
>>>>>           else if( !strcmp( psz_cmd, "title_n" ) )
>>>>> 
>>>>>  -            var_TriggerCallback( p_input, "next-title" );
>>>>> 
>>>>>  +            vlc_actions_do( p_intf, ACTIONID_TITLE_NEXT, true,
>1,
>>>>>               p_input
>>>>>>>>>  ); else if( !strcmp( psz_cmd, "title_p" ) )
>>>>> 
>>>>>  -            var_TriggerCallback( p_input, "prev-title" );
>>>>> 
>>>>>  +            vlc_actions_do( p_intf, ACTIONID_TITLE_PREV, true,
>1,
>>>>>               p_input
>>>>>>>>  
>>>> 
>>>>  Both ditto.
>>>> 
>>>>  
>>>> 
>>>>>  );
>>>>> 
>>>>>  
>>>>> 
>>>>>           i_error = VLC_SUCCESS;
>>>>> 
>>>>>       }
>>>>> 
>>>>  
>>>> 
>>>>  
>>>> 
>>> 
>>> 
>>> To avoid spreading handling of actions in multiple modules and
>>> 
>>> centralize the "key" -> "response" in a single place, as opposed to
>>>>> having the same code being spread out in various modules.
>>> 
>>> Granted some of those are one liners, but when some logic is
>>> necessary
>>>>> to handle the action, it shouldn't be duplicated IMHO.
>>> 
>> 
>> I was _not_ referring to existing actions, but to every other cases,
>> where they are neither keys nor actions to demultiplex as yet.
>Fair enough, but we have proven, in the past, as we can see in most of
>the interfaces and controls module, that the more intf touch variables
>directly, the worse it is.
>--
>Jean-Baptiste Kempf -  President
>+33 672 704 734
> 

I dislike the VLC variables subsystem too. But replacing a variable setting with a multiplexed call to the same variable setting, does not look like an improvement to me. Did I miss something?
-- 
Rémi Denis-Courmont
Typed on an inconvenient virtual keyboard
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20170810/74977031/attachment.html>


More information about the vlc-devel mailing list