Merge lp:~phablet-team/media-hub/fix-1538703-take2 into lp:media-hub

[Jim Hodapp]

A rewrite of how the current player is set which is what the MPRIS control interface actively uses.

A rewrite of how the current player is set which is what the MPRIS control interface actively uses.

[Konrad Zapałowicz (kzapalowicz)]

Minor issue

review: Needs Fixing (code)

[lp:~phablet-team/media-hub/fix-1538703-take2]

199. By Jim Hodapp on 2016-05-03

Make sure to check if player_service is not nullptr before using it in is_current_player()

[Konrad Zapałowicz (kzapalowicz)]

LGTM

review: Approve (code)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:198
http://jenkins.qa.ubuntu.com/job/media-hub-ci/406/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/246
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/246
        deb: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/246/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/246

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/406/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:199
http://jenkins.qa.ubuntu.com/job/media-hub-ci/407/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/247
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/247
        deb: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/247/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/247

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/407/rebuild

review: Approve (continuous-integration)

[Alfonso Sanchez-Beato (alfonsosanchezbeato)]

Looks good, just curious why you had to remove the version dependency for libdbus-cpp-dev ?

review: Needs Information

[lp:~phablet-team/media-hub/fix-1538703-take2]

200. By Jim Hodapp on 2016-05-04

Restore the libdbus-cpp-dev min version check

[Alfonso Sanchez-Beato (alfonsosanchezbeato)]

LGTM

review: Approve

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2016-04-04 18:54:45 +0000
+++ CMakeLists.txt	2016-05-04 13:11:44 +0000
@@ -20,11 +20,11 @@
   # we define the version to be 5.0.0
   if (${DISTRO_CODENAME} STREQUAL "vivid")
     set(UBUNTU_MEDIA_HUB_VERSION_MAJOR 4)
-    set(UBUNTU_MEDIA_HUB_VERSION_MINOR 1)
+    set(UBUNTU_MEDIA_HUB_VERSION_MINOR 2)
     set(UBUNTU_MEDIA_HUB_VERSION_PATCH 0)
   else ()
     set(UBUNTU_MEDIA_HUB_VERSION_MAJOR 5)
-    set(UBUNTU_MEDIA_HUB_VERSION_MINOR 0)
+    set(UBUNTU_MEDIA_HUB_VERSION_MINOR 1)
     set(UBUNTU_MEDIA_HUB_VERSION_PATCH 0)
   endif()
 endif()
 
=== modified file 'debian/changelog'
--- debian/changelog	2016-04-07 02:00:40 +0000
+++ debian/changelog	2016-05-04 13:11:44 +0000
@@ -1,3 +1,10 @@
+media-hub (4.2.0+16.04.20160407.1-0ubuntu1) UNRELEASED; urgency=medium
+
+  * Improve the MPRIS player control interface so that it only
+    controls the last Player instance that called play(). 
+
+ -- Jim Hodapp <[e-mail]>  Tue, 03 May 2016 11:32:14 -0400
+
 media-hub (4.1.0+16.04.20160407.1-0ubuntu1) xenial; urgency=medium
 
   * Add a proper logger to media-hub that includes traces, timestamps
 
=== modified file 'debian/libmedia-hub-doc.install'
--- debian/libmedia-hub-doc.install	2014-03-06 22:51:51 +0000
+++ debian/libmedia-hub-doc.install	2016-05-04 13:11:44 +0000
@@ -1,1 +1,1 @@
-usr/share/doc/music-hub/html
+usr/share/doc/media-hub/html
 
=== modified file 'doc/CMakeLists.txt'
--- doc/CMakeLists.txt	2013-08-14 18:05:23 +0000
+++ doc/CMakeLists.txt	2016-05-04 13:11:44 +0000
@@ -26,5 +26,5 @@
     COMMENT "Generating API documentation with Doxygen" VERBATIM)
   install(
     DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/html
-    DESTINATION share/doc/music-hub)
-endif(DOXYGEN_FOUND)
 \ No newline at end of file
+    DESTINATION share/doc/media-hub)
+endif(DOXYGEN_FOUND)
 
