[vlc-devel] [PATCH] Updated mrpis to spec 2.2
mirsal
mirsal at mirsal.fr
Thu Feb 9 15:05:59 CET 2012
Hi Kevin, and thanks for the patch !
Comments inline.
On Wed, 2012-02-08 at 21:54 -0500, Kevin Anthony wrote:
> Added Fullscreen and CanSetFullscreen properties,
> including the underlying methods to set properties in dbus_root
> ---
> modules/control/dbus/dbus.c | 13 ++-
> modules/control/dbus/dbus_common.h | 3 +-
> modules/control/dbus/dbus_introspect.h | 2 +
> modules/control/dbus/dbus_root.c | 175 ++++++++++++++++++++++++++++++++
> modules/control/dbus/dbus_root.h | 2 +
> 5 files changed, 193 insertions(+), 2 deletions(-)
>
> diff --git a/modules/control/dbus/dbus.c b/modules/control/dbus/dbus.c
> index 21945e2..e238870 100644
> --- a/modules/control/dbus/dbus.c
> +++ b/modules/control/dbus/dbus.c
> @@ -531,9 +531,10 @@ static void ProcessEvents( intf_thread_t *p_intf,
> playlist_t *p_playlist = p_intf->p_sys->p_playlist;
> bool b_can_play = p_intf->p_sys->b_can_play;
>
> - vlc_dictionary_t player_properties, tracklist_properties;
> + vlc_dictionary_t player_properties, tracklist_properties, root_properties;
> vlc_dictionary_init( &player_properties, 0 );
> vlc_dictionary_init( &tracklist_properties, 0 );
> + vlc_dictionary_init( &root_properties, 0 );
>
> for( int i = 0; i < i_events; i++ )
> {
> @@ -565,6 +566,9 @@ static void ProcessEvents( intf_thread_t *p_intf,
> case SIGNAL_RANDOM:
> vlc_dictionary_insert( &player_properties, "Shuffle", NULL );
> break;
> + case SIGNAL_FULLSCREEN:
> + vlc_dictionary_insert( &root_properties, "Fullscreen", NULL );
> + break;
> case SIGNAL_REPEAT:
> case SIGNAL_LOOP:
> vlc_dictionary_insert( &player_properties, "LoopStatus", NULL );
> @@ -623,8 +627,12 @@ static void ProcessEvents( intf_thread_t *p_intf,
> if( vlc_dictionary_keys_count( &tracklist_properties ) )
> TrackListPropertiesChangedEmit( p_intf, &tracklist_properties );
>
> + if( vlc_dictionary_keys_count( &root_properties ) )
> + RootPropertiesChangedEmit( p_intf, &root_properties );
> +
> vlc_dictionary_clear( &player_properties, NULL, NULL );
> vlc_dictionary_clear( &tracklist_properties, NULL, NULL );
> + vlc_dictionary_clear( &root_properties, NULL, NULL );
> }
>
> /**
> @@ -1049,6 +1057,9 @@ static int AllCallback( vlc_object_t *p_this, const char *psz_var,
> else if( !strcmp( "random", psz_var ) )
> info->signal = SIGNAL_RANDOM;
>
> + else if( !strcmp( "fullscreen", psz_var ) )
> + info->signal = SIGNAL_FULLSCREEN;
> +
The AllCallback function is not connected to any variable change event
with that name. This will never be executed unless you add a proper call
to AddCallback when initializing the module, which should be done in
dbus.c, in the Open() function.
> else if( !strcmp( "repeat", psz_var ) )
> info->signal = SIGNAL_REPEAT;
>
> diff --git a/modules/control/dbus/dbus_common.h b/modules/control/dbus/dbus_common.h
> index c977138..30d6e2c 100644
> --- a/modules/control/dbus/dbus_common.h
> +++ b/modules/control/dbus/dbus_common.h
> @@ -121,7 +121,8 @@ enum
> SIGNAL_CAN_SEEK,
> SIGNAL_CAN_PAUSE,
> SIGNAL_VOLUME_CHANGE,
> - SIGNAL_VOLUME_MUTED
> + SIGNAL_VOLUME_MUTED,
> + SIGNAL_FULLSCREEN
> };
>
> enum
> diff --git a/modules/control/dbus/dbus_introspect.h b/modules/control/dbus/dbus_introspect.h
> index 5056c9b..cc53e45 100644
> --- a/modules/control/dbus/dbus_introspect.h
> +++ b/modules/control/dbus/dbus_introspect.h
> @@ -63,6 +63,8 @@ static const char* psz_introspection_xml =
> " <property name=\"SupportedUriSchemes\" type=\"as\" access=\"read\" />\n"
> " <property name=\"HasTrackList\" type=\"b\" access=\"read\" />\n"
> " <property name=\"CanQuit\" type=\"b\" access=\"read\" />\n"
> +" <property name=\"CanSetFullscreen\" type=\"b\" access=\"read\" />\n"
> +" <property name=\"Fullscreen\" type=\"b\" access=\"readwrite\" />\n"
> " <property name=\"CanRaise\" type=\"b\" access=\"read\" />\n"
> " <method name=\"Quit\" />\n"
> " <method name=\"Raise\" />\n"
> diff --git a/modules/control/dbus/dbus_root.c b/modules/control/dbus/dbus_root.c
> index 5de8af6..1b92463 100644
> --- a/modules/control/dbus/dbus_root.c
> +++ b/modules/control/dbus/dbus_root.c
> @@ -30,6 +30,10 @@
>
> #include <vlc_common.h>
> #include <vlc_interface.h>
> +#include <vlc_input.h>
> +#include <vlc_vout.h>
> +#include <vlc_plugin.h>
> +#include <vlc_playlist.h>
>
> #include <unistd.h>
> #include <limits.h>
> @@ -95,6 +99,79 @@ DBUS_METHOD( Identity )
> }
>
> static int
> +MarshalCanSetFullscreen( intf_thread_t *p_intf, DBusMessageIter *container )
> +{
> + VLC_UNUSED( p_intf );
> + const dbus_bool_t b_ret = TRUE;
I am not sure that VLC's fullscreen state can be changed at all time.
> +
> + dbus_message_iter_append_basic( container, DBUS_TYPE_BOOLEAN, &b_ret );
> + return VLC_SUCCESS;
> +}
> +
> +DBUS_METHOD( CanSetFullscreen )
> +{
> + REPLY_INIT;
> + OUT_ARGUMENTS;
> +
> + DBusMessageIter v;
> + dbus_message_iter_open_container( &args, DBUS_TYPE_VARIANT, "b", &v );
> +
> + MarshalCanSetFullscreen( p_this, &v );
> +
> + if( !dbus_message_iter_close_container( &args, &v ) )
> + return DBUS_HANDLER_RESULT_NEED_MEMORY;
> +
> + REPLY_SEND;
> +}
> +
> +static void
> +MarshalFullscreen( intf_thread_t *p_intf, DBusMessageIter *container )
> +{
> + vout_thread_t *p_vout = (vout_thread_t *)p_intf;
> + dbus_bool_t b_fullscreen;
> + if ( p_vout ) {
> + b_fullscreen = var_GetBool( p_vout , "fullscreen" );
> + } else {
> + b_fullscreen = FALSE;
> + }
Coding style (have a look at: http://wiki.videolan.org/Code_Conventions)
> + dbus_message_iter_append_basic( container, DBUS_TYPE_BOOLEAN, &b_fullscreen );
> +}
> +
> +DBUS_METHOD( FullscreenGet )
> +{
> + REPLY_INIT;
> + OUT_ARGUMENTS;
> +
> + DBusMessageIter v;
> +
> + if( !dbus_message_iter_open_container( &args, DBUS_TYPE_VARIANT, "b", &v ) )
> + return DBUS_HANDLER_RESULT_NEED_MEMORY;
> +
> + MarshalFullscreen( p_this, &v );
> +
> + if( !dbus_message_iter_close_container( &args, &v ) )
> + return DBUS_HANDLER_RESULT_NEED_MEMORY;
> +
> + REPLY_SEND;
> +}
> +
> +DBUS_METHOD( FullscreenSet )
> +{
> + REPLY_INIT;
> + dbus_bool_t b_fullscreen;
> +
> + if( VLC_SUCCESS != DemarshalSetPropertyValue( p_from, &b_fullscreen ) )
> + return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
> +
> + vout_thread_t *p_vout = (vout_thread_t *)p_this;
This is not the right way to access the vout thread (it is done by
calling input_GetVout with an input thread as an argument) and anyway
you should rather use the "fullscreen" variable provided by the playlist
object, which nicely hides the details of manipulating the input and
vout threads away.
> +
> + if ( p_vout ){
> + var_SetBool( p_vout, "fullscreen", ( b_fullscreen == TRUE ) );
> + }
Coding style, and see the comment above about using the wrong variable.
> + REPLY_SEND;
> +}
> +
> +static int
> MarshalCanQuit( intf_thread_t *p_intf, DBusMessageIter *container )
> {
> VLC_UNUSED( p_intf );
> @@ -343,10 +420,38 @@ DBUS_METHOD( GetProperty )
> PROPERTY_FUNC( DBUS_MPRIS_ROOT_INTERFACE, "SupportedUriSchemes", SupportedUriSchemes )
> PROPERTY_FUNC( DBUS_MPRIS_ROOT_INTERFACE, "HasTrackList", HasTrackList )
> PROPERTY_FUNC( DBUS_MPRIS_ROOT_INTERFACE, "CanQuit", CanQuit )
> + PROPERTY_FUNC( DBUS_MPRIS_ROOT_INTERFACE, "CanSetFullscreen", CanSetFullscreen )
> + PROPERTY_FUNC( DBUS_MPRIS_ROOT_INTERFACE, "Fullscreen", FullscreenGet )
> PROPERTY_FUNC( DBUS_MPRIS_ROOT_INTERFACE, "CanRaise", CanRaise )
> PROPERTY_MAPPING_END
> }
>
> +DBUS_METHOD( SetProperty )
> +{
> + DBusError error;
> +
> + char *psz_interface_name = NULL;
> + char *psz_property_name = NULL;
> +
> + dbus_error_init( &error );
> + dbus_message_get_args( p_from, &error,
> + DBUS_TYPE_STRING, &psz_interface_name,
> + DBUS_TYPE_STRING, &psz_property_name,
> + DBUS_TYPE_INVALID );
> +
> + if( dbus_error_is_set( &error ) )
> + {
> + msg_Err( (vlc_object_t*) p_this, "D-Bus message reading : %s",
> + error.message );
> + dbus_error_free( &error );
> + return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
> + }
> +
> + PROPERTY_MAPPING_BEGIN
> + PROPERTY_FUNC( DBUS_MPRIS_ROOT_INTERFACE, "Fullscreen", FullscreenSet )
> + PROPERTY_MAPPING_END
> +}
> +
> #undef PROPERTY_MAPPING_BEGIN
> #undef PROPERTY_GET_FUNC
> #undef PROPERTY_MAPPING_END
> @@ -425,6 +530,7 @@ DBUS_METHOD( GetAllProperties )
> ADD_PROPERTY( SupportedUriSchemes, "as" );
> ADD_PROPERTY( HasTrackList, "b" );
> ADD_PROPERTY( CanQuit, "b" );
> + ADD_PROPERTY( CanSetFullscreen, "b" );
> ADD_PROPERTY( CanRaise, "b" );
>
> dbus_message_iter_close_container( &args, &dict );
> @@ -444,6 +550,7 @@ handle_root ( DBusConnection *p_conn, DBusMessage *p_from, void *p_this )
> {
> METHOD_MAPPING_BEGIN
> METHOD_FUNC( DBUS_INTERFACE_PROPERTIES, "Get", GetProperty );
> + METHOD_FUNC( DBUS_INTERFACE_PROPERTIES, "Set", SetProperty );
> METHOD_FUNC( DBUS_INTERFACE_PROPERTIES, "GetAll", GetAllProperties );
> METHOD_FUNC( DBUS_MPRIS_ROOT_INTERFACE, "Quit", Quit );
> METHOD_FUNC( DBUS_MPRIS_ROOT_INTERFACE, "Raise", Raise );
> @@ -453,3 +560,71 @@ handle_root ( DBusConnection *p_conn, DBusMessage *p_from, void *p_this )
> #undef METHOD_MAPPING_BEGIN
> #undef METHOD_FUNC
> #undef METHOD_MAPPING_END
> +/**
> + * PropertiesChangedSignal() synthetizes and sends the
> + * org.freedesktop.DBus.Properties.PropertiesChanged signal
> + */
> +
> +static DBusHandlerResult
> +PropertiesChangedSignal( intf_thread_t *p_intf,
> + vlc_dictionary_t *p_changed_properties )
> +{
> + DBusConnection *p_conn = p_intf->p_sys->p_conn;
> + DBusMessageIter changed_properties, invalidated_properties, entry, variant;
> + const char *psz_interface_name = DBUS_MPRIS_ROOT_INTERFACE;
> + char **ppsz_properties = NULL;
> + int i_properties = 0;
> +
> + SIGNAL_INIT( DBUS_INTERFACE_PROPERTIES,
> + DBUS_MPRIS_OBJECT_PATH,
> + "PropertiesChanged" );
> +
> + OUT_ARGUMENTS;
> + ADD_STRING( &psz_interface_name );
> + dbus_message_iter_open_container( &args, DBUS_TYPE_ARRAY, "{sv}",
> + &changed_properties );
> +
> + i_properties = vlc_dictionary_keys_count( p_changed_properties );
> + ppsz_properties = vlc_dictionary_all_keys( p_changed_properties );
> +
> + for( int i = 0; i < i_properties; i++ )
> + {
> + dbus_message_iter_open_container( &changed_properties,
> + DBUS_TYPE_DICT_ENTRY, NULL,
> + &entry );
> +
> + dbus_message_iter_append_basic( &entry, DBUS_TYPE_STRING,
> + &ppsz_properties[i] );
> +
> + if( !strcmp( ppsz_properties[i], "Fullscreen" ) )
> + {
> + dbus_message_iter_open_container( &entry,
> + DBUS_TYPE_VARIANT, "b",
> + &variant );
> + MarshalFullscreen( p_intf, &variant );
> + dbus_message_iter_close_container( &entry, &variant );
> + }
> + dbus_message_iter_close_container( &changed_properties, &entry );
> + free( ppsz_properties[i] );
> + }
> + dbus_message_iter_close_container( &args, &changed_properties );
> + dbus_message_iter_open_container( &args, DBUS_TYPE_ARRAY, "s",
> + &invalidated_properties );
> + dbus_message_iter_close_container( &args, &invalidated_properties );
> + free( ppsz_properties );
> +
> + SIGNAL_SEND;
> +}
> +
> +/*****************************************************************************
> + * PropertiesChangedEmitRoot: Emits the Seeked signal
> + *****************************************************************************/
Wrong function name ;)
> +int RootPropertiesChangedEmit( intf_thread_t * p_intf,
> + vlc_dictionary_t * p_changed_properties )
> +{
> + if( p_intf->p_sys->b_dead )
> + return VLC_SUCCESS;
> +
> + PropertiesChangedSignal( p_intf, p_changed_properties );
> + return VLC_SUCCESS;
> +}
> diff --git a/modules/control/dbus/dbus_root.h b/modules/control/dbus/dbus_root.h
> index 38267ee..86adaed 100644
> --- a/modules/control/dbus/dbus_root.h
> +++ b/modules/control/dbus/dbus_root.h
> @@ -36,5 +36,7 @@
> DBusHandlerResult handle_root ( DBusConnection *p_conn,
> DBusMessage *p_from,
> void *p_this );
> +int RootPropertiesChangedEmit ( intf_thread_t *,
Trailing whitespace
> + vlc_dictionary_t * );
>
> #endif //dbus-root.h
These are trivial changes, so I can just amend your patch myself if you want.
Thanks !
All the best,
--
mirsal
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20120209/88236e10/attachment.sig>
More information about the vlc-devel
mailing list