[Merge] lp:~thomas-voss/media-hub/complete-mpris-spec into lp:media-hub

Nick Dedekind nick.dedekind at canonical.com
Wed Aug 27 10:01:59 UTC 2014


Review: Needs Fixing

Just a few comments attached. Nothing marjor, otherwise everything looks ok.

Diff comments:

> === modified file 'src/core/media/CMakeLists.txt'
> --- src/core/media/CMakeLists.txt	2014-08-08 14:36:29 +0000
> +++ src/core/media/CMakeLists.txt	2014-08-25 14:53:44 +0000
> @@ -1,6 +1,8 @@
>  pkg_check_modules(PC_GSTREAMER_1_0 REQUIRED gstreamer-1.0)
>  include_directories(${PC_GSTREAMER_1_0_INCLUDE_DIRS} ${HYBRIS_MEDIA_CFLAGS})
>  
> +file(GLOB MPRIS_HEADERS mpris/ *.h)
> +
>  add_library(
>    media-hub-common SHARED
>  
> @@ -62,6 +64,8 @@
>  add_library(
>      media-hub-service
>  
> +    ${MPRIS_HEADERS}
> +
>      engine.cpp
>      gstreamer/engine.cpp
>  
> 
> === removed file 'src/core/media/mpris/macros.h'
> --- src/core/media/mpris/macros.h	2014-07-10 12:19:48 +0000
> +++ src/core/media/mpris/macros.h	1970-01-01 00:00:00 +0000
> @@ -1,91 +0,0 @@
> -/*
> - * Copyright © 2013 Canonical Ltd.
> - *
> - * This program is free software: you can redistribute it and/or modify it
> - * under the terms of the GNU Lesser General Public License version 3,
> - * as published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - * GNU Lesser General Public License for more details.
> - *
> - * You should have received a copy of the GNU Lesser General Public License
> - * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> - *
> - * Authored by: Thomas Voß <thomas.voss at canonical.com>
> - */
> -
> -#ifndef MPRIS_MACROS_H_
> -#define MPRIS_MACROS_H_
> -
> -#include <core/dbus/types/object_path.h>
> -
> -#include <chrono>
> -#include <string>
> -
> -#define SECONDS(seconds) std::chrono::seconds{seconds};
> -
> -#define METHOD_WITH_TIMEOUT_MS(Name, Itf, Timeout) \
> -    struct Name \
> -    { \
> -        typedef Itf Interface; \
> -        inline static const std::string& name() \
> -        { \
> -            static const std::string s{#Name}; \
> -            return s; \
> -        } \
> -        inline static const std::chrono::milliseconds default_timeout() { return std::chrono::milliseconds{Timeout}; } \
> -    };\
> -
> -#define METHOD(Name, Itf, Timeout) \
> -    struct Name \
> -    { \
> -        typedef Itf Interface; \
> -        inline static const std::string& name() \
> -        { \
> -            static const std::string s{#Name}; \
> -            return s; \
> -        } \
> -        inline static const std::chrono::milliseconds default_timeout() { return std::chrono::milliseconds{7000}; } \
> -    };\
> -
> -#define SIGNAL(Name, Itf, ArgType) \
> -    struct Name \
> -    { \
> -        inline static std::string name() \
> -        { \
> -            return #Name; \
> -        }; \
> -        typedef Itf Interface; \
> -        typedef ArgType ArgumentType; \
> -    };\
> -
> -#define READABLE_PROPERTY(Name, Itf, Type) \
> -    struct Name \
> -    { \
> -        inline static std::string name() \
> -        { \
> -            return #Name; \
> -        }; \
> -        typedef Itf Interface; \
> -        typedef Type ValueType; \
> -        static const bool readable = true; \
> -        static const bool writable = false; \
> -    }; \
> -
> -#define WRITABLE_PROPERTY(Name, Itf, Type) \
> -    struct Name \
> -    { \
> -        inline static std::string name() \
> -        { \
> -            return #Name; \
> -        }; \
> -        typedef Itf Interface; \
> -        typedef Type ValueType; \
> -        static const bool readable = true; \
> -        static const bool writable = true; \
> -    }; \
> -
> -#endif // MPRIS_MACROS_H_
> -
> 
> === added file 'src/core/media/mpris/media_player2.h'
> --- src/core/media/mpris/media_player2.h	1970-01-01 00:00:00 +0000
> +++ src/core/media/mpris/media_player2.h	2014-08-25 14:53:44 +0000
> @@ -0,0 +1,146 @@
> +/*
> + * Copyright © 2013 Canonical Ltd.

2014

> + *
> + * This program is free software: you can redistribute it and/or modify it
> + * under the terms of the GNU Lesser General Public License version 3,
> + * as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + *
> + * Authored by: Thomas Voß <thomas.voss at canonical.com>
> + */
> +
> +#ifndef MPRIS_MEDIA_PLAYER2_H_
> +#define MPRIS_MEDIA_PLAYER2_H_
> +
> +#include <core/dbus/macros.h>
> +#include <core/dbus/object.h>
> +#include <core/dbus/property.h>
> +
> +#include <string>
> +#include <vector>
> +
> +namespace mpris
> +{
> +// Models interface org.mpris.MediaPlayer2, see:
> +//   http://specifications.freedesktop.org/mpris-spec/latest/Media_Player.html
> +// for detailed documentation
> +struct MediaPlayer2
> +{
> +    static const std::string& name()
> +    {
> +        static const std::string s{"org.mpris.MediaPlayer2"}; return s;
> +    }
> +
> +    struct Methods
> +    {
> +        // Brings the media player's user interface to the front using any appropriate
> +        // mechanism available.
> +        // The media player may be unable to control how its user interface is displayed,
> +        // or it may not have a graphical user interface at all. In this case,
> +        // the CanRaise property is false and this method does nothing.
> +        DBUS_CPP_METHOD_DEF(Raise, MediaPlayer2)
> +
> +        // Causes the media player to stop running.
> +        // The media player may refuse to allow clients to shut it down. In this case, the
> +        // CanQuit property is false and this method does nothing.
> +        DBUS_CPP_METHOD_DEF(Quit, MediaPlayer2)
> +    };
> +
> +    struct Properties
> +    {
> +        // If false, calling Quit will have no effect, and may raise a NotSupported error.
> +        // If true, calling Quit will cause the media application to attempt to quit
> +        // (although it may still be prevented from quitting by the user, for example).
> +        DBUS_CPP_READABLE_PROPERTY_DEF(CanQuit, MediaPlayer2, bool)
> +
> +        // Whether the media player is occupying the fullscreen.
> +        // This property is optional. Clients should handle its absence gracefully.
> +        DBUS_CPP_WRITABLE_PROPERTY_DEF(Fullscreen, MediaPlayer2, bool)
> +
> +        // If false, attempting to set Fullscreen will have no effect, and may raise an error.
> +        // If true, attempting to set Fullscreen will not raise an error, and (if it is different
> +        // from the current value) will cause the media player to attempt to enter or exit fullscreen mode.
> +        // This property is optional. Clients should handle its absence gracefully.
> +        DBUS_CPP_READABLE_PROPERTY_DEF(CanSetFullscreen, MediaPlayer2, bool)
> +
> +        // If false, calling Raise will have no effect, and may raise a NotSupported error. If true, calling Raise
> +        // will cause the media application to attempt to bring its user interface to the front,
> +        // although it may be prevented from doing so (by the window manager, for example).
> +        DBUS_CPP_READABLE_PROPERTY_DEF(CanRaise, MediaPlayer2, bool)
> +
> +        // Indicates whether the /org/mpris/MediaPlayer2 object implements the
> +        // org.mpris.MediaPlayer2.TrackList interface.
> +        DBUS_CPP_READABLE_PROPERTY_DEF(HasTrackList, MediaPlayer2, bool)
> +
> +        // A friendly name to identify the media player to users.
> +        DBUS_CPP_READABLE_PROPERTY_DEF(Identity, MediaPlayer2, std::string)
> +
> +        // The basename of an installed .desktop file which complies with the Desktop entry specification,
> +        // with the ".desktop" extension stripped.
> +        // This property is optional. Clients should handle its absence gracefully.
> +        DBUS_CPP_READABLE_PROPERTY_DEF(DesktopEntry, MediaPlayer2, std::string)
> +
> +        // The URI schemes supported by the media player.
> +        DBUS_CPP_READABLE_PROPERTY_DEF(SupportedUriSchemes, MediaPlayer2, std::vector<std::string>)
> +
> +        // The mime-types supported by the media player.
> +        DBUS_CPP_READABLE_PROPERTY_DEF(SupportedMimeTypes, MediaPlayer2, std::vector<std::string>)
> +    };
> +
> +    struct Skeleton
> +    {
> +        // Creation time properties go here.
> +        struct Configuration
> +        {
> +            // The bus connection that should be used
> +            core::dbus::Bus::Ptr bus;
> +            // The dbus object that should implement org.mpris.MediaPlayer2
> +            core::dbus::Object::Ptr object;     
> +        };
> +
> +        // Creates a new instance, sets up player properties and installs method handlers.
> +        Skeleton(const Configuration& configuration)
> +            : configuration(configuration),
> +              properties
> +              {
> +                  configuration.object->get_property<Properties::CanQuit>(),
> +                  configuration.object->get_property<Properties::Fullscreen>(),
> +                  configuration.object->get_property<Properties::CanSetFullscreen>(),
> +                  configuration.object->get_property<Properties::CanRaise>(),
> +                  configuration.object->get_property<Properties::HasTrackList>(),
> +                  configuration.object->get_property<Properties::Identity>(),
> +                  configuration.object->get_property<Properties::DesktopEntry>(),
> +                  configuration.object->get_property<Properties::SupportedUriSchemes>(),
> +                  configuration.object->get_property<Properties::SupportedMimeTypes>()
> +              }
> +        {            
> +        }
> +
> +        // We just store creation time properties here.
> +        Configuration configuration;
> +
> +        // All property instances go here.
> +        struct
> +        {
> +            std::shared_ptr<core::dbus::Property<Properties::CanQuit>> can_quit;
> +            std::shared_ptr<core::dbus::Property<Properties::Fullscreen>> fullscreen;
> +            std::shared_ptr<core::dbus::Property<Properties::CanSetFullscreen>> can_set_fullscreen;
> +            std::shared_ptr<core::dbus::Property<Properties::CanRaise>> can_raise;
> +            std::shared_ptr<core::dbus::Property<Properties::HasTrackList>> has_track_list;
> +            std::shared_ptr<core::dbus::Property<Properties::Identity>> identity;
> +            std::shared_ptr<core::dbus::Property<Properties::DesktopEntry>> desktop_entry;
> +            std::shared_ptr<core::dbus::Property<Properties::SupportedUriSchemes>> supported_uri_schemes;
> +            std::shared_ptr<core::dbus::Property<Properties::SupportedMimeTypes>> supported_mime_types;
> +        } properties;
> +    };
> +};
> +}
> +
> +#endif // MPRIS_MEDIA_PLAYER2_H_
> 
> === modified file 'src/core/media/mpris/player.h'
> --- src/core/media/mpris/player.h	2014-07-10 12:19:48 +0000
> +++ src/core/media/mpris/player.h	2014-08-25 14:53:44 +0000
> @@ -22,8 +22,12 @@
>  #include <core/media/player.h>
>  #include <core/media/track.h>
>  
> -#include "macros.h"
> +#include "core/media/codec.h"
>  
> +#include <core/dbus/bus.h>
> +#include <core/dbus/macros.h>
> +#include <core/dbus/object.h>
> +#include <core/dbus/property.h>
>  #include <core/dbus/types/any.h>
>  #include <core/dbus/types/object_path.h>
>  #include <core/dbus/types/variant.h>
> @@ -42,50 +46,175 @@
>  {
>      static const std::string& name()
>      {
> -        static const std::string s{"core.ubuntu.media.Service.Player"};
> +        static const std::string s{"org.mpris.MediaPlayer2.Player"};
>          return s;
>      }
>  
> -    METHOD(Next, Player, std::chrono::seconds(1))
> -    METHOD(Previous, Player, std::chrono::seconds(1))
> -    METHOD_WITH_TIMEOUT_MS(Pause, Player, 1000)
> -    METHOD(PlayPause, Player, std::chrono::seconds(1))
> -    METHOD(Stop, Player, std::chrono::seconds(1))
> -    METHOD(Play, Player, std::chrono::seconds(1))
> -    METHOD(Seek, Player, std::chrono::seconds(1))
> -    METHOD(SetPosition, Player, std::chrono::seconds(1))
> -    METHOD(CreateVideoSink, Player, std::chrono::seconds(1))
> -    METHOD(Key, Player, std::chrono::seconds(1))
> -    METHOD(OpenUri, Player, std::chrono::seconds(1))
> +    struct LoopStatus
> +    {
> +        LoopStatus() = delete;
> +
> +        static constexpr const char* none{"None"};
> +        static constexpr const char* track{"Track"};
> +        static constexpr const char* playlist{"Playlist"};
> +    };
> +
> +    struct PlaybackStatus
> +    {
> +        PlaybackStatus() = delete;
> +
> +        static const char* from(core::ubuntu::media::Player::PlaybackStatus status)
> +        {
> +            switch(status)
> +            {
> +            case core::ubuntu::media::Player::PlaybackStatus::null:
> +            case core::ubuntu::media::Player::PlaybackStatus::ready:
> +            case core::ubuntu::media::Player::PlaybackStatus::stopped:
> +                return PlaybackStatus::stopped;
> +
> +            case core::ubuntu::media::Player::PlaybackStatus::playing:
> +                return PlaybackStatus::playing;
> +            case core::ubuntu::media::Player::PlaybackStatus::paused:
> +                return PlaybackStatus::paused;
> +            }
> +
> +            return nullptr;
> +        }
> +
> +        static constexpr const char* playing{"Playing"};
> +        static constexpr const char* paused{"Paused"};
> +        static constexpr const char* stopped{"Stopped"};
> +    };
> +
> +    typedef std::map<std::string, core::dbus::types::Variant> Dictionary;
> +
> +    DBUS_CPP_METHOD_DEF(Next, Player)
> +    DBUS_CPP_METHOD_DEF(Previous, Player)
> +    DBUS_CPP_METHOD_DEF(Pause, Player)
> +    DBUS_CPP_METHOD_DEF(PlayPause, Player)
> +    DBUS_CPP_METHOD_DEF(Stop, Player)
> +    DBUS_CPP_METHOD_DEF(Play, Player)
> +    DBUS_CPP_METHOD_DEF(Seek, Player)
> +    DBUS_CPP_METHOD_DEF(SetPosition, Player)
> +    DBUS_CPP_METHOD_DEF(CreateVideoSink, Player)
> +    DBUS_CPP_METHOD_DEF(Key, Player)
> +    DBUS_CPP_METHOD_DEF(OpenUri, Player)
>  
>      struct Signals
>      {
> -        SIGNAL(Seeked, Player, uint64_t)
> -        SIGNAL(EndOfStream, Player, void)
> -        SIGNAL(PlaybackStatusChanged, Player, core::ubuntu::media::Player::PlaybackStatus)
> +        DBUS_CPP_SIGNAL_DEF(Seeked, Player, std::uint64_t)
> +        DBUS_CPP_SIGNAL_DEF(EndOfStream, Player, void)
> +        DBUS_CPP_SIGNAL_DEF(PlaybackStatusChanged, Player, core::ubuntu::media::Player::PlaybackStatus)
>      };
>  
>      struct Properties
>      {
> -        READABLE_PROPERTY(PlaybackStatus, Player, core::ubuntu::media::Player::PlaybackStatus)
> -        WRITABLE_PROPERTY(LoopStatus, Player, core::ubuntu::media::Player::LoopStatus)
> -        WRITABLE_PROPERTY(PlaybackRate, Player, core::ubuntu::media::Player::PlaybackRate)
> -        WRITABLE_PROPERTY(Rate, Player, double)
> -        WRITABLE_PROPERTY(Shuffle, Player, bool)
> -        READABLE_PROPERTY(MetaData, Player, core::ubuntu::media::Track::MetaData)
> -        WRITABLE_PROPERTY(Volume, Player, double)
> -        READABLE_PROPERTY(Position, Player, uint64_t)
> -        READABLE_PROPERTY(Duration, Player, uint64_t)
> -        READABLE_PROPERTY(MinimumRate, Player, double)
> -        READABLE_PROPERTY(MaximumRate, Player, double)
> -        READABLE_PROPERTY(IsVideoSource, Player, bool)
> -        READABLE_PROPERTY(IsAudioSource, Player, bool)
> -        READABLE_PROPERTY(CanGoNext, Player, bool)
> -        READABLE_PROPERTY(CanGoPrevious, Player, bool)
> -        READABLE_PROPERTY(CanPlay, Player, bool)
> -        READABLE_PROPERTY(CanPause, Player, bool)
> -        READABLE_PROPERTY(CanSeek, Player, bool)
> -        READABLE_PROPERTY(CanControl, Player, bool)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(PlaybackStatus, Player, std::string)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(TypedPlaybackStatus, Player, core::ubuntu::media::Player::PlaybackStatus)
> +
> +        DBUS_CPP_WRITABLE_PROPERTY_DEF(LoopStatus, Player, std::string)
> +        DBUS_CPP_WRITABLE_PROPERTY_DEF(TypedLoopStatus, Player, core::ubuntu::media::Player::LoopStatus)
> +
> +        DBUS_CPP_WRITABLE_PROPERTY_DEF(PlaybackRate, Player, double)
> +        DBUS_CPP_WRITABLE_PROPERTY_DEF(Rate, Player, double)
> +        DBUS_CPP_WRITABLE_PROPERTY_DEF(Shuffle, Player, bool)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(MetaData, Player, Dictionary)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(TypedMetaData, Player, core::ubuntu::media::Track::MetaData)
> +        DBUS_CPP_WRITABLE_PROPERTY_DEF(Volume, Player, double)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(Position, Player, std::uint64_t)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(Duration, Player, std::uint64_t)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(MinimumRate, Player, double)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(MaximumRate, Player, double)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(IsVideoSource, Player, bool)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(IsAudioSource, Player, bool)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(CanGoNext, Player, bool)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(CanGoPrevious, Player, bool)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(CanPlay, Player, bool)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(CanPause, Player, bool)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(CanSeek, Player, bool)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(CanControl, Player, bool)
> +    };
> +
> +    // Convenience struct to create a skeleton implementation for org.mpris.MediaPlayer2.Player
> +    struct Skeleton
> +    {
> +        // Creation time options go here.
> +        struct Configuration
> +        {
> +            // The bus connection that should be used
> +            core::dbus::Bus::Ptr bus;
> +            // The dbus object that should implement org.mpris.MediaPlayer2
> +            core::dbus::Object::Ptr object;
> +        };
> +
> +        Skeleton(const Configuration& configuration)
> +            : configuration(configuration),
> +              properties
> +              {
> +                  configuration.object->template get_property<mpris::Player::Properties::CanPlay>(),
> +                  configuration.object->template get_property<mpris::Player::Properties::CanPause>(),
> +                  configuration.object->template get_property<mpris::Player::Properties::CanSeek>(),
> +                  configuration.object->template get_property<mpris::Player::Properties::CanControl>(),
> +                  configuration.object->template get_property<mpris::Player::Properties::CanGoNext>(),
> +                  configuration.object->template get_property<mpris::Player::Properties::CanGoPrevious>(),
> +                  configuration.object->template get_property<mpris::Player::Properties::IsVideoSource>(),
> +                  configuration.object->template get_property<mpris::Player::Properties::IsAudioSource>(),
> +                  configuration.object->template get_property<mpris::Player::Properties::PlaybackStatus>(),
> +                  configuration.object->template get_property<mpris::Player::Properties::TypedPlaybackStatus>(),
> +                  configuration.object->template get_property<mpris::Player::Properties::LoopStatus>(),
> +                  configuration.object->template get_property<mpris::Player::Properties::TypedLoopStatus>(),
> +                  configuration.object->template get_property<mpris::Player::Properties::PlaybackRate>(),
> +                  configuration.object->template get_property<mpris::Player::Properties::Shuffle>(),
> +                  configuration.object->template get_property<mpris::Player::Properties::TypedMetaData>(),
> +                  configuration.object->template get_property<mpris::Player::Properties::Volume>(),
> +                  configuration.object->template get_property<mpris::Player::Properties::Position>(),
> +                  configuration.object->template get_property<mpris::Player::Properties::Duration>(),
> +                  configuration.object->template get_property<mpris::Player::Properties::MinimumRate>(),
> +                  configuration.object->template get_property<mpris::Player::Properties::MaximumRate>()
> +              },
> +              signals
> +              {
> +                  configuration.object->template get_signal<mpris::Player::Signals::Seeked>(),
> +                  configuration.object->template get_signal<mpris::Player::Signals::EndOfStream>(),
> +                  configuration.object->template get_signal<mpris::Player::Signals::PlaybackStatusChanged>()
> +              }
> +        {            
> +        }
> +
> +        // We just store creation time properties
> +        Configuration configuration;
> +        // All the properties exposed to the bus go here.
> +        struct
> +        {
> +            std::shared_ptr<core::dbus::Property<mpris::Player::Properties::CanPlay>> can_play;
> +            std::shared_ptr<core::dbus::Property<mpris::Player::Properties::CanPause>> can_pause;
> +            std::shared_ptr<core::dbus::Property<mpris::Player::Properties::CanSeek>> can_seek;
> +            std::shared_ptr<core::dbus::Property<mpris::Player::Properties::CanControl>> can_control;
> +            std::shared_ptr<core::dbus::Property<mpris::Player::Properties::CanGoNext>> can_go_next;
> +            std::shared_ptr<core::dbus::Property<mpris::Player::Properties::CanGoPrevious>> can_go_previous;
> +            std::shared_ptr<core::dbus::Property<mpris::Player::Properties::IsVideoSource>> is_video_source;
> +            std::shared_ptr<core::dbus::Property<mpris::Player::Properties::IsAudioSource>> is_audio_source;
> +
> +            std::shared_ptr<core::dbus::Property<mpris::Player::Properties::PlaybackStatus>> playback_status;
> +            std::shared_ptr<core::dbus::Property<mpris::Player::Properties::TypedPlaybackStatus>> typed_playback_status;
> +            std::shared_ptr<core::dbus::Property<mpris::Player::Properties::LoopStatus>> loop_status;
> +            std::shared_ptr<core::dbus::Property<mpris::Player::Properties::TypedLoopStatus>> typed_loop_status;
> +            std::shared_ptr<core::dbus::Property<mpris::Player::Properties::PlaybackRate>> playback_rate;
> +            std::shared_ptr<core::dbus::Property<mpris::Player::Properties::Shuffle>> is_shuffle;
> +            std::shared_ptr<core::dbus::Property<mpris::Player::Properties::TypedMetaData>> meta_data_for_current_track;
> +            std::shared_ptr<core::dbus::Property<mpris::Player::Properties::Volume>> volume;
> +            std::shared_ptr<core::dbus::Property<mpris::Player::Properties::Position>> position;
> +            std::shared_ptr<core::dbus::Property<mpris::Player::Properties::Duration>> duration;
> +            std::shared_ptr<core::dbus::Property<mpris::Player::Properties::MinimumRate>> minimum_playback_rate;
> +            std::shared_ptr<core::dbus::Property<mpris::Player::Properties::MaximumRate>> maximum_playback_rate;
> +        } properties;
> +
> +        struct
> +        {
> +            typename core::dbus::Signal<mpris::Player::Signals::Seeked, mpris::Player::Signals::Seeked::ArgumentType>::Ptr seeked_to;
> +            typename core::dbus::Signal<mpris::Player::Signals::EndOfStream, mpris::Player::Signals::EndOfStream::ArgumentType>::Ptr end_of_stream;
> +            typename core::dbus::Signal<mpris::Player::Signals::PlaybackStatusChanged, mpris::Player::Signals::PlaybackStatusChanged::ArgumentType>::Ptr playback_status_changed;
> +        } signals;
>      };
>  };
>  }

There are a few methods/signals/properties that exist here but not in the mpris spec (CreateVideoSink, Key, EndOfStream, Duration, etc). Have they got custom impl in QtMultimedia?

> 
> === modified file 'src/core/media/mpris/service.h'
> --- src/core/media/mpris/service.h	2014-04-23 19:00:55 +0000
> +++ src/core/media/mpris/service.h	2014-08-25 14:53:44 +0000
> @@ -19,7 +19,7 @@
>  #ifndef MPRIS_SERVICE_H_
>  #define MPRIS_SERVICE_H_
>  
> -#include "macros.h"
> +#include <core/dbus/macros.h>
>  
>  #include <chrono>
>  #include <string>
> @@ -49,8 +49,8 @@
>          };
>      };
>  
> -    METHOD(CreateSession, Service, std::chrono::seconds(1))
> -    METHOD(PauseOtherSessions, Service, std::chrono::seconds(1))
> +    DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(CreateSession, Service, 1000)
> +    DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(PauseOtherSessions, Service, 1000)
>  };
>  }

