Merge lp:~phablet-team/media-hub/mpris-take-2 into lp:media-hub/stable

[Alfonso Sanchez-Beato]

Re-enable MPRIS player controls and improve background playlists

Re-enable MPRIS player controls and improve background playlists

[Alfonso Sanchez-Beato (alfonsosanchezbeato)]

Bugs created to track the "TODO" comments:

https://bugs.launchpad.net/ubuntu/+source/media-hub/+bug/1498962
https://bugs.launchpad.net/ubuntu/+source/media-hub/+bug/1498961

Re: the trace in reset_current_player() I prefer to keep it so it is symmetrical with the one in set_current_player(). These traces are not printed very frequently and are quite useful for debugging as they happen in kind of critical moments.

[Simon Fels (morphis)]

review: Approve

[Manuel de la Peña (mandel)]

review: Approve

[lp:~phablet-team/media-hub/mpris-take-2]

183. By Alfonso Sanchez-Beato on 2015-09-28

[ Simon Fels ]
* Prevent us from leaking file descriptors and memory due to not releasing
  gstreamer elements correctly.
[ CI Train Bot ]
* No-change rebuild.

184. By Alfonso Sanchez-Beato on 2015-09-28

Fix changelog

[Jim Hodapp (jhodapp)]

A few comments inline below.

review: Needs Fixing (code)

[Jim Hodapp (jhodapp)]

One more change below

review: Needs Fixing

[lp:~phablet-team/media-hub/mpris-take-2]

185. By Alfonso Sanchez-Beato on 2015-09-28

Addess review comments

186. By Alfonso Sanchez-Beato on 2015-09-29

Update CanPlay and CanPause depending on whether there are items on the
playlist or not.

[Jim Hodapp (jhodapp)]

Looks good

review: Approve

[lp:~phablet-team/media-hub/mpris-take-2]

187. By Jim Hodapp on 2015-10-06

Merged with stable

188. By Jim Hodapp on 2015-10-14

Make sure that other sessions pause when the MPRIS play method is called.

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2015-04-10 16:21:16 +0000
+++ CMakeLists.txt	2015-10-14 20:54:59 +0000
@@ -3,7 +3,7 @@
 project(ubuntu-media-hub)
 
 set(UBUNTU_MEDIA_HUB_VERSION_MAJOR 3)
-set(UBUNTU_MEDIA_HUB_VERSION_MINOR 1)
+set(UBUNTU_MEDIA_HUB_VERSION_MINOR 2)
 set(UBUNTU_MEDIA_HUB_VERSION_PATCH 0)
 
 set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra -fPIC -pthread")

=== modified file 'README'
--- README	2014-08-08 14:36:29 +0000
+++ README	2015-10-14 20:54:59 +0000
@@ -1,3 +1,12 @@
+Coding Convention:
+------------------
+
+NOTE: the media-hub code did not start out with the following coding convention, but it is being introduced to try and
+converge on a standard from this point forward:
+
+https://google-styleguide.googlecode.com/svn/trunk/cppguide.html
+
+
 To Build:
 ---------
 

=== modified file 'debian/changelog'
--- debian/changelog	2015-10-02 14:23:08 +0000
+++ debian/changelog	2015-10-14 20:54:59 +0000
@@ -1,3 +1,13 @@
+media-hub (3.2.0+15.04.20150928-0ubuntu1) UNRELEASED; urgency=medium
+
+  * Make sure the correct player is set as the current player controled by
+    MPRIS.
+  * Improved the can_go_next() and can_go_previous() logic to always return
+    true if the loop_status is currently set to loop over the entire
+    playlist.
+
+ -- Jim Hodapp <[e-mail]>  Tue, 08 Sep 2015 17:18:15 -0400
+
 media-hub (3.1.0+15.04.20151002-0ubuntu1) vivid; urgency=medium
 
   * Avoid getting additional references for audio_sink, which were not

=== modified file 'include/core/media/service.h'
--- include/core/media/service.h	2015-04-14 02:43:56 +0000
+++ include/core/media/service.h	2015-10-14 20:54:59 +0000
@@ -60,6 +60,9 @@
     /** @brief Resumes a fixed-name session directly by player key. */
     virtual std::shared_ptr<Player> resume_session(Player::PlayerKey) = 0;
 
+    /** @brief Sets the current player that the MPRIS interface will control */
+    virtual void set_current_player(Player::PlayerKey) = 0;
+
     /** @brief Pauses sessions other than the supplied one. */
     virtual void pause_other_sessions(Player::PlayerKey) = 0;
 