=== modified file 'include/core/media/service.h'
--- include/core/media/service.h	2015-09-08 20:03:56 +0000
+++ include/core/media/service.h	2016-05-04 13:11:44 +0000
@@ -60,9 +60,6 @@
     /** @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/hashed_keyed_player_store.cpp'
--- src/core/media/hashed_keyed_player_store.cpp	2016-03-07 14:51:01 +0000
+++ src/core/media/hashed_keyed_player_store.cpp	2016-05-04 13:11:44 +0000
@@ -64,6 +64,7 @@
 void media::HashedKeyedPlayerStore::remove_player_for_key(const media::Player::PlayerKey& key)
 {
     std::lock_guard<std::recursive_mutex> lg{guard};
+
     auto it = map.find(key);
     if (it != map.end())
     {
 
=== modified file 'src/core/media/mpris/service.h'
--- src/core/media/mpris/service.h	2016-01-04 08:30:35 +0000
+++ src/core/media/mpris/service.h	2016-05-04 13:11:44 +0000
@@ -127,7 +127,6 @@
     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_configuration.h'
--- src/core/media/player_configuration.h	2015-04-17 16:44:30 +0000
+++ src/core/media/player_configuration.h	2016-05-04 13:11:44 +0000
@@ -19,6 +19,7 @@
 #define CORE_UBUNTU_MEDIA_PLAYER_CLIENT_CONFIGURATION_H_
 
 #include <core/media/player.h>
+#include <core/media/service.h>
 
 #include <core/dbus/bus.h>
 #include <core/dbus/object.h>
@@ -35,6 +36,8 @@
     std::shared_ptr<core::dbus::Service> service;
     // The actual session object representing a player instance.
     std::shared_ptr<core::dbus::Object> session;
+    // The Service instance that manages Player instances
+    core::ubuntu::media::Service* player_service;
 };
 
 #endif // CORE_UBUNTU_MEDIA_PLAYER_CLIENT_CONFIGURATION_H_
 
=== modified file 'src/core/media/player_implementation.cpp'
--- src/core/media/player_implementation.cpp	2016-04-06 15:28:29 +0000
+++ src/core/media/player_implementation.cpp	2016-05-04 13:11:44 +0000
@@ -17,7 +17,10 @@
  *              Jim Hodapp <[e-mail]>
  */
 
+#include <core/media/service.h>
+
 #include "player_implementation.h"
+#include "service_skeleton.h"
 #include "util/timeout.h"
 
 #include <unistd.h>
@@ -301,14 +304,14 @@
         }
     }
 
-    void update_mpris_properties(void)
+    void update_mpris_properties()
     {
-        bool has_previous =    track_list->has_previous()
+        const bool has_previous = track_list->has_previous()
                             or parent->Parent::loop_status() != Player::LoopStatus::none;
-        bool has_next =    track_list->has_next()
+        const 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;
+        const auto n_tracks = track_list->tracks()->size();
+        const bool has_tracks = (n_tracks > 0) ? true : false;
 
         MH_INFO("Updating MPRIS TrackList properties:");
         MH_INFO("\tTracks: %d", n_tracks);
@@ -322,6 +325,58 @@
         parent->can_go_next().set(has_next);
     }
 
+    bool pause_other_players(media::Player::PlayerKey key)
+    {
+        if (not config.parent.player_service)
+            return false;
+
+        media::ServiceSkeleton* skel {
+            reinterpret_cast<media::ServiceSkeleton*>(config.parent.player_service)
+        };
+        skel->pause_other_sessions(key);
+        return true;
+    }
+
+    bool update_current_player(media::Player::PlayerKey key)
+    {
+        if (not config.parent.player_service)
+            return false;
+
+        media::ServiceSkeleton* skel {
+            reinterpret_cast<media::ServiceSkeleton*>(config.parent.player_service)
+        };
+        skel->set_current_player(key);
+        return true;
+    }
+
+    bool is_current_player() const
+    {
+        if (not config.parent.player_service)
+            return false;
+
+        media::ServiceSkeleton* skel {
+            reinterpret_cast<media::ServiceSkeleton*>(config.parent.player_service)
+        };
+        return skel->is_current_player(parent->key());
+    }
+
+    bool is_multimedia_role() const
+    {
+        return parent->audio_stream_role() == media::Player::AudioStreamRole::multimedia;
+    }
+
+    bool reset_current_player()
+    {
+        if (not config.parent.player_service)
+            return false;
+
+        media::ServiceSkeleton* skel {
+            reinterpret_cast<media::ServiceSkeleton*>(config.parent.player_service)
+        };
+        skel->reset_current_player();
+        return true;
+    }
+
     // Our link back to our parent.
     media::PlayerImplementation<Parent>* parent;
     // We just store the parameters passed on construction.