Custom qt mpris interface? Doesn't exist in mpris spec.

>  
> 
> === modified file 'src/core/media/mpris/track_list.h'
> --- src/core/media/mpris/track_list.h	2014-02-12 15:53:57 +0000
> +++ src/core/media/mpris/track_list.h	2014-08-25 14:53:44 +0000
> @@ -19,7 +19,7 @@
>  #ifndef MPRIS_TRACK_LIST_H_
>  #define MPRIS_TRACK_LIST_H_
>  
> -#include "macros.h"
> +#include <core/dbus/macros.h>
>  
>  #include <core/dbus/types/any.h>
>  #include <core/dbus/types/object_path.h>
> @@ -43,35 +43,35 @@
>          return s;

static const std::string s{"core.ubuntu.media.Service.Player.TrackList"}
Should it not be:
"org.mpris.MediaPlayer2.TrackList" ?

>      }
>  
> -    METHOD(GetTracksMetadata, TrackList, SECONDS(1))
> -    METHOD(AddTrack, TrackList, SECONDS(1))
> -    METHOD(RemoveTrack, TrackList, SECONDS(1))
> -    METHOD(GoTo, TrackList, SECONDS(1))
> +    DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(GetTracksMetadata, TrackList, 1000)
> +    DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(AddTrack, TrackList, 1000)
> +    DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(RemoveTrack, TrackList, 1000)
> +    DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(GoTo, TrackList, 1000)
>  
>      struct Signals
>      {
> -        SIGNAL
> +        DBUS_CPP_SIGNAL_DEF
>          (
>              TrackListReplaced,
>              TrackList,
>              BOOST_IDENTITY_TYPE((std::tuple<std::vector<dbus::types::ObjectPath>, dbus::types::ObjectPath>))
>          )
>  
> -        SIGNAL
> +        DBUS_CPP_SIGNAL_DEF
>          (
>              TrackAdded,
>              TrackList,
>              BOOST_IDENTITY_TYPE((std::tuple<std::map<std::string, dbus::types::Variant>, dbus::types::ObjectPath>))
>          )
>  
> -        SIGNAL
> +        DBUS_CPP_SIGNAL_DEF
>          (
>              TrackRemoved,
>              TrackList,
>              dbus::types::ObjectPath
>          )
>  
> -        SIGNAL
> +        DBUS_CPP_SIGNAL_DEF
>          (
>              TrackMetadataChanged,
>              TrackList,
> @@ -81,8 +81,8 @@
>  
>      struct Properties
>      {
> -        READABLE_PROPERTY(Tracks, TrackList, std::vector<std::string>)
> -        READABLE_PROPERTY(CanEditTracks, TrackList, bool)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(Tracks, TrackList, std::vector<std::string>)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(CanEditTracks, TrackList, bool)
>      };
>  };
>  }
> 
> === modified file 'src/core/media/player_configuration.h'
> --- src/core/media/player_configuration.h	2014-02-12 15:53:57 +0000
> +++ src/core/media/player_configuration.h	2014-08-25 14:53:44 +0000
> @@ -20,11 +20,13 @@
>  
>  #include <core/media/player.h>
>  
> -#include <core/dbus/types/object_path.h>
> +#include <core/dbus/bus.h>
> +#include <core/dbus/object.h>
>  
>  struct core::ubuntu::media::Player::Configuration
>  {
> -    core::dbus::types::ObjectPath object_path;
> +    std::shared_ptr<core::dbus::Bus> bus;
> +    std::shared_ptr<core::dbus::Object> session;
>  };
>  
>  #endif // CORE_UBUNTU_MEDIA_PLAYER_CLIENT_CONFIGURATION_H_
> 
> === modified file 'src/core/media/player_implementation.cpp'
> --- src/core/media/player_implementation.cpp	2014-08-13 01:41:00 +0000
> +++ src/core/media/player_implementation.cpp	2014-08-25 14:53:44 +0000
> @@ -284,13 +284,14 @@
>  };
>  
>  media::PlayerImplementation::PlayerImplementation(
> -        const dbus::types::ObjectPath& session_path,
> +        const std::shared_ptr<core::dbus::Bus>& bus,
> +        const std::shared_ptr<core::dbus::Object>& session,
>          const std::shared_ptr<Service>& service,
>          PlayerKey key)
> -    : media::PlayerSkeleton(session_path),
> +    : media::PlayerSkeleton(bus, session),
>        d(new Private(
>              this,
> -            session_path,
> +            session->path(),
>              service,
>              key))
>  {
> 
> === modified file 'src/core/media/player_implementation.h'
> --- src/core/media/player_implementation.h	2014-04-25 17:53:00 +0000
> +++ src/core/media/player_implementation.h	2014-08-25 14:53:44 +0000
> @@ -36,7 +36,8 @@
>  {
>  public:
>      PlayerImplementation(
> -            const core::dbus::types::ObjectPath& session_path,
> +            const std::shared_ptr<core::dbus::Bus>& bus,
> +            const std::shared_ptr<core::dbus::Object>& session,
>              const std::shared_ptr<Service>& service,
>              PlayerKey key);
>      ~PlayerImplementation();
> 
> === modified file 'src/core/media/player_skeleton.cpp'
> --- src/core/media/player_skeleton.cpp	2014-08-18 18:05:40 +0000
> +++ src/core/media/player_skeleton.cpp	2014-08-25 14:53:44 +0000
> @@ -36,9 +36,12 @@
>  
>  struct media::PlayerSkeleton::Private
>  {
> -    Private(media::PlayerSkeleton* player, const dbus::types::ObjectPath& session)
> +    Private(media::PlayerSkeleton* player,
> +            const std::shared_ptr<core::dbus::Bus>& bus,
> +            const std::shared_ptr<core::dbus::Object>& session)
>          : impl(player),
> -          object(impl->access_service()->add_object_for_path(session)),
> +          bus(bus),
> +          object(session),
>            apparmor_session(nullptr),
>            properties
>            {
> @@ -50,11 +53,11 @@
>                object->get_property<mpris::Player::Properties::CanGoPrevious>(),
>                object->get_property<mpris::Player::Properties::IsVideoSource>(),
>                object->get_property<mpris::Player::Properties::IsAudioSource>(),
> -              object->get_property<mpris::Player::Properties::PlaybackStatus>(),
> -              object->get_property<mpris::Player::Properties::LoopStatus>(),
> +              object->get_property<mpris::Player::Properties::TypedPlaybackStatus>(),
> +              object->get_property<mpris::Player::Properties::TypedLoopStatus>(),
>                object->get_property<mpris::Player::Properties::PlaybackRate>(),
>                object->get_property<mpris::Player::Properties::Shuffle>(),
> -              object->get_property<mpris::Player::Properties::MetaData>(),
> +              object->get_property<mpris::Player::Properties::TypedMetaData>(),
>                object->get_property<mpris::Player::Properties::Volume>(),
>                object->get_property<mpris::Player::Properties::Position>(),
>                object->get_property<mpris::Player::Properties::Duration>(),
> @@ -74,21 +77,21 @@
>      {
>          impl->next();
>          auto reply = dbus::Message::make_method_return(msg);
> -        impl->access_bus()->send(reply);
> +        bus->send(reply);
>      }
>  
>      void handle_previous(const core::dbus::Message::Ptr& msg)
>      {
>          impl->previous();
>          auto reply = dbus::Message::make_method_return(msg);
> -        impl->access_bus()->send(reply);
> +        bus->send(reply);
>      }
>  
>      void handle_pause(const core::dbus::Message::Ptr& msg)
>      {
>          impl->pause();
>          auto reply = dbus::Message::make_method_return(msg);
> -        impl->access_bus()->send(reply);
> +        bus->send(reply);
>      }
>  
>      void handle_playpause(DBusMessage*)
> @@ -99,14 +102,14 @@
>      {
>          impl->stop();
>          auto reply = dbus::Message::make_method_return(msg);
> -        impl->access_bus()->send(reply);
> +        bus->send(reply);
>      }
>  
>      void handle_play(const core::dbus::Message::Ptr& msg)
>      {
>          impl->play();
>          auto reply = dbus::Message::make_method_return(msg);
> -        impl->access_bus()->send(reply);
> +        bus->send(reply);
>      }
>  
>      void handle_seek(const core::dbus::Message::Ptr& in)
> @@ -116,7 +119,7 @@
>          impl->seek_to(std::chrono::microseconds(ticks));
>  
>          auto reply = dbus::Message::make_method_return(in);
> -        impl->access_bus()->send(reply);
> +        bus->send(reply);
>      }
>  
>      void handle_set_position(const core::dbus::Message::Ptr&)
> @@ -130,7 +133,7 @@
>          impl->create_video_sink(texture_id);
>  
>          auto reply = dbus::Message::make_method_return(in);
> -        impl->access_bus()->send(reply);
> +        bus->send(reply);
>      }
>  
>      std::string get_client_apparmor_context(const core::dbus::Message::Ptr& msg)
> @@ -231,7 +234,7 @@
>      {
>          auto reply = dbus::Message::make_method_return(in);
>          reply->writer() << impl->key();
> -        impl->access_bus()->send(reply);
> +        bus->send(reply);
>      }
>  
>      void handle_open_uri(const core::dbus::Message::Ptr& in)
> @@ -247,12 +250,14 @@
>              reply->writer() << impl->open_uri(uri);
>          else
>              reply->writer() << false;
> -        impl->access_bus()->send(reply);
> +        bus->send(reply);
>      }
>  
>      media::PlayerSkeleton* impl;
> +    dbus::Bus::Ptr bus;
>      dbus::Object::Ptr object;
>      dbus::Object::Ptr apparmor_session;
> +
>      struct
>      {
>          std::shared_ptr<core::dbus::Property<mpris::Player::Properties::CanPlay>> can_play;
> @@ -264,11 +269,11 @@
>          std::shared_ptr<core::dbus::Property<mpris::Player::Properties::IsVideoSource>> is_video_source;
>          std::shared_ptr<core::dbus::Property<mpris::Player::Properties::IsAudioSource>> is_audio_source;
>  
> -        std::shared_ptr<core::dbus::Property<mpris::Player::Properties::PlaybackStatus>> playback_status;
> -        std::shared_ptr<core::dbus::Property<mpris::Player::Properties::LoopStatus>> loop_status;
> +        std::shared_ptr<core::dbus::Property<mpris::Player::Properties::TypedPlaybackStatus>> playback_status;
> +        std::shared_ptr<core::dbus::Property<mpris::Player::Properties::TypedLoopStatus>> loop_status;
>          std::shared_ptr<core::dbus::Property<mpris::Player::Properties::PlaybackRate>> playback_rate;
>          std::shared_ptr<core::dbus::Property<mpris::Player::Properties::Shuffle>> is_shuffle;
> -        std::shared_ptr<core::dbus::Property<mpris::Player::Properties::MetaData>> meta_data_for_current_track;
> +        std::shared_ptr<core::dbus::Property<mpris::Player::Properties::TypedMetaData>> meta_data_for_current_track;
>          std::shared_ptr<core::dbus::Property<mpris::Player::Properties::Volume>> volume;
>          std::shared_ptr<core::dbus::Property<mpris::Player::Properties::Position>> position;
>          std::shared_ptr<core::dbus::Property<mpris::Player::Properties::Duration>> duration;
> @@ -325,9 +330,9 @@
>  };
>  
>  media::PlayerSkeleton::PlayerSkeleton(
> -    const core::dbus::types::ObjectPath& session_path)
> -        : dbus::Skeleton<media::Player>(the_session_bus()),
> -          d(new Private{this, session_path})
> +        const std::shared_ptr<core::dbus::Bus>& bus,
> +        const std::shared_ptr<core::dbus::Object>& session)
> +        : d(new Private{this, bus, session})
>  {
>      d->object->install_method_handler<mpris::Player::Next>(
>          std::bind(&Private::handle_next,
> 
> === modified file 'src/core/media/player_skeleton.h'
> --- src/core/media/player_skeleton.h	2014-04-25 17:53:00 +0000
> +++ src/core/media/player_skeleton.h	2014-08-25 14:53:44 +0000
> @@ -38,7 +38,7 @@
>  {
>  class Service;
>  
> -class PlayerSkeleton : public core::dbus::Skeleton<core::ubuntu::media::Player>
> +class PlayerSkeleton : public core::ubuntu::media::Player
>  {
>    public:
>      ~PlayerSkeleton();
> @@ -71,7 +71,9 @@
>      virtual core::Signal<PlaybackStatus>& playback_status_changed();
>  
>    protected:
> -    PlayerSkeleton(const core::dbus::types::ObjectPath& session_path);
> +    PlayerSkeleton(
> +            const std::shared_ptr<core::dbus::Bus>& bus,
> +            const std::shared_ptr<core::dbus::Object>& session);
>  
>      virtual core::Property<PlaybackStatus>& playback_status();
>      virtual core::Property<bool>& can_play();
> 
> === modified file 'src/core/media/player_stub.cpp'
> --- src/core/media/player_stub.cpp	2014-04-28 21:10:03 +0000
> +++ src/core/media/player_stub.cpp	2014-08-25 14:53:44 +0000
> @@ -45,8 +45,7 @@
>  struct media::PlayerStub::Private
>  {
>      Private(const std::shared_ptr<Service>& parent,
> -            const std::shared_ptr<dbus::Service>& remote,
> -            const dbus::types::ObjectPath& path
> +            const std::shared_ptr<core::dbus::Object>& object
>              ) : parent(parent),
>                  texture_id(0),
>                  igbc_wrapper(nullptr),
> @@ -54,8 +53,7 @@
>                  decoding_session(decoding_service_create_session()),
>                  frame_available_cb(nullptr),
>                  frame_available_context(nullptr),
> -                path(path),
> -                object(remote->object_for_path(path)),
> +                object(object),
>                  properties
>                  {
>                      object->get_property<mpris::Player::Properties::CanPlay>(),
> @@ -66,11 +64,11 @@
>                      object->get_property<mpris::Player::Properties::CanGoPrevious>(),
>                      object->get_property<mpris::Player::Properties::IsVideoSource>(),
>                      object->get_property<mpris::Player::Properties::IsAudioSource>(),
> -                    object->get_property<mpris::Player::Properties::PlaybackStatus>(),
> -                    object->get_property<mpris::Player::Properties::LoopStatus>(),
> +                    object->get_property<mpris::Player::Properties::TypedPlaybackStatus>(),
> +                    object->get_property<mpris::Player::Properties::TypedLoopStatus>(),
>                      object->get_property<mpris::Player::Properties::PlaybackRate>(),
>                      object->get_property<mpris::Player::Properties::Shuffle>(),
> -                    object->get_property<mpris::Player::Properties::MetaData>(),
> +                    object->get_property<mpris::Player::Properties::TypedMetaData>(),
>                      object->get_property<mpris::Player::Properties::Volume>(),
>                      object->get_property<mpris::Player::Properties::Position>(),
>                      object->get_property<mpris::Player::Properties::Duration>(),
> @@ -138,8 +136,6 @@
>      FrameAvailableCb frame_available_cb;
>      void *frame_available_context;
>  
> -    dbus::Bus::Ptr bus;
> -    dbus::types::ObjectPath path;
>      dbus::Object::Ptr object;
>  
>      struct
> @@ -153,11 +149,11 @@
>          std::shared_ptr<core::dbus::Property<mpris::Player::Properties::IsVideoSource>> is_video_source;
>          std::shared_ptr<core::dbus::Property<mpris::Player::Properties::IsAudioSource>> is_audio_source;
>  
> -        std::shared_ptr<core::dbus::Property<mpris::Player::Properties::PlaybackStatus>> playback_status;
> -        std::shared_ptr<core::dbus::Property<mpris::Player::Properties::LoopStatus>> loop_status;
> +        std::shared_ptr<core::dbus::Property<mpris::Player::Properties::TypedPlaybackStatus>> playback_status;
> +        std::shared_ptr<core::dbus::Property<mpris::Player::Properties::TypedLoopStatus>> loop_status;
>          std::shared_ptr<core::dbus::Property<mpris::Player::Properties::PlaybackRate>> playback_rate;
>          std::shared_ptr<core::dbus::Property<mpris::Player::Properties::Shuffle>> is_shuffle;
> -        std::shared_ptr<core::dbus::Property<mpris::Player::Properties::MetaData>> meta_data_for_current_track;
> +        std::shared_ptr<core::dbus::Property<mpris::Player::Properties::TypedMetaData>> meta_data_for_current_track;
>          std::shared_ptr<core::dbus::Property<mpris::Player::Properties::Volume>> volume;
>          std::shared_ptr<core::dbus::Property<mpris::Player::Properties::Position>> position;
>          std::shared_ptr<core::dbus::Property<mpris::Player::Properties::Duration>> duration;
> @@ -230,9 +226,8 @@
>  
>  media::PlayerStub::PlayerStub(
>      const std::shared_ptr<Service>& parent,
> -    const dbus::types::ObjectPath& object_path)
> -        : dbus::Stub<Player>(the_session_bus()),
> -          d(new Private{parent, access_service(), object_path})
> +    const std::shared_ptr<core::dbus::Object>& object)
> +        : d(new Private{parent, object})
>  {
>      auto bus = the_session_bus();
>      worker = std::move(std::thread([bus]()
> @@ -256,7 +251,7 @@
>      {
>          d->track_list = std::make_shared<media::TrackListStub>(
>                      shared_from_this(),
> -                    dbus::types::ObjectPath(d->path.as_string() + "/TrackList"));
> +                    dbus::types::ObjectPath(d->object->path().as_string() + "/TrackList"));
>      }
>      return d->track_list;
>  }
> 
> === modified file 'src/core/media/player_stub.h'
> --- src/core/media/player_stub.h	2014-04-25 17:53:00 +0000
> +++ src/core/media/player_stub.h	2014-08-25 14:53:44 +0000
> @@ -33,12 +33,12 @@
>  {
>  class Service;
>  
> -class PlayerStub : public core::dbus::Stub<Player>
> +class PlayerStub : public Player
>  {
>    public:
>      explicit PlayerStub(
>          const std::shared_ptr<Service>& parent,
> -        const core::dbus::types::ObjectPath& object);
> +        const std::shared_ptr<core::dbus::Object>& object);
>  
>      ~PlayerStub();
>  
> 
> === modified file 'src/core/media/service_implementation.cpp'
> --- src/core/media/service_implementation.cpp	2014-04-23 19:00:55 +0000
> +++ src/core/media/service_implementation.cpp	2014-08-25 14:53:44 +0000
> @@ -88,7 +88,7 @@
>          const media::Player::Configuration& conf)
>  {
>      std::shared_ptr<media::Player> player = std::make_shared<media::PlayerImplementation>(
> -            conf.object_path, shared_from_this(), d->key());
> +            conf.bus, conf.session, shared_from_this(), d->key());
>      d->track_player(player);
>      return player;
>  }
> 
> === modified file 'src/core/media/service_skeleton.cpp'
> --- src/core/media/service_skeleton.cpp	2014-04-23 19:00:55 +0000
> +++ src/core/media/service_skeleton.cpp	2014-08-25 14:53:44 +0000
> @@ -64,7 +64,11 @@
>          ss << "/core/ubuntu/media/Service/sessions/" << session_counter++;
>  
>          dbus::types::ObjectPath op{ss.str()};
> -        media::Player::Configuration config{op};
> +        media::Player::Configuration config
> +        {
> +            impl->access_bus(),
> +            impl->access_service()->add_object_for_path(op)
> +        };
>  
>          try
>          {
> @@ -104,7 +108,6 @@
>  
>      media::ServiceSkeleton* impl;
>      dbus::Object::Ptr object;
> -
>  };
>  
>  media::ServiceSkeleton::ServiceSkeleton()
> 
> === modified file 'src/core/media/service_stub.cpp'
> --- src/core/media/service_stub.cpp	2014-04-23 19:00:55 +0000
> +++ src/core/media/service_stub.cpp	2014-08-25 14:53:44 +0000
> @@ -53,7 +53,11 @@
>      if (op.is_error())
>          throw std::runtime_error("Problem creating session: " + op.error());
>  
> -    return std::shared_ptr<media::Player>(new media::PlayerStub(shared_from_this(), op.value()));
> +    return std::shared_ptr<media::Player>(new media::PlayerStub
> +    {
> +        shared_from_this(),
> +        access_service()->object_for_path(op.value())
> +    });
>  }
>  
>  void media::ServiceStub::pause_other_sessions(media::Player::PlayerKey key)
> 


-- 
https://code.launchpad.net/~thomas-voss/media-hub/complete-mpris-spec/+merge/232057
Your team Ubuntu Phablet Team is subscribed to branch lp:media-hub.



More information about the Ubuntu-reviews mailing list