[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