=== modified file 'src/core/media/mpris/player.h'
--- src/core/media/mpris/player.h	2015-04-06 20:59:14 +0000
+++ src/core/media/mpris/player.h	2015-10-14 20:54:59 +0000
@@ -192,12 +192,12 @@
             // Default values for properties
             struct Defaults
             {
-                Properties::CanPlay::ValueType can_play{true};
-                Properties::CanPause::ValueType can_pause{true};
+                Properties::CanPlay::ValueType can_play{false};
+                Properties::CanPause::ValueType can_pause{false};
                 Properties::CanSeek::ValueType can_seek{true};
                 Properties::CanControl::ValueType can_control{true};
-                Properties::CanGoNext::ValueType can_go_next{true};
-                Properties::CanGoPrevious::ValueType can_go_previous{true};
+                Properties::CanGoNext::ValueType can_go_next{false};
+                Properties::CanGoPrevious::ValueType can_go_previous{false};
                 Properties::IsVideoSource::ValueType is_video_source{false};
                 Properties::IsAudioSource::ValueType is_audio_source{true};
                 Properties::PlaybackStatus::ValueType playback_status{PlaybackStatus::stopped};
@@ -307,6 +307,26 @@
             {
                 on_property_value_changed<Properties::Shuffle>(shuffle);
             });
+
+            properties.can_play->changed().connect([this](bool can_play)
+            {
+                on_property_value_changed<Properties::CanPlay>(can_play);
+            });
+
+            properties.can_pause->changed().connect([this](bool can_pause)
+            {
+                on_property_value_changed<Properties::CanPause>(can_pause);
+            });
+
+            properties.can_go_next->changed().connect([this](bool can_go_next)
+            {
+                on_property_value_changed<Properties::CanGoNext>(can_go_next);
+            });
+
+            properties.can_go_previous->changed().connect([this](bool can_go_previous)
+            {
+                on_property_value_changed<Properties::CanGoPrevious>(can_go_previous);
+            });
         }
 
         template<typename Property>

=== modified file 'src/core/media/mpris/service.h'
--- src/core/media/mpris/service.h	2015-04-10 16:13:55 +0000
+++ src/core/media/mpris/service.h	2015-10-14 20:54:59 +0000
@@ -115,6 +115,7 @@
     DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(DestroySession, Service, 1000)
     DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(CreateFixedSession, Service, 1000)
     DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(ResumeSession, Service, 1000)
+    DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(SetCurrentPlayer, Service, 1000)
     DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(PauseOtherSessions, Service, 1000)
 };
 }

=== modified file 'src/core/media/player_implementation.cpp'
--- src/core/media/player_implementation.cpp	2015-07-27 22:15:33 +0000
+++ src/core/media/player_implementation.cpp	2015-10-14 20:54:59 +0000
@@ -301,6 +301,27 @@
         }
     }
 
+    void update_mpris_properties(void)
+    {
+        bool has_previous =    track_list->has_previous()
+                            or parent->Parent::loop_status() != Player::LoopStatus::none;
+        bool has_next =    track_list->has_next()
+                        or parent->Parent::loop_status() != Player::LoopStatus::none;
+        auto n_tracks = track_list->tracks()->size();
+        bool has_tracks = (n_tracks > 0) ? true : false;
+
+        std::cout << "Updating MPRIS TrackList properties"
+                  << "; Tracks: " << n_tracks
+                  << ", has_previous: " << has_previous
+                  << ", has_next: " << has_next << std::endl;
+
+        // Update properties
+        parent->can_play().set(has_tracks);
+        parent->can_pause().set(has_tracks);
+        parent->can_go_previous().set(has_previous);
+        parent->can_go_next().set(has_next);
+    }
+
     // Our link back to our parent.
     media::PlayerImplementation<Parent>* parent;
     // We just store the parameters passed on construction.
@@ -327,8 +348,8 @@
       d{std::make_shared<Private>(this, config)}
 {
     // Initialize default values for Player interface properties
-    Parent::can_play().set(true);
-    Parent::can_pause().set(true);
+    Parent::can_play().set(false);
+    Parent::can_pause().set(false);
     Parent::can_seek().set(true);
     Parent::can_go_previous().set(false);
     Parent::can_go_next().set(false);
@@ -376,13 +397,15 @@
 
     std::function<bool()> can_go_next_getter = [this]()
     {
-        return d->track_list->has_next();
+        // If LoopStatus == playlist, then there is always a next track
+        return d->track_list->has_next() or Parent::loop_status() != Player::LoopStatus::none;
     };
     Parent::can_go_next().install(can_go_next_getter);
 
     std::function<bool()> can_go_previous_getter = [this]()
     {
-        return d->track_list->has_previous();
+        // If LoopStatus == playlist, then there is always a next previous
+        return d->track_list->has_previous() or Parent::loop_status() != Player::LoopStatus::none;
     };
     Parent::can_go_previous().install(can_go_previous_getter);
 
@@ -520,7 +543,26 @@
         std::cout << "** Track was added, handling in PlayerImplementation" << std::endl;
         if (d->track_list->tracks()->size() == 1)
             d->open_first_track_from_tracklist(id);
-    });
+
+        d->update_mpris_properties();
+    });
+
+    d->track_list->on_track_removed().connect([this](const media::Track::Id&)
+    {
+        d->update_mpris_properties();
+    });
+
+    d->track_list->on_track_changed().connect([this](const media::Track::Id&)
+    {
+        d->update_mpris_properties();
+    });
+
+    d->track_list->on_track_list_replaced().connect(
+        [this](const media::TrackList::ContainerTrackIdTuple&)
+        {
+            d->update_mpris_properties();
+        }
+    );
 
     // Everything is setup, we now subscribe to death notifications.
     std::weak_ptr<Private> wp{d};

=== modified file 'src/core/media/service_implementation.cpp'
--- src/core/media/service_implementation.cpp	2015-04-17 16:44:30 +0000
+++ src/core/media/service_implementation.cpp	2015-10-14 20:54:59 +0000
@@ -246,8 +246,15 @@
   return std::shared_ptr<media::Player>();
 }
 
+void media::ServiceImplementation::set_current_player(Player::PlayerKey key)
+{
+  // no impl
+}
+
 void media::ServiceImplementation::pause_other_sessions(media::Player::PlayerKey key)
 {
+    std::cout << __PRETTY_FUNCTION__ << std::endl;
+
     if (not d->configuration.player_store->has_player_for_key(key))
     {
         cerr << "Could not find Player by key: " << key << endl;

=== modified file 'src/core/media/service_implementation.h'
--- src/core/media/service_implementation.h	2015-04-16 15:19:35 +0000
+++ src/core/media/service_implementation.h	2015-10-14 20:54:59 +0000
@@ -49,6 +49,7 @@
     void destroy_session(const std::string&, const Player::Configuration&);
     std::shared_ptr<Player> create_fixed_session(const std::string& name, const Player::Configuration&);
     std::shared_ptr<Player> resume_session(Player::PlayerKey key);
+    void set_current_player(Player::PlayerKey key);
     void pause_other_sessions(Player::PlayerKey key);
 
 private:

=== modified file 'src/core/media/service_skeleton.cpp'
--- src/core/media/service_skeleton.cpp	2015-08-11 16:10:14 +0000
+++ src/core/media/service_skeleton.cpp	2015-10-14 20:54:59 +0000
@@ -59,7 +59,7 @@
           object(impl->access_service()->add_object_for_path(
                      dbus::traits::Service<media::Service>::object_path())),
           configuration(config),
-          exported(impl->access_bus(), config.cover_art_resolver)
+          exported(impl->access_bus(), config.cover_art_resolver, impl)
     {
         object->install_method_handler<mpris::Service::CreateSession>(
                     std::bind(
@@ -91,6 +91,11 @@
                         &Private::handle_resume_session,
                         this,
                         std::placeholders::_1));
+        object->install_method_handler<mpris::Service::SetCurrentPlayer>(
+                    std::bind(
+                        &Private::handle_set_current_player,
+                        this,
+                        std::placeholders::_1));
         object->install_method_handler<mpris::Service::PauseOtherSessions>(
                     std::bind(
                         &Private::handle_pause_other_sessions,
@@ -102,7 +107,7 @@
     {
         static unsigned int session_counter = 0;
 
-        unsigned int current_session = session_counter++;
+        const unsigned int current_session = session_counter++;
         boost::uuids::uuid uuid = gen();
 
         std::stringstream ss;
@@ -134,7 +139,8 @@
             configuration.player_store->add_player_for_key(key, impl->create_session(config));
             uuid_player_map.insert(std::make_pair(uuid, key));
 
-            request_context_resolver->resolve_context_for_dbus_name_async(msg->sender(), [this, key, msg](const media::apparmor::ubuntu::Context& context)
+            request_context_resolver->resolve_context_for_dbus_name_async(msg->sender(),
+                    [this, key, msg](const media::apparmor::ubuntu::Context& context)
             {
                 fprintf(stderr, "%s():%d -- app_name='%s', attached\n", __func__, __LINE__, context.str().c_str());
                 player_owner_map.insert(std::make_pair(key, std::make_tuple(context.str(), true, msg->sender())));
@@ -213,7 +219,8 @@
                 ss << "/core/ubuntu/media/Service/sessions/" << key;
                 dbus::types::ObjectPath op{ss.str()};
 
-                request_context_resolver->resolve_context_for_dbus_name_async(msg->sender(), [this, msg, key, op](const media::apparmor::ubuntu::Context& context)
+                request_context_resolver->resolve_context_for_dbus_name_async(msg->sender(),
+                        [this, msg, key, op](const media::apparmor::ubuntu::Context& context)
                 {
                     auto info = player_owner_map.at(key);
                     fprintf(stderr, "%s():%d -- reattach app_name='%s', info='%s', '%s'\n", __func__, __LINE__, context.str().c_str(), std::get<0>(info).c_str(), std::get<2>(info).c_str());
@@ -224,6 +231,12 @@
                         // Signal player reconnection
                         auto player = configuration.player_store->player_for_key(key);
                         player->reconnect();
+                        // We only care to allow the MPRIS controls to apply to multimedia player (i.e. audio, video)
+                        if (player->audio_stream_role() == media::Player::AudioStreamRole::multimedia)
+                        {
+                            std::cout << "Setting current_player" << std::endl;
+                            exported.set_current_player(player);
+                        }
 
                         auto reply = dbus::Message::make_method_return(msg);
                         reply->writer() << op;
@@ -280,7 +293,8 @@
                 // the session is no longer usable.
                 uuid_player_map.erase(uuid);
 
-                request_context_resolver->resolve_context_for_dbus_name_async(msg->sender(), [this, msg, key](const media::apparmor::ubuntu::Context& context)
+                request_context_resolver->resolve_context_for_dbus_name_async(msg->sender(),
+                        [this, msg, key](const media::apparmor::ubuntu::Context& context)
                 {
                     auto info = player_owner_map.at(key);
                     fprintf(stderr, "%s():%d -- Destroying app_name='%s', info='%s', '%s'\n", __func__, __LINE__, context.str().c_str(), std::get<0>(info).c_str(), std::get<2>(info).c_str());
@@ -424,9 +438,18 @@
         }
     }
 
+    void handle_set_current_player(const core::dbus::Message::Ptr& msg)
+    {
+        Player::PlayerKey key;
+        msg->reader() >> key;
+        impl->set_current_player(key);
+
+        auto reply = dbus::Message::make_method_return(msg);
+        impl->access_bus()->send(reply);
+    }
+
     void handle_pause_other_sessions(const core::dbus::Message::Ptr& msg)
     {
-        std::cout << __PRETTY_FUNCTION__ << std::endl;
         Player::PlayerKey key;
         msg->reader() >> key;
         impl->pause_other_sessions(key);
@@ -469,45 +492,33 @@
         {
             mpris::Player::Skeleton::Configuration::Defaults defaults;
 
-            // Disabled as track list is not fully implemented yet.
-            defaults.can_go_next = false;
-            // Disabled as track list is not fully implemented yet.
-            defaults.can_go_previous = false;
-
             return defaults;
         }
 
-        static std::string service_name()
-        {
-            static const bool export_to_indicator_sound_via_mpris
-            {
-                core::posix::this_process::env::get("UBUNTU_MEDIA_HUB_EXPORT_TO_INDICATOR_VIA_MPRIS", "0") == "1"
-            };
-
-            return export_to_indicator_sound_via_mpris ? "org.mpris.MediaPlayer2.MediaHub" :
-                                                         "hidden.org.mpris.MediaPlayer2.MediaHub";
-        }
-
-        explicit Exported(const dbus::Bus::Ptr& bus, const media::CoverArtResolver& cover_art_resolver)
+        explicit Exported(const dbus::Bus::Ptr& bus, const media::CoverArtResolver& cover_art_resolver,
+                media::ServiceSkeleton* impl)
             : bus{bus},
-              service{dbus::Service::add_service(bus, service_name())},
+              /* Export MediaHub service interface on dbus */
+              service{dbus::Service::add_service(bus, "org.mpris.MediaPlayer2.MediaHub")},
               object{service->add_object_for_path(dbus::types::ObjectPath{"/org/mpris/MediaPlayer2"})},
               media_player{mpris::MediaPlayer2::Skeleton::Configuration{bus, object, media_player_defaults()}},
               player{mpris::Player::Skeleton::Configuration{bus, object, player_defaults()}},
               playlists{mpris::Playlists::Skeleton::Configuration{bus, object, mpris::Playlists::Skeleton::Configuration::Defaults{}}},
-              cover_art_resolver{cover_art_resolver}
+              cover_art_resolver{cover_art_resolver},
+              impl{impl}
         {
             object->install_method_handler<core::dbus::interfaces::Properties::GetAll>([this](const core::dbus::Message::Ptr& msg)
             {
                 // Extract the interface
-                std::string itf; msg->reader() >> itf;
+                std::string interface;
+                msg->reader() >> interface;
                 core::dbus::Message::Ptr reply = core::dbus::Message::make_method_return(msg);
 
-                if (itf == mpris::Player::name())
+                if (interface == mpris::Player::name())
                     reply->writer() << player.get_all_properties();
-                else if (itf == mpris::MediaPlayer2::name())
+                else if (interface == mpris::MediaPlayer2::name())
                     reply->writer() << media_player.get_all_properties();
-                else if (itf == mpris::Playlists::name())
+                else if (interface == mpris::Playlists::name())
                     reply->writer() << playlists.get_all_properties();
 
                 Exported::bus->send(reply);
@@ -516,9 +527,9 @@
             // Setup method handlers for mpris::Player methods.
             auto next = [this](const core::dbus::Message::Ptr& msg)
             {
-                auto sp = current_player.lock();
+                const auto sp = current_player.lock();
 
-                if (sp)
+                if (is_multimedia_role())
                     sp->next();
 
                 Exported::bus->send(core::dbus::Message::make_method_return(msg));
@@ -527,9 +538,9 @@
 
             auto previous = [this](const core::dbus::Message::Ptr& msg)
             {
-                auto sp = current_player.lock();
+                const auto sp = current_player.lock();
 
-                if (sp)
+                if (is_multimedia_role())
                     sp->previous();
 
                 Exported::bus->send(core::dbus::Message::make_method_return(msg));
@@ -538,9 +549,9 @@
 
             auto pause = [this](const core::dbus::Message::Ptr& msg)
             {
-                auto sp = current_player.lock();
+                const auto sp = current_player.lock();
 
-                if (sp)
+                if (is_multimedia_role() and sp->can_pause())
                     sp->pause();
 
                 Exported::bus->send(core::dbus::Message::make_method_return(msg));
@@ -549,36 +560,52 @@
 
             auto stop = [this](const core::dbus::Message::Ptr& msg)
             {
-                auto sp = current_player.lock();
+                const auto sp = current_player.lock();
 
-                if (sp)
+                if (is_multimedia_role())
                     sp->stop();
 
                 Exported::bus->send(core::dbus::Message::make_method_return(msg));
             };
             object->install_method_handler<mpris::Player::Stop>(stop);
 
-            auto play = [this](const core::dbus::Message::Ptr& msg)
+            auto play = [this, impl](const core::dbus::Message::Ptr& msg)
             {
-                auto sp = current_player.lock();
-
-                if (sp)
+                const auto sp = current_player.lock();
+
+                if (is_multimedia_role() and sp->can_play())
+                {
+                    // Make sure other player sessions that are already playing
+                    // are paused before triggering new player (sp) to play
+                    if (impl)
+                        impl->pause_other_sessions(sp->key());
+
                     sp->play();
+                }
 
                 Exported::bus->send(core::dbus::Message::make_method_return(msg));
             };
             object->install_method_handler<mpris::Player::Play>(play);
 
-            auto play_pause = [this](const core::dbus::Message::Ptr& msg)
+            auto play_pause = [this, impl](const core::dbus::Message::Ptr& msg)
             {
-                auto sp = current_player.lock();
+                const auto sp = current_player.lock();
 
-                if (sp)
+                if (is_multimedia_role())
                 {
-                    if (sp->playback_status() == media::Player::PlaybackStatus::playing)
+                    if (sp->playback_status() == media::Player::PlaybackStatus::playing
+                            and sp->can_pause())
                         sp->pause();
-                    else if (sp->playback_status() != media::Player::PlaybackStatus::null)
+                    else if (sp->playback_status() != media::Player::PlaybackStatus::null
+                                and sp->can_play())
+                    {
+                        // Make sure other player sessions that are already playing
+                        // are paused before triggering new player (sp) to play
+                        if (impl)
+                            impl->pause_other_sessions(sp->key());
+
                         sp->play();
+                    }
                 }
 
                 Exported::bus->send(core::dbus::Message::make_method_return(msg));
@@ -586,15 +613,21 @@
             object->install_method_handler<mpris::Player::PlayPause>(play_pause);
         }
 
+        inline bool is_multimedia_role()
+        {
+            const auto sp = current_player.lock();
+
+            return (sp ? sp->audio_stream_role() == media::Player::AudioStreamRole::multimedia : false);
+        }
+
         void set_current_player(const std::shared_ptr<media::Player>& cp)
         {
-            unset_current_player();
-
+            std::cout << "*** " << __PRETTY_FUNCTION__ << std::endl;
             // We will not keep the object alive.
             current_player = cp;
 
             // And announce that we can be controlled again.
-            player.properties.can_control->set(false);
+            player.properties.can_control->set(true);
 
             // We wire up player state changes
             connections.seeked_to = cp->seeked_to().connect([this](std::uint64_t position)
@@ -612,68 +645,103 @@
                 player.properties.position->set(position);
             });
 
-            connections.playback_status_changed = cp->playback_status().changed().connect([this](core::ubuntu::media::Player::PlaybackStatus status)
+            connections.playback_status_changed = cp->playback_status().changed().connect(
+                [this](core::ubuntu::media::Player::PlaybackStatus status)
             {
                 player.properties.playback_status->set(mpris::Player::PlaybackStatus::from(status));
             });
 
-            connections.loop_status_changed = cp->loop_status().changed().connect([this](core::ubuntu::media::Player::LoopStatus status)
+            connections.loop_status_changed = cp->loop_status().changed().connect(
+                [this](core::ubuntu::media::Player::LoopStatus status)
             {
                 player.properties.loop_status->set(mpris::Player::LoopStatus::from(status));
             });
 
-            connections.meta_data_changed = cp->meta_data_for_current_track().changed().connect([this](const core::ubuntu::media::Track::MetaData& md)
-            {
-                mpris::Player::Dictionary dict;
-
-                bool has_title = md.count(xesam::Title::name) > 0;
-                bool has_album_name = md.count(xesam::Album::name) > 0;
-                bool has_artist_name = md.count(xesam::Artist::name) > 0;
-
-                if (has_title)
-                    dict[xesam::Title::name] = dbus::types::Variant::encode(md.get(xesam::Title::name));
-                if (has_album_name)
-                    dict[xesam::Album::name] = dbus::types::Variant::encode(md.get(xesam::Album::name));
-                if (has_artist_name)
-                    dict[xesam::Artist::name] = dbus::types::Variant::encode(md.get(xesam::Artist::name));
-
-                dict[mpris::metadata::ArtUrl::name] = dbus::types::Variant::encode(
-                            cover_art_resolver(
-                                has_title ? md.get(xesam::Title::name) : "",
-                                has_album_name ? md.get(xesam::Album::name) : "",
-                                has_artist_name ? md.get(xesam::Artist::name) : ""));
-
-                mpris::Player::Dictionary wrap;
-                wrap[mpris::Player::Properties::Metadata::name()] = dbus::types::Variant::encode(dict);
-
-                player.signals.properties_changed->emit(
-                            std::make_tuple(
-                                dbus::traits::Service<mpris::Player::Properties::Metadata::Interface>::interface_name(),
-                                wrap,
-                                std::vector<std::string>()));
-            });
+            connections.can_play_changed = cp->can_play().changed().connect(
+                [this](bool can_play)
+            {
+                player.properties.can_play->set(can_play);
+            });
+
+            connections.can_pause_changed = cp->can_pause().changed().connect(
+                [this](bool can_pause)
+            {
+                player.properties.can_pause->set(can_pause);
+            });
+
+            connections.can_go_previous_changed = cp->can_go_previous().changed().connect(
+                [this](bool can_go_previous)
+            {
+                player.properties.can_go_previous->set(can_go_previous);
+            });
+
+            connections.can_go_next_changed = cp->can_go_next().changed().connect(
+                [this](bool can_go_next)
+            {
+                player.properties.can_go_next->set(can_go_next);
+            });
+
+            // Sync property values between session and player mpris::Player instances
+            // TODO Getters from media::Player actually return values from a
+            // mpris::Player::Skeleton instance different from "player". Each of them use
+            // different DBus object paths, /core/ubuntu/media/Service/sessions/<n>
+            // and /org/mpris/MediaPlayer2 (this is the one enforced by the MPRIS spec).
+            // Discuss why this is needed with tvoss.
+            player.properties.duration->set(cp->duration().get());
+            player.properties.position->set(cp->position().get());
+            player.properties.playback_status->set(mpris::Player::PlaybackStatus::from(
+                                                       cp->playback_status().get()));
+            player.properties.loop_status->set(mpris::Player::LoopStatus::from(
+                                                   cp->loop_status().get()));
+            player.properties.can_play->set(cp->can_play().get());
+            player.properties.can_pause->set(cp->can_pause().get());
+            player.properties.can_go_previous->set(cp->can_go_previous().get());
+            player.properties.can_go_next->set(cp->can_go_next().get());
+
+#if 0
+            // TODO cover_art_resolver() is not implemented yet
+            connections.meta_data_changed = cp->meta_data_for_current_track().changed().connect(
+                [this](const core::ubuntu::media::Track::MetaData& md)
+                {
+                    mpris::Player::Dictionary dict;
+
+                    bool has_title = md.count(xesam::Title::name) > 0;
+                    bool has_album_name = md.count(xesam::Album::name) > 0;
+                    bool has_artist_name = md.count(xesam::Artist::name) > 0;
+
+                    if (has_title)
+                        dict[xesam::Title::name] = dbus::types::Variant::encode(md.get(xesam::Title::name));
+                    if (has_album_name)
+                        dict[xesam::Album::name] = dbus::types::Variant::encode(md.get(xesam::Album::name));
+                    if (has_artist_name)
+                        dict[xesam::Artist::name] = dbus::types::Variant::encode(md.get(xesam::Artist::name));
+
+                    dict[mpris::metadata::ArtUrl::name] = dbus::types::Variant::encode(
+                        cover_art_resolver(
+                            has_title ? md.get(xesam::Title::name) : "",
+                            has_album_name ? md.get(xesam::Album::name) : "",
+                            has_artist_name ? md.get(xesam::Artist::name) : ""));
+
+                    mpris::Player::Dictionary wrap;
+                    wrap[mpris::Player::Properties::Metadata::name()] = dbus::types::Variant::encode(dict);
+
+                    player.signals.properties_changed->emit(
+                        std::make_tuple(
+                            dbus::traits::Service<
+                            mpris::Player::Properties::Metadata::Interface>
+                            ::interface_name(),
+                            wrap,
+                            std::vector<std::string>()));
+                });
+#endif
         }
 
-        void unset_current_player()
+        void reset_current_player()
         {
+            std::cout << __PRETTY_FUNCTION__ << std::endl;
+            // And announce that we can no longer be controlled.
+            player.properties.can_control->set(false);
             current_player.reset();
-
-            // We disconnect all previous event connections.
-            connections.seeked_to.disconnect();
-            connections.duration_changed.disconnect();
-            connections.position_changed.disconnect();
-            connections.playback_status_changed.disconnect();
-            connections.loop_status_changed.disconnect();
-            connections.meta_data_changed.disconnect();
-
-            // And announce that we cannot be controlled anymore.
-            player.properties.can_control->set(false);
-        }
-
-        void unset_if_current(const std::shared_ptr<media::Player>& cp)
-        {
-            if (cp == current_player.lock())
-                unset_current_player();
         }
 
         dbus::Bus::Ptr bus;
@@ -688,6 +756,9 @@
         media::CoverArtResolver cover_art_resolver;
         // The actual player instance.
         std::weak_ptr<media::Player> current_player;
+
+        media::ServiceSkeleton* impl;
+
         // We track event connections.
         struct
         {
@@ -711,6 +782,22 @@
             {
                 the_empty_signal.connect([](){})
             };
+            core::Connection can_play_changed
+            {
+                the_empty_signal.connect([](){})
+            };
+            core::Connection can_pause_changed
+            {
+                the_empty_signal.connect([](){})
+            };
+            core::Connection can_go_previous_changed
+            {
+                the_empty_signal.connect([](){})
+            };
+            core::Connection can_go_next_changed
+            {
+                the_empty_signal.connect([](){})
+            };
             core::Connection meta_data_changed
             {
                 the_empty_signal.connect([](){})
@@ -759,6 +846,14 @@
     return d->configuration.impl->resume_session(key);
 }
 
+void media::ServiceSkeleton::set_current_player(media::Player::PlayerKey key)
+{
+    const auto player = d->configuration.player_store->player_for_key(key);
+    // We only care to allow the MPRIS controls to apply to multimedia player (i.e. audio, video)
+    if (player->audio_stream_role() == media::Player::AudioStreamRole::multimedia)
+        d->exported.set_current_player(player);
+}
+
 void media::ServiceSkeleton::pause_other_sessions(media::Player::PlayerKey key)
 {
     d->configuration.impl->pause_other_sessions(key);

=== modified file 'src/core/media/service_skeleton.h'
--- src/core/media/service_skeleton.h	2015-04-16 15:19:35 +0000
+++ src/core/media/service_skeleton.h	2015-10-14 20:54:59 +0000
@@ -59,6 +59,7 @@
     void destroy_session(const std::string&, const media::Player::Configuration&);
     std::shared_ptr<Player> create_fixed_session(const std::string& name, const Player::Configuration&);
     std::shared_ptr<Player> resume_session(Player::PlayerKey);
+    void set_current_player(Player::PlayerKey key);
     void pause_other_sessions(Player::PlayerKey key);
 
     void run();

=== modified file 'src/core/media/service_stub.cpp'
--- src/core/media/service_stub.cpp	2015-04-17 16:44:30 +0000
+++ src/core/media/service_stub.cpp	2015-10-14 20:54:59 +0000
@@ -139,9 +139,17 @@
     });
 }
 
+void media::ServiceStub::set_current_player(Player::PlayerKey key)
+{
+    auto op = d->object->invoke_method_synchronously<mpris::Service::SetCurrentPlayer,
+         void>(key);
+
+    if (op.is_error())
+        throw std::runtime_error("Problem setting current player: " + op.error());
+}
+
 void media::ServiceStub::pause_other_sessions(media::Player::PlayerKey key)
 {
-    std::cout << __PRETTY_FUNCTION__ << std::endl;
     auto op = d->object->invoke_method_synchronously<mpris::Service::PauseOtherSessions,
          void>(key);
 

=== modified file 'src/core/media/service_stub.h'
--- src/core/media/service_stub.h	2015-04-10 16:13:55 +0000
+++ src/core/media/service_stub.h	2015-10-14 20:54:59 +0000
@@ -45,6 +45,7 @@
     void destroy_session(const std::string& uuid, const Player::Configuration&);
     std::shared_ptr<Player> create_fixed_session(const std::string& name, const Player::Configuration&);
     std::shared_ptr<Player> resume_session(Player::PlayerKey key);
+    void set_current_player(Player::PlayerKey key);
     void pause_other_sessions(Player::PlayerKey key);
 
   private:

=== modified file 'src/core/media/track_list_implementation.cpp'
--- src/core/media/track_list_implementation.cpp	2015-08-11 15:25:50 +0000
+++ src/core/media/track_list_implementation.cpp	2015-10-14 20:54:59 +0000
@@ -215,20 +215,26 @@
 
 void media::TrackListImplementation::reset()
 {
-    std::cout << __PRETTY_FUNCTION__ << std::endl;
-
     // Make sure playback stops
     on_end_of_tracklist()();
+    // And make sure there is no "current" track
+    media::TrackListSkeleton::reset();
 
     auto result = tracks().update([this](TrackList::Container& container)
     {
-        container.clear();
+        TrackList::Container ids = container;
+
+        for (auto it = container.begin(); it != container.end(); ) {
+            auto id = *it;
+            it = container.erase(it);
+            on_track_removed()(id);
+        }
+
         container.resize(0);
         d->track_counter = 0;
+
         return true;
     });
 
-    media::TrackListSkeleton::reset();
-
     (void) result;
 }

=== modified file 'src/core/media/track_list_skeleton.cpp'
--- src/core/media/track_list_skeleton.cpp	2015-08-11 15:25:50 +0000
+++ src/core/media/track_list_skeleton.cpp	2015-10-14 20:54:59 +0000
@@ -241,16 +241,31 @@
 
 bool media::TrackListSkeleton::has_next()
 {
-    if (tracks().get().empty())
+    const auto n_tracks = tracks().get().size();
+
+    if (n_tracks == 0)
         return false;
 
+    // TODO Using current_iterator() makes media-hub crash later. Logic for
+    // handling the iterators must be reviewed. As a minimum updates to the
+    // track list should update current_track instead of the list being sneakly
+    // changed in player_implementation.cpp.
+    // To avoid the crash we consider that current_track will be eventually
+    // initialized to the first track when current_iterator() gets called.
+    if (d->current_track == d->empty_iterator) {
+        if (n_tracks < 2)
+            return false;
+        else
+            return true;
+    }
+
     const auto next_track = std::next(current_iterator());
     return !is_last_track(next_track);
 }
 
 bool media::TrackListSkeleton::has_previous()
 {
-    if (tracks().get().empty())
+    if (tracks().get().empty() || d->current_track == d->empty_iterator)
         return false;
 
     // If we are looping over the entire list, then there is always a previous track
@@ -273,8 +288,11 @@
 media::Track::Id media::TrackListSkeleton::next()
 {
     std::cout << __PRETTY_FUNCTION__ << std::endl;
-    if (tracks().get().empty())
-        return *(d->empty_iterator);
+    if (tracks().get().empty()) {
+        // TODO Change ServiceSkeleton to return with error from DBus call
+        std::cerr << "ERROR: no tracks, cannot go to next" << std::endl;
+        return media::Track::Id{};
+    }
 
     const auto next_track = std::next(current_iterator());
     bool do_go_to_next_track = false;
@@ -328,8 +346,11 @@
 media::Track::Id media::TrackListSkeleton::previous()
 {
     std::cout << __PRETTY_FUNCTION__ << std::endl;
-    if (tracks().get().empty())
-        return *(d->empty_iterator);
+    if (tracks().get().empty()) {
+        // TODO Change ServiceSkeleton to return with error from DBus call
+        std::cerr << "ERROR: no tracks, cannot go to previous" << std::endl;
+        return media::Track::Id{};
+    }
 
     bool do_go_to_previous_track = false;
 
@@ -521,7 +542,6 @@
 
 void media::TrackListSkeleton::reset()
 {
-    std::cout << __PRETTY_FUNCTION__ << std::endl;
     d->current_track = d->empty_iterator;
 }
 

=== modified file 'src/core/media/track_list_skeleton.h'
--- src/core/media/track_list_skeleton.h	2015-07-27 22:15:33 +0000
+++ src/core/media/track_list_skeleton.h	2015-10-14 20:54:59 +0000
@@ -51,10 +51,12 @@
     const core::Property<Container>& tracks() const;
 
     const core::Signal<ContainerTrackIdTuple>& on_track_list_replaced() const;
+    core::Signal<ContainerTrackIdTuple>& on_track_list_replaced();
     const core::Signal<Track::Id>& on_track_added() const;
     core::Signal<Track::Id>& on_track_added();
     const core::Signal<Track::Id>& on_track_removed() const;
     const core::Signal<Track::Id>& on_track_changed() const;
+    core::Signal<Track::Id>& on_track_changed();
     const core::Signal<std::pair<Track::Id, bool>>& on_go_to_track() const;
     core::Signal<std::pair<Track::Id, bool>>& on_go_to_track();
     const core::Signal<void>& on_end_of_tracklist() const;
@@ -77,9 +79,6 @@
 
     core::Property<bool>& can_edit_tracks();
 
-    core::Signal<ContainerTrackIdTuple>& on_track_list_replaced();
-    core::Signal<Track::Id>& on_track_changed();
-
     void reset();
 
 private: