[Merge] lp:~thomas-voss/media-hub/decouple-service-skeleton-and-implementation into lp:media-hub
Thomas Voß
thomas.voss at canonical.com
Mon Dec 15 14:49:09 UTC 2014
Diff comments:
> === modified file 'src/core/media/CMakeLists.txt'
> --- src/core/media/CMakeLists.txt 2014-12-01 13:20:31 +0000
> +++ src/core/media/CMakeLists.txt 2014-12-01 13:20:31 +0000
> @@ -87,7 +87,8 @@
> ${MPRIS_HEADERS}
>
> client_death_observer.cpp
> - hybris_client_death_observer.cpp
> + hashed_keyed_player_store.cpp
> + hybris_client_death_observer.cpp
> cover_art_resolver.cpp
> engine.cpp
>
>
> === added file 'src/core/media/hashed_keyed_player_store.cpp'
> --- src/core/media/hashed_keyed_player_store.cpp 1970-01-01 00:00:00 +0000
> +++ src/core/media/hashed_keyed_player_store.cpp 2014-12-01 13:20:31 +0000
> @@ -0,0 +1,80 @@
> +/*
> + * Copyright © 2014 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>
> + */
> +
> +#include <core/media/hashed_keyed_player_store.h>
> +
> +namespace media = core::ubuntu::media;
> +
> +media::HashedKeyedPlayerStore::HashedKeyedPlayerStore()
> +{
> +}
> +
> +const core::Property<std::shared_ptr<media::Player>>& media::HashedKeyedPlayerStore::current_player() const
> +{
> + return prop_current_player;
> +}
> +
> +bool media::HashedKeyedPlayerStore::has_player_for_key(const media::Player::PlayerKey& key) const
> +{
> + std::lock_guard<std::mutex> lg{guard};
> + return map.count(key) > 0;
> +}
> +
> +std::shared_ptr<media::Player> media::HashedKeyedPlayerStore::player_for_key(const media::Player::PlayerKey& key) const
> +{
> + std::lock_guard<std::mutex> lg{guard};
> + auto it = map.find(key);
> +
> + if (it == map.end()) throw std::out_of_range
> + {
> + "HashedKeyedPlayerStore::player_for_key: No player known for " + std::to_string(key)
> + };
> +
> + return it->second;
> +}
> +
> +void media::HashedKeyedPlayerStore::enumerate_players(const media::KeyedPlayerStore::PlayerEnumerator& enumerator) const
> +{
> + std::lock_guard<std::mutex> lg{guard};
> + for (const auto& pair : map)
> + enumerator(pair.first, pair.second);
> +}
> +
> +void media::HashedKeyedPlayerStore::add_player_for_key(const media::Player::PlayerKey& key, const std::shared_ptr<media::Player>& player)
> +{
> + std::lock_guard<std::mutex> lg{guard};
> + map[key] = player;
> +}
> +
> +void media::HashedKeyedPlayerStore::remove_player_for_key(const media::Player::PlayerKey& key)
> +{
> + std::lock_guard<std::mutex> lg{guard};
> + auto it = map.find(key);
> + if (it != map.end())
> + {
> + if (prop_current_player == it->second)
> + prop_current_player = nullptr;
> +
> + map.erase(it);
> + }
> +}
> +
> +void media::HashedKeyedPlayerStore::set_current_player_for_key(const media::Player::PlayerKey& key)
> +{
> + prop_current_player = player_for_key(key);
> +}
>
> === added file 'src/core/media/hashed_keyed_player_store.h'
> --- src/core/media/hashed_keyed_player_store.h 1970-01-01 00:00:00 +0000
> +++ src/core/media/hashed_keyed_player_store.h 2014-12-01 13:20:31 +0000
> @@ -0,0 +1,76 @@
> +/*
> + * Copyright © 2014 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 CORE_UBUNTU_MEDIA_HASHED_KEYED_PLAYER_STORE_H_
> +#define CORE_UBUNTU_MEDIA_HASHED_KEYED_PLAYER_STORE_H_
> +
> +#include <core/media/keyed_player_store.h>
> +
> +#include <mutex>
> +#include <unordered_map>
> +
> +namespace core
> +{
> +namespace ubuntu
> +{
> +namespace media
> +{
> +// Implements KeyedPlayerStore using a std::unordered_map.
> +class HashedKeyedPlayerStore : public KeyedPlayerStore
> +{
> +public:
> + HashedKeyedPlayerStore();
> + // We keep track of the "current" player, that is, the one
Yup, adjusted the comment
> + // that has been created most recently and provide a getable/observable
> + // access to that designated instance.
> + const core::Property<std::shared_ptr<media::Player>>& current_player() const override;
> +
> + // We keep track of all known player sessions here and render them accessible via
> + // the key. All of these functions are thread-safe but not reentrant.
> + // Returns true iff a player is known for the given key.
> + bool has_player_for_key(const Player::PlayerKey& key) const override;
> +
> + // Returns the player for the given key or throws std::out_of_range if no player is known
> + // for the given key.
> + // Throws std::out_of_range if no player is known for the key.
> + std::shared_ptr<Player> player_for_key(const Player::PlayerKey& key) const override;
> +
> + // Enumerates all known players and invokes the given enumerator for each
> + // (key, player) pair.
> + void enumerate_players(const PlayerEnumerator& enumerator) const override;
> +
> + // Adds the given player with the given key.
> + void add_player_for_key(const Player::PlayerKey& key, const std::shared_ptr<Player>& player) override;
> +
> + // Removes the player for the given key, and unsets it if it is the current one.
> + void remove_player_for_key(const Player::PlayerKey& key) override;
> +
> + // Makes the player known under the given key current.
> + // Throws std::out_of_range if no player is known for the key.
> + void set_current_player_for_key(const Player::PlayerKey& key) override;
> +
> +private:
> + core::Property<std::shared_ptr<Player>> prop_current_player;
> + mutable std::mutex guard;
> + std::unordered_map<Player::PlayerKey, std::shared_ptr<Player>> map;
> +};
> +}
> +}
> +}
> +
> +#endif // CORE_UBUNTU_MEDIA_KEYED_PLAYER_STORE_H_
>
> === added file 'src/core/media/keyed_player_store.cpp'
> === added file 'src/core/media/keyed_player_store.h'
> --- src/core/media/keyed_player_store.h 1970-01-01 00:00:00 +0000
> +++ src/core/media/keyed_player_store.h 2014-12-01 13:20:31 +0000
> @@ -0,0 +1,83 @@
> +/*
> + * Copyright © 2014 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 CORE_UBUNTU_MEDIA_KEYED_PLAYER_STORE_H_
> +#define CORE_UBUNTU_MEDIA_KEYED_PLAYER_STORE_H_
> +
> +#include <core/media/player.h>
> +
> +#include <core/property.h>
> +
> +#include <functional>
> +#include <memory>
> +
> +namespace core
> +{
> +namespace ubuntu
> +{
> +namespace media
> +{
> +// An interface abstracting keyed lookups of known Player instances.
> +class KeyedPlayerStore
> +{
> +public:
> + // Save us some typing.
> + typedef std::shared_ptr<KeyedPlayerStore> Ptr;
> + // Functor for enumerating all known (key, player) pairs.
> + typedef std::function
> + <
> + void(
> + // The key of the player.
> + const Player::PlayerKey&,
> + // The actual player instance.
> + const std::shared_ptr<Player>&
> + )
> + > PlayerEnumerator;
> + // We keep track of the "current" player, that is, the one
> + // that has been created most recently and provide a getable/observable
> + // access to that designated instance.
> + virtual const core::Property<std::shared_ptr<Player>>& current_player() const = 0;
> +
> + // We keep track of all known player sessions here and render them accessible via
> + // the key. All of these functions are thread-safe but not reentrant.
> + // Returns true iff a player is known for the given key.
> + virtual bool has_player_for_key(const Player::PlayerKey& key) const = 0;
> + // Returns the player for the given key or throws std::out_of_range if no player is known
> + // for the given key.
> + virtual std::shared_ptr<Player> player_for_key(const Player::PlayerKey& key) const = 0;
> + // Enumerates all known players and invokes the given enumerator for each
> + // (key, player) pair.
> + virtual void enumerate_players(const PlayerEnumerator& enumerator) const = 0;
> + // Adds the given player with the given key.
> + virtual void add_player_for_key(const Player::PlayerKey& key, const std::shared_ptr<Player>& player) = 0;
> + // Removes the player for the given key, and unsets it if it is the current one.
> + virtual void remove_player_for_key(const Player::PlayerKey& key) = 0;
> + // Makes the player known under the given key current.
> + virtual void set_current_player_for_key(const Player::PlayerKey& key) = 0;
> +
> +protected:
> + KeyedPlayerStore() = default;
> + KeyedPlayerStore(const KeyedPlayerStore&) = delete;
> + virtual ~KeyedPlayerStore() = default;
> + KeyedPlayerStore& operator=(const KeyedPlayerStore&) = delete;
> +};
> +}
> +}
> +}
> +
> +#endif // CORE_UBUNTU_MEDIA_KEYED_PLAYER_STORE_H_
>
> === modified file 'src/core/media/server/server.cpp'
> --- src/core/media/server/server.cpp 2014-12-01 13:20:31 +0000
> +++ src/core/media/server/server.cpp 2014-12-01 13:20:31 +0000
> @@ -20,6 +20,7 @@
> #include <core/media/player.h>
> #include <core/media/track_list.h>
>
> +#include "core/media/hashed_keyed_player_store.h"
> #include "core/media/service_implementation.h"
>
> #include <core/posix/signal.h>
> @@ -96,23 +97,37 @@
> }
> };
>
> + // Our common player store instance for tracking player instances.
> + auto player_store = std::make_shared<media::HashedKeyedPlayerStore>();
> // We assemble the configuration for executing the service now.
> media::ServiceImplementation::Configuration service_config
> {
> + std::make_shared<media::HashedKeyedPlayerStore>(),
> external_services
> };
>
> - auto service = std::make_shared<media::ServiceImplementation>(service_config);
> + auto impl = std::make_shared<media::ServiceImplementation>(media::ServiceImplementation::Configuration
> + {
> + player_store,
> + external_services
> + });
> +
> + auto skeleton = std::make_shared<media::ServiceSkeleton>(media::ServiceSkeleton::Configuration
> + {
> + impl,
> + player_store,
> +
> + });
>
> std::thread service_worker
> {
> - [&shutdown_requested, service]()
> + [&shutdown_requested, skeleton]()
> {
> while (not shutdown_requested)
> {
> try
> {
> - service->run();
> + skeleton->run();
> }
> catch (const std::exception& e)
> {
> @@ -134,7 +149,7 @@
>
> // And stop execution of helper and actual service.
> external_services.stop();
> - service->stop();
> + skeleton->stop();
>
> if (external_services_worker.joinable())
> external_services_worker.join();
>
> === modified file 'src/core/media/service_implementation.cpp'
> --- src/core/media/service_implementation.cpp 2014-12-01 13:20:31 +0000
> +++ src/core/media/service_implementation.cpp 2014-12-01 13:20:31 +0000
> @@ -74,7 +74,7 @@
> display_state_lock->request_release(media::power::DisplayState::off);
> }
>
> - media::ServiceImplementation::Configuration configuration;
> + media::ServiceImplementation::Configuration configuration;
> // This holds the key of the multimedia role Player instance that was paused
> // when the battery level reached 10% or 5%
> media::Player::PlayerKey resume_key;
> @@ -86,7 +86,7 @@
> media::audio::OutputObserver::Ptr audio_output_observer;
> media::apparmor::ubuntu::RequestContextResolver::Ptr request_context_resolver;
> media::apparmor::ubuntu::RequestAuthenticator::Ptr request_authenticator;
> - media::telephony::CallMonitor::Ptr call_monitor;
> + media::telephony::CallMonitor::Ptr call_monitor;
> std::list<media::Player::PlayerKey> paused_sessions;
> };
>
> @@ -170,11 +170,15 @@
> // until all dispatches are done
> d->configuration.external_services.io_service.post([this, key]()
> {
> +<<<<<<< TREE
> if (!has_player_for_key(key))
> return;
>
> if (player_for_key(key)->lifetime() == Player::Lifetime::normal)
> remove_player_for_key(key);
> +=======
> + d->configuration.player_store->remove_player_for_key(key);
> +>>>>>>> MERGE-SOURCE
> });
> });
>
> @@ -195,19 +199,19 @@
>
> void media::ServiceImplementation::pause_other_sessions(media::Player::PlayerKey key)
> {
> - if (not has_player_for_key(key))
> + if (not d->configuration.player_store->has_player_for_key(key))
> {
> cerr << "Could not find Player by key: " << key << endl;
> return;
> }
>
> - auto current_player = player_for_key(key);
> + auto 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)
> - set_current_player_for_key(key);
> + d->configuration.player_store->set_current_player_for_key(key);
>
> - 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
> @@ -227,7 +231,7 @@
>
> void media::ServiceImplementation::pause_all_multimedia_sessions()
> {
> - enumerate_players([this](const media::Player::PlayerKey& key, const std::shared_ptr<media::Player>& player)
> + d->configuration.player_store->enumerate_players([this](const media::Player::PlayerKey& key, const std::shared_ptr<media::Player>& player)
> {
> if (player->playback_status() == Player::playing
> && player->audio_stream_role() == media::Player::multimedia)
> @@ -240,19 +244,20 @@
>
> void media::ServiceImplementation::resume_paused_multimedia_sessions()
> {
> - std::for_each(d->paused_sessions.begin(), d->paused_sessions.end(), [this](const media::Player::PlayerKey& key) {
> - player_for_key(key)->play();
> - });
> + std::for_each(d->paused_sessions.begin(), d->paused_sessions.end(), [this](const media::Player::PlayerKey& key)
> + {
> + d->configuration.player_store->player_for_key(key)->play();
> + });
>
> d->paused_sessions.clear();
> }
>
> void media::ServiceImplementation::resume_multimedia_session()
> {
> - if (not has_player_for_key(d->resume_key))
> + if (not d->configuration.player_store->has_player_for_key(d->resume_key))
> return;
>
> - auto player = player_for_key(d->resume_key);
> + auto player = d->configuration.player_store->player_for_key(d->resume_key);
>
> if (player->playback_status() == Player::paused)
> {
>
> === modified file 'src/core/media/service_implementation.h'
> --- src/core/media/service_implementation.h 2014-12-01 13:20:31 +0000
> +++ src/core/media/service_implementation.h 2014-12-01 13:20:31 +0000
> @@ -30,12 +30,13 @@
> {
> class Player;
>
> -class ServiceImplementation : public ServiceSkeleton
> +class ServiceImplementation : public Service
> {
> public:
> // All creation time arguments go here.
> struct Configuration
> {
> + KeyedPlayerStore::Ptr player_store;
> helper::ExternalServices& external_services;
> };
>
>
> === modified file 'src/core/media/service_skeleton.cpp'
> --- src/core/media/service_skeleton.cpp 2014-12-01 13:20:31 +0000
> +++ src/core/media/service_skeleton.cpp 2014-12-01 13:20:31 +0000
> @@ -49,11 +49,12 @@
>
> struct media::ServiceSkeleton::Private
> {
> - Private(media::ServiceSkeleton* impl, const media::CoverArtResolver& resolver)
> + Private(media::ServiceSkeleton* impl, const ServiceSkeleton::Configuration& config)
> : impl(impl),
> object(impl->access_service()->add_object_for_path(
> - dbus::traits::Service<media::Service>::object_path())),
> - exported(impl->access_bus(), resolver)
> + dbus::traits::Service<media::Service>::object_path())),
> + exported(impl->access_bus(), config.cover_art_resolver),
> + configuration(config)
> {
> object->install_method_handler<mpris::Service::CreateSession>(
> std::bind(
> @@ -252,15 +253,7 @@
>
> try
> {
> - auto session = impl->create_session(config);
> -
> - bool inserted = false;
> - std::tie(std::ignore, inserted)
> - = session_store.insert(std::make_pair(key, session));
> -
> - if (!inserted)
> - throw std::runtime_error("Problem persisting session in session store.");
> -
> + configuration.player_store->add_player_for_key(key, impl->create_session(config));
> auto reply = dbus::Message::make_method_return(msg);
> reply->writer() << op;
>
> @@ -290,9 +283,16 @@
> media::ServiceSkeleton* impl;
> dbus::Object::Ptr object;
>
> +<<<<<<< TREE
> + // We query the apparmor profile to obtain an identity for players.
> + org::freedesktop::dbus::DBus::Stub dbus_stub;
> // We track all running player instances.
> std::map<media::Player::PlayerKey, std::shared_ptr<media::Player>> session_store;
> std::map<std::string, media::Player::PlayerKey> fixed_session_store;
> +=======
> + // We remember all our creation time arguments.
> + ServiceSkeleton::Configuration configuration;
> +>>>>>>> MERGE-SOURCE
> // We expose the entire service as an MPRIS player.
> struct Exported
> {
> @@ -526,7 +526,7 @@
> mpris::Player::Skeleton player;
> mpris::Playlists::Skeleton playlists;
>
> - // Helper to resolve (title, artist, album) tuples to cover art.
> + // The CoverArtResolver used by the exported player.
> media::CoverArtResolver cover_art_resolver;
> // The actual player instance.
> std::weak_ptr<media::Player> current_player;
> @@ -561,9 +561,9 @@
> } exported;
> };
>
> -media::ServiceSkeleton::ServiceSkeleton(const media::CoverArtResolver& resolver)
> +media::ServiceSkeleton::ServiceSkeleton(const Configuration& configuration)
> : dbus::Skeleton<media::Service>(the_session_bus()),
> - d(new Private(this, resolver))
> + d(new Private(this, configuration))
> {
> }
>
> @@ -571,6 +571,7 @@
> {
> }
>
> +<<<<<<< TREE
> bool media::ServiceSkeleton::has_player_for_key(const media::Player::PlayerKey& key) const
> {
> return d->session_store.count(key) > 0;
> @@ -611,6 +612,16 @@
> break;
> }
> }
> +=======
> +std::shared_ptr<media::Player> media::ServiceSkeleton::create_session(const media::Player::Configuration& config)
> +{
> + return d->configuration.impl->create_session(config);
> +}
> +
> +void media::ServiceSkeleton::pause_other_sessions(media::Player::PlayerKey key)
> +{
> + d->configuration.impl->pause_other_sessions(key);
> +>>>>>>> MERGE-SOURCE
> }
>
> void media::ServiceSkeleton::run()
>
> === modified file 'src/core/media/service_skeleton.h'
> --- src/core/media/service_skeleton.h 2014-09-09 21:27:29 +0000
> +++ src/core/media/service_skeleton.h 2014-12-01 13:20:31 +0000
> @@ -22,6 +22,7 @@
> #include <core/media/service.h>
>
> #include "cover_art_resolver.h"
> +#include "keyed_player_store.h"
> #include "service_traits.h"
>
> #include <core/dbus/skeleton.h>
> @@ -37,34 +38,20 @@
> class ServiceSkeleton : public core::dbus::Skeleton<core::ubuntu::media::Service>
> {
> public:
> - // Functor for enumerating all known (key, player) pairs.
> - typedef std::function
> - <
> - void(
> - // The key of the player.
> - const core::ubuntu::media::Player::PlayerKey&,
> - // The actual player instance.
> - const std::shared_ptr<core::ubuntu::media::Player>&
> - )
> - > PlayerEnumerator;
> + // Creation time arguments go here.
> + struct Configuration
> + {
> + std::shared_ptr<Service> impl;
> + KeyedPlayerStore::Ptr player_store;
> + CoverArtResolver cover_art_resolver;
> + };
>
> - ServiceSkeleton(const CoverArtResolver& cover_art_resolver = always_missing_cover_art_resolver());
> + ServiceSkeleton(const Configuration& configuration);
> ~ServiceSkeleton();
>
> - // We keep track of all known player sessions here and render them accessible via
> - // the key. All of these functions are thread-safe but not reentrant.
> - // Returns true iff a player is known for the given key.
> - bool has_player_for_key(const Player::PlayerKey& key) const;
> - // Returns the player for the given key or throws std::out_of_range if no player is known
> - // for the given key.
> - std::shared_ptr<Player> player_for_key(const Player::PlayerKey& key) const;
> - // Enumerates all known players and invokes the given enumerator for each
> - // (key, player) pair.
> - void enumerate_players(const PlayerEnumerator& enumerator) const;
> - // Removes the player for the given key, and unsets it if it is the current one.
> - void remove_player_for_key(const Player::PlayerKey& key);
> - // Makes the player known under the given key current.
> - void set_current_player_for_key(const Player::PlayerKey& key);
> + // From media::Service
> + std::shared_ptr<Player> create_session(const Player::Configuration&);
> + void pause_other_sessions(Player::PlayerKey key);
>
> void run();
> void stop();
>
--
https://code.launchpad.net/~thomas-voss/media-hub/decouple-service-skeleton-and-implementation/+merge/243283
Your team Ubuntu Phablet Team is subscribed to branch lp:media-hub.
More information about the Ubuntu-reviews
mailing list