@@ -478,6 +533,16 @@
         // are cleared
         d->clear_wakelocks();
         d->track_list->reset();
+
+        // This is not a fatal error but merely a warning that should
+        // be logged
+        if (d->is_multimedia_role() and d->is_current_player())
+        {
+            MH_DEBUG("==== Resetting current player");
+            if (not d->reset_current_player())
+                MH_WARNING("Failed to reset current player");
+        }
+
         // And tell the outside world that the client has gone away
         d->on_client_disconnected();
     });
@@ -687,7 +752,8 @@
     d->track_list->reset();
 
     // If empty uri, give the same meaning as QMediaPlayer::setMedia("")
-    if (uri.empty()) {
+    if (uri.empty())
+    {
         MH_DEBUG("Resetting current media");
         return true;
     }
@@ -697,6 +763,7 @@
     // Don't set new track as the current track to play since we're calling open_resource_for_uri above
     static const bool make_current = false;
     d->track_list->add_track_with_uri_at(uri, media::TrackList::after_empty_track(), make_current);
+
     return ret;
 }
 
@@ -722,6 +789,19 @@
 void media::PlayerImplementation<Parent>::play()
 {
     MH_TRACE("");
+    if (d->is_multimedia_role())
+    {
+        MH_DEBUG("==== Pausing all other multimedia player sessions");
+        if (not d->pause_other_players(d->config.key))
+            MH_WARNING("Failed to pause other player sessions");
+
+        MH_DEBUG("==== Updating the current player");
+        // This player will begin playing so make sure it's the current player. If
+        // this operation fails it is not a fatal condition but should be logged
+        if (not d->update_current_player(d->config.key))
+            MH_WARNING("Failed to update current player");
+    }
+
     d->engine->play();
 }
 
 
=== modified file 'src/core/media/player_skeleton.h'
--- src/core/media/player_skeleton.h	2015-04-17 15:13:56 +0000
+++ src/core/media/player_skeleton.h	2016-05-04 13:11:44 +0000
@@ -57,6 +57,8 @@
         std::shared_ptr<core::dbus::Service> service;
         // The session object that we want to expose the skeleton upon.
         std::shared_ptr<core::dbus::Object> session;
+        // The Service instance that manages Player instances
+        core::ubuntu::media::Service* player_service;
         // Our functional dependencies.
         apparmor::ubuntu::RequestContextResolver::Ptr request_context_resolver;
         apparmor::ubuntu::RequestAuthenticator::Ptr request_authenticator;
 
=== modified file 'src/core/media/service_implementation.cpp'
--- src/core/media/service_implementation.cpp	2016-04-05 19:27:50 +0000
+++ src/core/media/service_implementation.cpp	2016-05-04 13:11:44 +0000
@@ -186,11 +186,13 @@
     auto player = std::make_shared<media::PlayerImplementation<media::PlayerSkeleton>>
                     (media::PlayerImplementation<media::PlayerSkeleton>::Configuration
     {
+        // Derive a PlayerSkeleton-specific Configuration based on Player::Configuration
         media::PlayerSkeleton::Configuration
         {
             conf.bus,
             conf.service,
             conf.session,
+            conf.player_service,
             d->request_context_resolver,
             d->request_authenticator
         },
@@ -258,11 +260,6 @@
   return std::shared_ptr<media::Player>();
 }
 
-void media::ServiceImplementation::set_current_player(Player::PlayerKey)
-{
-  // no impl
-}
-
 void media::ServiceImplementation::pause_other_sessions(media::Player::PlayerKey key)
 {
     MH_TRACE("");
@@ -276,11 +273,9 @@
     const std::shared_ptr<media::Player> current_player =
             d->configuration.player_store->player_for_key(key);
 
-    // We immediately make the player known as new current player.
-    if (current_player->audio_stream_role() == media::Player::multimedia)
-        d->configuration.player_store->set_current_player_for_key(key);
-
-    d->configuration.player_store->enumerate_players([current_player, key](const media::Player::PlayerKey& other_key, const std::shared_ptr<media::Player>& other_player)
+    d->configuration.player_store->enumerate_players([current_player, key]
+            (const media::Player::PlayerKey& other_key,
+             const std::shared_ptr<media::Player>& other_player)
     {
         // Only pause a Player if all of the following criteria are met:
         // 1) currently playing
 
=== modified file 'src/core/media/service_implementation.h'
--- src/core/media/service_implementation.h	2015-09-08 20:03:56 +0000
+++ src/core/media/service_implementation.h	2016-05-04 13:11:44 +0000
@@ -49,7 +49,6 @@
     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	2016-04-06 15:28:29 +0000
+++ src/core/media/service_skeleton.cpp	2016-05-04 13:11:44 +0000
@@ -63,7 +63,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, impl)
+          exported(impl->access_bus(), config.cover_art_resolver, impl, configuration)
     {
         object->install_method_handler<mpris::Service::CreateSession>(
                     std::bind(
@@ -95,11 +95,6 @@
                         &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,
@@ -133,7 +128,8 @@
             key,
             impl->access_bus(),
             impl->access_service(),
-            impl->access_service()->add_object_for_path(op)
+            impl->access_service()->add_object_for_path(op),
+            impl
         };
 
         MH_DEBUG("Session created by request of: %s, key: %d, uuid: %d, path: %s",
@@ -241,12 +237,6 @@
                         // 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)
-                        {
-                            MH_TRACE("Setting current_player");
-                            exported.set_current_player(player);
-                        }
 
                         auto reply = dbus::Message::make_method_return(msg);
                         reply->writer() << op;
@@ -369,7 +359,8 @@
                     key,
                     impl->access_bus(),
                     impl->access_service(),
-                    impl->access_service()->add_object_for_path(op)
+                    impl->access_service()->add_object_for_path(op),
+                    impl
                 };
 
                 auto session = impl->create_session(config);
@@ -543,7 +534,7 @@
         }
 
         explicit Exported(const dbus::Bus::Ptr& bus, const media::CoverArtResolver& cover_art_resolver,
-                media::ServiceSkeleton* impl)
+                media::ServiceSkeleton* impl, const ServiceSkeleton::Configuration& config)
             : bus{bus},
               /* Export MediaHub service interface on dbus */
               service{dbus::Service::add_service(bus, "org.mpris.MediaPlayer2.MediaHub")},
@@ -552,7 +543,8 @@
               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},
-              impl{impl}
+              impl{impl},
+              service_skel_config(config)
         {
             object->install_method_handler<core::dbus::interfaces::Properties::GetAll>([this](const core::dbus::Message::Ptr& msg)
             {
@@ -574,7 +566,7 @@
             // Setup method handlers for mpris::Player methods.
             auto next = [this](const core::dbus::Message::Ptr& msg)
             {
-                const auto sp = current_player.lock();
+                const auto sp = service_skel_config.player_store->current_player().get();
 
                 if (is_multimedia_role())
                     sp->next();
@@ -585,7 +577,7 @@
 
             auto previous = [this](const core::dbus::Message::Ptr& msg)
             {
-                const auto sp = current_player.lock();
+                const auto sp = service_skel_config.player_store->current_player().get();
 
                 if (is_multimedia_role())
                     sp->previous();
@@ -596,7 +588,7 @@
 
             auto pause = [this](const core::dbus::Message::Ptr& msg)
             {
-                const auto sp = current_player.lock();
+                const auto sp = service_skel_config.player_store->current_player().get();
 
                 if (is_multimedia_role() and sp->can_pause())
                     sp->pause();
@@ -607,7 +599,7 @@
 
             auto stop = [this](const core::dbus::Message::Ptr& msg)
             {
-                const auto sp = current_player.lock();
+                const auto sp = service_skel_config.player_store->current_player().get();
 
                 if (is_multimedia_role())
                     sp->stop();
@@ -618,7 +610,7 @@
 
             auto play = [this, impl](const core::dbus::Message::Ptr& msg)
             {
-                const auto sp = current_player.lock();
+                const auto sp = service_skel_config.player_store->current_player().get();
 
                 if (is_multimedia_role() and sp->can_play())
                 {
@@ -636,7 +628,7 @@
 
             auto play_pause = [this, impl](const core::dbus::Message::Ptr& msg)
             {
-                const auto sp = current_player.lock();
+                const auto sp = service_skel_config.player_store->current_player().get();
 
                 if (is_multimedia_role())
                 {
@@ -662,67 +654,79 @@
 
         inline bool is_multimedia_role()
         {
-            const auto sp = current_player.lock();
+            MH_TRACE("");
 
+            const auto sp = service_skel_config.player_store->current_player().get();
             return (sp ? sp->audio_stream_role() == media::Player::AudioStreamRole::multimedia : false);
         }
 
-        void set_current_player(const std::shared_ptr<media::Player>& cp)
+        void set_current_player(media::Player::PlayerKey key)
         {
             MH_TRACE("");
-            // We will not keep the object alive.
-            current_player = cp;
+
+            // Update the current player in the Player store
+            service_skel_config.player_store->set_current_player_for_key(key);
+            const auto player_sp = service_skel_config.player_store->current_player().get();
 
             // And announce that we can be controlled again.
             player.properties.can_control->set(true);
 
             // We wire up player state changes
-            connections.seeked_to = cp->seeked_to().connect([this](std::uint64_t position)
+            connections.seeked_to = player_sp->seeked_to().connect([this](std::uint64_t position)
             {
                 player.signals.seeked_to->emit(position);
             });
 
-            connections.duration_changed = cp->duration().changed().connect([this](std::uint64_t duration)
+            connections.duration_changed = player_sp->duration().changed().connect([this](std::uint64_t duration)
             {
                 player.properties.duration->set(duration);
             });
 
-            connections.position_changed = cp->position().changed().connect([this](std::uint64_t position)
+            connections.position_changed = player_sp->position().changed().connect([this](std::uint64_t position)
             {
                 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 = player_sp->playback_status().changed().connect(
+                [this, key, player_sp](core::ubuntu::media::Player::PlaybackStatus status)
             {
-                player.properties.playback_status->set(mpris::Player::PlaybackStatus::from(status));
+                const auto cp = service_skel_config.player_store->current_player().get();
+                // If key points to the current player's key, then update status
+                if (cp and key == cp->key())
+                    player.properties.playback_status->set(mpris::Player::PlaybackStatus::from(status));
             });
 
-            connections.loop_status_changed = cp->loop_status().changed().connect(
+            connections.loop_status_changed = player_sp->loop_status().changed().connect(
                 [this](core::ubuntu::media::Player::LoopStatus status)
             {
                 player.properties.loop_status->set(mpris::Player::LoopStatus::from(status));
             });
 
-            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(
+            connections.can_play_changed = player_sp->can_play().changed().connect(
+                [this, key, player_sp](bool can_play)
+            {
+                const auto cp = service_skel_config.player_store->current_player().get();
+                // If key points to the current player's key, then update can_play
+                if (cp and key == cp->key())
+                    player.properties.can_play->set(can_play);
+            });
+
+            connections.can_pause_changed = player_sp->can_pause().changed().connect(
+                [this, key, player_sp](bool can_pause)
+            {
+                const auto cp = service_skel_config.player_store->current_player().get();
+                // If key points to the current player's key, then update can_pause
+                if (cp and key == cp->key())
+                    player.properties.can_pause->set(can_pause);
+            });
+
+            connections.can_go_previous_changed = player_sp->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(
+            connections.can_go_next_changed = player_sp->can_go_next().changed().connect(
                 [this](bool can_go_next)
             {
                 player.properties.can_go_next->set(can_go_next);
@@ -734,16 +738,16 @@
             // 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.duration->set(player_sp->duration().get());
+            player.properties.position->set(player_sp->position().get());
             player.properties.playback_status->set(mpris::Player::PlaybackStatus::from(
-                                                       cp->playback_status().get()));
+                                                       player_sp->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());
+                                                   player_sp->loop_status().get()));
+            player.properties.can_play->set(player_sp->can_play().get());
+            player.properties.can_pause->set(player_sp->can_pause().get());
+            player.properties.can_go_previous->set(player_sp->can_go_previous().get());
+            player.properties.can_go_next->set(player_sp->can_go_next().get());
 
 #if 0
             // TODO cover_art_resolver() is not implemented yet
@@ -788,7 +792,27 @@
             MH_TRACE("");
             // And announce that we can no longer be controlled.
             player.properties.can_control->set(false);
-            current_player.reset();
+            player.properties.can_play->set(false);
+            player.properties.can_pause->set(false);
+            player.properties.can_go_previous->set(false);
+            player.properties.can_go_next->set(false);
+
+            // Reset to null event connections
+            connections.seeked_to = the_empty_signal.connect([](){});
+            connections.duration_changed = the_empty_signal.connect([](){});
+            connections.position_changed = the_empty_signal.connect([](){});
+            connections.playback_status_changed = the_empty_signal.connect([](){});
+            connections.loop_status_changed = the_empty_signal.connect([](){});
+            connections.can_play_changed = the_empty_signal.connect([](){});
+            connections.can_pause_changed = the_empty_signal.connect([](){});
+            connections.can_go_previous_changed = the_empty_signal.connect([](){});
+            connections.can_go_next_changed = the_empty_signal.connect([](){});
+        }
+
+        bool is_current_player(media::Player::PlayerKey key)
+
+        {
+            return key == service_skel_config.player_store->current_player().get()->key();
         }
 
         dbus::Bus::Ptr bus;
@@ -801,10 +825,9 @@
 
         // The CoverArtResolver used by the exported player.
         media::CoverArtResolver cover_art_resolver;
-        // The actual player instance.
-        std::weak_ptr<media::Player> current_player;
 
         media::ServiceSkeleton* impl;
+        ServiceSkeleton::Configuration service_skel_config;
 
         // We track event connections.
         struct
@@ -899,11 +922,23 @@
         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);
+        d->exported.set_current_player(key);
+}
+
+bool media::ServiceSkeleton::is_current_player(media::Player::PlayerKey key) const
+{
+    return d->exported.is_current_player(key);
+}
+
+void media::ServiceSkeleton::reset_current_player()
+{
+    MH_TRACE("");
+    d->exported.reset_current_player();
 }
 
 void media::ServiceSkeleton::pause_other_sessions(media::Player::PlayerKey key)
 {
+    MH_TRACE("");
     d->configuration.impl->pause_other_sessions(key);
 }
 
 
=== modified file 'src/core/media/service_skeleton.h'
--- src/core/media/service_skeleton.h	2015-09-08 20:03:56 +0000
+++ src/core/media/service_skeleton.h	2016-05-04 13:11:44 +0000
@@ -60,6 +60,8 @@
     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);
+    bool is_current_player(Player::PlayerKey key) const;
+    void reset_current_player();
     void pause_other_sessions(Player::PlayerKey key);
 
     void run();
 
=== modified file 'src/core/media/service_stub.cpp'
--- src/core/media/service_stub.cpp	2015-09-08 20:03:56 +0000
+++ src/core/media/service_stub.cpp	2016-05-04 13:11:44 +0000
@@ -139,15 +139,6 @@
     });
 }
 
-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)
 {
     auto op = d->object->invoke_method_synchronously<mpris::Service::PauseOtherSessions,
 
=== modified file 'src/core/media/service_stub.h'
--- src/core/media/service_stub.h	2015-09-08 20:03:56 +0000
+++ src/core/media/service_stub.h	2016-05-04 13:11:44 +0000
@@ -45,7 +45,6 @@
     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: