[Merge] lp:~ricmm/media-hub/session-reattach-nonfixed into lp:media-hub
Jim Hodapp
jim.hodapp at canonical.com
Thu Apr 16 13:13:53 UTC 2015
Some added comments to my review and tvoss' review.
Diff comments:
> === modified file 'CMakeLists.txt'
> --- CMakeLists.txt 2015-01-23 13:00:36 +0000
> +++ CMakeLists.txt 2015-04-15 18:29:47 +0000
> @@ -3,7 +3,7 @@
> project(ubuntu-media-hub)
>
> set(UBUNTU_MEDIA_HUB_VERSION_MAJOR 3)
> -set(UBUNTU_MEDIA_HUB_VERSION_MINOR 0)
> +set(UBUNTU_MEDIA_HUB_VERSION_MINOR 1)
> set(UBUNTU_MEDIA_HUB_VERSION_PATCH 0)
>
> set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra -fPIC -pthread")
>
> === modified file 'debian/changelog'
> --- debian/changelog 2015-04-02 16:43:42 +0000
> +++ debian/changelog 2015-04-15 18:29:47 +0000
> @@ -1,3 +1,9 @@
> +media-hub (3.1.0) UNRELEASED; urgency=medium
> +
> + * Support for signalling player reconnection up the stack.
> +
> + -- Ricardo Mendoza <ricardo.mendoza at canonical.com> Fri, 10 Apr 2015 18:20:32 +0200
> +
> media-hub (3.0.0+15.04.20150402-0ubuntu1) vivid; urgency=medium
>
> [ Jim Hodapp ]
>
> === modified file 'include/core/media/player.h'
> --- include/core/media/player.h 2015-04-15 18:29:47 +0000
> +++ include/core/media/player.h 2015-04-15 18:29:47 +0000
> @@ -126,6 +126,9 @@
> Player& operator=(const Player&) = delete;
> bool operator==(const Player&) const = delete;
>
> + virtual std::string uuid() const = 0;
I'm ok with leaving it as uuid as the verbosity inherently shows that the id is unique by the name. Yes usually ids are unique almost by definition, but this eliminates any doubt without having to look at the documentation.
> + virtual void reconnect() = 0;
> +
> virtual std::shared_ptr<TrackList> track_list() = 0;
> virtual PlayerKey key() const = 0;
>
>
> === modified file 'include/core/media/service.h'
> --- include/core/media/service.h 2014-09-30 06:51:15 +0000
> +++ include/core/media/service.h 2015-04-15 18:29:47 +0000
> @@ -42,9 +42,25 @@
> Service& operator=(const Service&) = delete;
> bool operator==(const Service&) const = delete;
>
> + /** @brief Creates a session with the media-hub service. */
> virtual std::shared_ptr<Player> create_session(const Player::Configuration&) = 0;
> +
> + /** @brief Detaches a UUID-identified session for later resuming. */
> + virtual void detach_session(const std::string& uuid, const Player::Configuration&) = 0;
> +
> + /** @brief Reattaches to a UUID-identified session that is in detached state. */
> + virtual std::shared_ptr<Player> reattach_session(const std::string& uuid, const Player::Configuration&) = 0;
> +
> + /** @brief Asks the service to destroy a session. The session is destroyed when the client exits. */
> + virtual void destroy_session(const std::string& uuid, const Player::Configuration&) = 0;
> +
> + /** @brief Creates a fixed-named session with the media-hub service. */
> virtual std::shared_ptr<Player> create_fixed_session(const std::string& name, const Player::Configuration&) = 0;
> +
> + /** @brief Resumes a fixed-name session directly by player key. */
> virtual std::shared_ptr<Player> resume_session(Player::PlayerKey) = 0;
> +
> + /** @brief Pauses sessions other than the supplied one. */
> virtual void pause_other_sessions(Player::PlayerKey) = 0;
>
> protected:
>
> === modified file 'src/core/media/mpris/service.h'
> --- src/core/media/mpris/service.h 2014-09-30 06:51:15 +0000
> +++ src/core/media/mpris/service.h 2015-04-15 18:29:47 +0000
> @@ -48,6 +48,42 @@
> }
> };
>
> + struct DetachingSession
> + {
> + static const std::string& name()
> + {
> + static const std::string s
> + {
> + "core.ubuntu.media.Service.Error.DetachingSession"
> + };
> + return s;
> + }
> + };
> +
> + struct ReattachingSession
> + {
> + static const std::string& name()
> + {
> + static const std::string s
> + {
> + "core.ubuntu.media.Service.Error.ReattachingSession"
> + };
> + return s;
> + }
> + };
> +
> + struct DestroyingSession
> + {
> + static const std::string& name()
> + {
> + static const std::string s
> + {
> + "core.ubuntu.media.Service.Error.DestroyingSession"
> + };
> + return s;
> + }
> + };
> +
> struct CreatingFixedSession
> {
> static const std::string& name()
> @@ -74,6 +110,9 @@
> };
>
> DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(CreateSession, Service, 1000)
> + DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(DetachSession, Service, 1000)
> + DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(ReattachSession, Service, 1000)
> + 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(PauseOtherSessions, Service, 1000)
>
> === modified file 'src/core/media/player_configuration.h'
> --- src/core/media/player_configuration.h 2014-11-26 16:23:19 +0000
> +++ src/core/media/player_configuration.h 2015-04-15 18:29:47 +0000
> @@ -27,7 +27,7 @@
> // to the implementation in a way that is opaque to the client.
> struct core::ubuntu::media::Player::Configuration
> {
> - // Unique key for identifying the session.
> + // Unique key for identifying the session path.
> core::ubuntu::media::Player::PlayerKey key;
> // The bus connection to expose objects on.
> std::shared_ptr<core::dbus::Bus> bus;
>
> === modified file 'src/core/media/player_implementation.cpp'
> --- src/core/media/player_implementation.cpp 2015-04-15 18:29:47 +0000
> +++ src/core/media/player_implementation.cpp 2015-04-15 18:29:47 +0000
> @@ -512,6 +512,18 @@
> }
>
> template<typename Parent>
> +std::string media::PlayerImplementation<Parent>::uuid() const
> +{
> + // No impl for now, as not needed internally.
But who would actually end up calling this? I agree that returning an empty string would be a good idea though.
> +}
> +
> +template<typename Parent>
> +void media::PlayerImplementation<Parent>::reconnect()
> +{
> + d->config.client_death_observer->register_for_death_notifications_with_key(d->config.key);
> +}
> +
> +template<typename Parent>
> std::shared_ptr<media::TrackList> media::PlayerImplementation<Parent>::track_list()
> {
> return d->track_list;
>
> === modified file 'src/core/media/player_implementation.h'
> --- src/core/media/player_implementation.h 2015-04-15 18:29:47 +0000
> +++ src/core/media/player_implementation.h 2015-04-15 18:29:47 +0000
> @@ -55,6 +55,9 @@
> PlayerImplementation(const Configuration& configuration);
> ~PlayerImplementation();
>
> + virtual std::string uuid() const;
> + virtual void reconnect();
> +
> virtual std::shared_ptr<TrackList> track_list();
> virtual Player::PlayerKey key() const;
>
>
> === modified file 'src/core/media/player_stub.cpp'
> --- src/core/media/player_stub.cpp 2015-04-15 18:29:47 +0000
> +++ src/core/media/player_stub.cpp 2015-04-15 18:29:47 +0000
> @@ -43,10 +43,12 @@
> struct media::PlayerStub::Private
> {
> Private(const std::shared_ptr<Service>& parent,
> - const std::shared_ptr<core::dbus::Object>& object
> + const std::shared_ptr<core::dbus::Object>& object,
> + const std::string uuid
> ) : parent(parent),
> object(object),
> key(object->invoke_method_synchronously<mpris::Player::Key, media::Player::PlayerKey>().value()),
> + uuid(uuid),
> sink_factory(media::video::make_platform_default_sink_factory(key)),
> properties
> {
> @@ -93,6 +95,7 @@
> std::shared_ptr<TrackList> track_list;
> dbus::Object::Ptr object;
> media::Player::PlayerKey key;
> + std::string uuid;
> media::video::SinkFactory sink_factory;
> struct
> {
> @@ -209,8 +212,9 @@
>
> media::PlayerStub::PlayerStub(
> const std::shared_ptr<Service>& parent,
> - const std::shared_ptr<core::dbus::Object>& object)
> - : d(new Private{parent, object})
> + const std::shared_ptr<core::dbus::Object>& object,
> + std::string uuid)
> + : d(new Private{parent, object, uuid})
> {
> }
>
> @@ -218,6 +222,16 @@
> {
> }
>
> +std::string media::PlayerStub::uuid() const
> +{
> + return d->uuid;
> +}
> +
> +void media::PlayerStub::reconnect()
> +{
> + // No implementation
> +}
> +
> std::shared_ptr<media::TrackList> media::PlayerStub::track_list()
> {
> if (!d->track_list)
>
> === modified file 'src/core/media/player_stub.h'
> --- src/core/media/player_stub.h 2015-04-15 18:29:47 +0000
> +++ src/core/media/player_stub.h 2015-04-15 18:29:47 +0000
> @@ -39,10 +39,14 @@
> public:
> explicit PlayerStub(
> const std::shared_ptr<Service>& parent,
> - const std::shared_ptr<core::dbus::Object>& object);
> + const std::shared_ptr<core::dbus::Object>& object,
> + std::string uuid = "");
I missed this one in my review, but I'd also like to see it be std::string& uuid = std::string{}
>
> ~PlayerStub();
>
> + virtual std::string uuid() const;
> + virtual void reconnect();
> +
> virtual std::shared_ptr<TrackList> track_list();
> virtual PlayerKey key() const;
>
>
> === modified file 'src/core/media/server/server.cpp'
> --- src/core/media/server/server.cpp 2014-12-15 14:43:44 +0000
> +++ src/core/media/server/server.cpp 2015-04-15 18:29:47 +0000
> @@ -123,7 +123,7 @@
> {
> impl,
> player_store,
> -
> + external_services
> });
>
> std::thread service_worker
>
> === modified file 'src/core/media/service_skeleton.cpp'
> --- src/core/media/service_skeleton.cpp 2015-04-15 18:29:47 +0000
> +++ src/core/media/service_skeleton.cpp 2015-04-15 18:29:47 +0000
> @@ -35,6 +35,10 @@
>
> #include <core/posix/this_process.h>
>
> +#include <boost/uuid/uuid.hpp>
> +#include <boost/uuid/uuid_generators.hpp>
> +#include <boost/uuid/uuid_io.hpp>
> +
> #include <map>
> #include <regex>
> #include <sstream>
> @@ -50,17 +54,33 @@
> struct media::ServiceSkeleton::Private
> {
> Private(media::ServiceSkeleton* impl, const ServiceSkeleton::Configuration& config)
> - : impl(impl),
> + : request_context_resolver(media::apparmor::ubuntu::make_platform_default_request_context_resolver(config.external_services)),
> + impl(impl),
> object(impl->access_service()->add_object_for_path(
> dbus::traits::Service<media::Service>::object_path())),
> - exported(impl->access_bus(), config.cover_art_resolver),
> - configuration(config)
> + configuration(config),
> + exported(impl->access_bus(), config.cover_art_resolver)
> {
> object->install_method_handler<mpris::Service::CreateSession>(
> std::bind(
> &Private::handle_create_session,
> this,
> std::placeholders::_1));
> + object->install_method_handler<mpris::Service::DetachSession>(
> + std::bind(
> + &Private::handle_detach_session,
> + this,
> + std::placeholders::_1));
> + object->install_method_handler<mpris::Service::ReattachSession>(
> + std::bind(
> + &Private::handle_reattach_session,
> + this,
> + std::placeholders::_1));
> + object->install_method_handler<mpris::Service::DestroySession>(
> + std::bind(
> + &Private::handle_destroy_session,
> + this,
> + std::placeholders::_1));
> object->install_method_handler<mpris::Service::CreateFixedSession>(
> std::bind(
> &Private::handle_create_fixed_session,
> @@ -78,24 +98,26 @@
> std::placeholders::_1));
> }
>
> - std::pair<std::string, media::Player::PlayerKey> create_session_info()
> + std::tuple<std::string, media::Player::PlayerKey, std::string> create_session_info()
> {
> static unsigned int session_counter = 0;
>
> unsigned int current_session = session_counter++;
> + boost::uuids::uuid uuid = gen();
What would you need to test with the id generation that you can't already test by calling create_session_info as it returns the new uuid as part of the tuple?
>
> std::stringstream ss;
> ss << "/core/ubuntu/media/Service/sessions/" << current_session;
>
> - return std::make_pair(ss.str(), media::Player::PlayerKey(current_session));
> + return std::make_tuple(ss.str(), media::Player::PlayerKey(current_session), to_string(uuid));
> }
>
> void handle_create_session(const core::dbus::Message::Ptr& msg)
> {
> - auto session_info = create_session_info();
> + auto session_info = create_session_info();
>
> - dbus::types::ObjectPath op{session_info.first};
> - media::Player::PlayerKey key{session_info.second};
> + dbus::types::ObjectPath op{std::get<0>(session_info)};
> + media::Player::PlayerKey key{std::get<1>(session_info)};
> + std::string uuid{std::get<2>(session_info)};
>
> media::Player::Configuration config
> {
> @@ -104,11 +126,21 @@
> impl->access_service()->add_object_for_path(op)
> };
>
> + std::cout << "Session created by request of: " << msg->sender() << ", uuid: " << uuid << ", path:" << op << std::endl;
> +
> try
> {
> 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)
> + {
> + 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())));
> + });
> +
> auto reply = dbus::Message::make_method_return(msg);
> - reply->writer() << op;
> + reply->writer() << std::make_tuple(op, uuid);
>
> impl->access_bus()->send(reply);
> } catch(const std::runtime_error& e)
> @@ -121,6 +153,171 @@
> }
> }
>
> + void handle_detach_session(const core::dbus::Message::Ptr& msg)
> + {
> + try
> + {
> + std::string uuid;
> + msg->reader() >> uuid;
> +
> + auto key = uuid_player_map.at(uuid);
> +
> + if (player_owner_map.count(key) != 0) {
> + auto info = player_owner_map.at(key);
> + // Check if session is attached(1) and that the detachment
> + // request comes from the same peer(2) that created the session.
> + if (std::get<1>(info) && (std::get<2>(info) == msg->sender())) { // Player is attached
> + std::get<1>(info) = false; // Detached
> + std::get<2>(info).clear(); // Clear registered sender/peer
> + auto player = configuration.player_store->player_for_key(key);
> + player->lifetime().set(media::Player::Lifetime::resumable);
> + }
> + }
> +
> + auto reply = dbus::Message::make_method_return(msg);
> + impl->access_bus()->send(reply);
> +
> + } catch(const std::runtime_error& e)
> + {
> + auto reply = dbus::Message::make_error(
> + msg,
> + mpris::Service::Errors::DetachingSession::name(),
> + e.what());
> + impl->access_bus()->send(reply);
> + }
> + }
> +
> + void handle_reattach_session(const core::dbus::Message::Ptr& msg)
> + {
> + try
> + {
> + std::string uuid;
> + msg->reader() >> uuid;
> +
> + if (uuid_player_map.count(uuid) != 0) {
> + auto key = uuid_player_map.at(uuid);
> + if (not configuration.player_store->has_player_for_key(key)) {
> + auto reply = dbus::Message::make_error(
> + msg,
> + mpris::Service::Errors::ReattachingSession::name(),
> + "Unable to locate player session");
> + impl->access_bus()->send(reply);
> + return;
> + }
> + std::stringstream ss;
> + 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)
> + {
> + 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());
> + if (std::get<0>(info) == context.str()) {
> + std::get<1>(info) = true; // Set to Attached
> + std::get<2>(info) = msg->sender(); // Register new owner
> +
> + // Signal player reconnection
> + auto player = configuration.player_store->player_for_key(key);
> + player->reconnect();
> +
> + auto reply = dbus::Message::make_method_return(msg);
> + reply->writer() << op;
> +
> + impl->access_bus()->send(reply);
> + }
> + else {
> + auto reply = dbus::Message::make_error(
> + msg,
> + mpris::Service::Errors::ReattachingSession::name(),
> + "Invalid permissions for the requested session");
> + impl->access_bus()->send(reply);
> + return;
> + }
> + });
> + }
> + else {
> + auto reply = dbus::Message::make_error(
> + msg,
> + mpris::Service::Errors::ReattachingSession::name(),
> + "Invalid session");
> + impl->access_bus()->send(reply);
> + return;
> + }
> + } catch(const std::runtime_error& e)
> + {
> + auto reply = dbus::Message::make_error(
> + msg,
> + mpris::Service::Errors::ReattachingSession::name(),
> + e.what());
> + impl->access_bus()->send(reply);
> + }
> + }
> +
> + void handle_destroy_session(const core::dbus::Message::Ptr& msg)
> + {
> +
> + try
> + {
> + std::string uuid;
> + msg->reader() >> uuid;
> +
> + if (uuid_player_map.count(uuid) != 0) {
> + auto key = uuid_player_map.at(uuid);
> + if (not configuration.player_store->has_player_for_key(key)) {
> + auto reply = dbus::Message::make_error(
> + msg,
> + mpris::Service::Errors::DestroyingSession::name(),
> + "Unable to locate player session");
> + impl->access_bus()->send(reply);
> + return;
> + }
> +
> + // Remove control entries from the map, at this point
> + // 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)
> + {
> + 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());
> + if (std::get<0>(info) == context.str()) {
> + player_owner_map.erase(key);
> +
> + // Reset lifecycle to non-resumable on the now-abandoned session
> + auto player = configuration.player_store->player_for_key(key);
> + player->lifetime().set(media::Player::Lifetime::normal);
> +
> + auto reply = dbus::Message::make_method_return(msg);
> + impl->access_bus()->send(reply);
> + }
> + else {
> + auto reply = dbus::Message::make_error(
> + msg,
> + mpris::Service::Errors::DestroyingSession::name(),
> + "Invalid permissions for the requested session");
> + impl->access_bus()->send(reply);
> + return;
> + }
> + });
> + }
> + else {
> + auto reply = dbus::Message::make_error(
> + msg,
> + mpris::Service::Errors::DestroyingSession::name(),
> + "Invalid session");
> + impl->access_bus()->send(reply);
> + return;
> + }
> + } catch(const std::runtime_error& e)
> + {
> + auto reply = dbus::Message::make_error(
> + msg,
> + mpris::Service::Errors::DestroyingSession::name(),
> + e.what());
> + impl->access_bus()->send(reply);
> + }
> + }
> +
> void handle_create_fixed_session(const core::dbus::Message::Ptr& msg)
> {
> try
> @@ -132,8 +329,8 @@
> // Create new session
> auto session_info = create_session_info();
>
> - dbus::types::ObjectPath op{session_info.first};
> - media::Player::PlayerKey key{session_info.second};
> + dbus::types::ObjectPath op{std::get<0>(session_info)};
> + media::Player::PlayerKey key{std::get<1>(session_info)};
>
> media::Player::Configuration config
> {
> @@ -147,7 +344,6 @@
>
> configuration.player_store->add_player_for_key(key, session);
>
> -
> named_player_map.insert(std::make_pair(name, key));
>
> auto reply = dbus::Message::make_method_return(msg);
> @@ -231,6 +427,7 @@
> impl->access_bus()->send(reply);
> }
>
> + media::apparmor::ubuntu::RequestContextResolver::Ptr request_context_resolver;
> media::ServiceSkeleton* impl;
> dbus::Object::Ptr object;
>
> @@ -238,6 +435,14 @@
> ServiceSkeleton::Configuration configuration;
> // We map named/fixed player instances to their respective keys.
> std::map<std::string, media::Player::PlayerKey> named_player_map;
> + // We map UUIDs to their respective keys.
> + std::map<std::string, media::Player::PlayerKey> uuid_player_map;
> + // We keep a list of keys and their respective owners and states.
> + // value: (owner context, attached state, attached dbus name)
> + std::map<media::Player::PlayerKey, std::tuple<std::string, bool, std::string>> player_owner_map;
> +
> + boost::uuids::random_generator gen;
> +
> // We expose the entire service as an MPRIS player.
> struct Exported
> {
>
> === modified file 'src/core/media/service_skeleton.h'
> --- src/core/media/service_skeleton.h 2014-12-15 14:43:44 +0000
> +++ src/core/media/service_skeleton.h 2015-04-15 18:29:47 +0000
> @@ -19,6 +19,8 @@
> #ifndef CORE_UBUNTU_MEDIA_SERVICE_SKELETON_H_
> #define CORE_UBUNTU_MEDIA_SERVICE_SKELETON_H_
>
> +#include "apparmor/ubuntu.h"
> +
> #include <core/media/service.h>
>
> #include "cover_art_resolver.h"
> @@ -43,6 +45,7 @@
> {
> std::shared_ptr<Service> impl;
> KeyedPlayerStore::Ptr player_store;
> + helper::ExternalServices& external_services;
> CoverArtResolver cover_art_resolver;
> };
>
>
> === modified file 'src/core/media/service_stub.cpp'
> --- src/core/media/service_stub.cpp 2014-10-23 14:42:59 +0000
> +++ src/core/media/service_stub.cpp 2015-04-15 18:29:47 +0000
> @@ -58,7 +58,7 @@
> std::shared_ptr<media::Player> media::ServiceStub::create_session(const media::Player::Configuration&)
> {
> auto op = d->object->invoke_method_synchronously<mpris::Service::CreateSession,
> - dbus::types::ObjectPath>();
> + std::tuple<dbus::types::ObjectPath, std::string>>();
>
> if (op.is_error())
> throw std::runtime_error("Problem creating session: " + op.error());
> @@ -66,8 +66,43 @@
> return std::shared_ptr<media::Player>(new media::PlayerStub
> {
> shared_from_this(),
> - access_service()->object_for_path(op.value())
> - });
> + access_service()->object_for_path(std::get<0>(op.value())),
> + std::get<1>(op.value())
> + });
> +}
> +
> +void media::ServiceStub::detach_session(const std::string& uuid, const media::Player::Configuration&)
> +{
> + auto op = d->object->invoke_method_synchronously<mpris::Service::DetachSession,
> + void>(uuid);
> +
> + if (op.is_error())
> + throw std::runtime_error("Problem detaching session: " + op.error());
> +}
> +
> +std::shared_ptr<media::Player> media::ServiceStub::reattach_session(const std::string& uuid, const media::Player::Configuration&)
> +{
> + auto op = d->object->invoke_method_synchronously<mpris::Service::ReattachSession,
> + dbus::types::ObjectPath>(uuid);
> +
> + if (op.is_error())
> + throw std::runtime_error("Problem reattaching session: " + op.error());
> +
> + return std::shared_ptr<media::Player>(new media::PlayerStub
> + {
> + shared_from_this(),
> + access_service()->object_for_path(op.value()),
> + uuid
> + });
> +}
> +
> +void media::ServiceStub::destroy_session(const std::string& uuid, const media::Player::Configuration&)
> +{
> + auto op = d->object->invoke_method_synchronously<mpris::Service::DestroySession,
> + void>(uuid);
> +
> + if (op.is_error())
> + throw std::runtime_error("Problem destroying session: " + op.error());
> }
>
> std::shared_ptr<media::Player> media::ServiceStub::create_fixed_session(const std::string& name, const media::Player::Configuration&)
>
> === modified file 'src/core/media/service_stub.h'
> --- src/core/media/service_stub.h 2014-10-23 14:42:59 +0000
> +++ src/core/media/service_stub.h 2015-04-15 18:29:47 +0000
> @@ -40,6 +40,9 @@
> ~ServiceStub();
>
> std::shared_ptr<Player> create_session(const Player::Configuration&);
> + void detach_session(const std::string& uuid, const Player::Configuration&);
> + std::shared_ptr<Player> reattach_session(const std::string& uuid, const Player::Configuration&);
> + 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 pause_other_sessions(Player::PlayerKey key);
>
> === modified file 'tests/test-track-list/test_track_list.cpp'
> --- tests/test-track-list/test_track_list.cpp 2015-04-15 18:29:47 +0000
> +++ tests/test-track-list/test_track_list.cpp 2015-04-15 18:29:47 +0000
> @@ -44,7 +44,7 @@
> {
> }
>
> -void media::TestTrackList::create_new_player_session()
> +std::string media::TestTrackList::create_new_player_session()
> {
> try {
> m_hubPlayerSession = m_hubService->create_session(media::Player::Client::default_configuration());
> @@ -59,9 +59,51 @@
> catch (std::runtime_error &e) {
> cerr << "FATAL: Failed to retrieve the current player's TrackList: " << e.what() << endl;
> }
> -}
> -
> -void media::TestTrackList::destroy_player_session()
> +
> + std::string uuid;
> + try {
> + uuid.assign(m_hubPlayerSession->uuid());
> + }
> + catch (std::runtime_error &e) {
> + cerr << "FATAL: Failed to retrieve the current player's uuid: " << e.what() << endl;
> + }
> +
> + cout << "Connected to session " << uuid << endl;
> + return uuid;
> +}
> +
> +void media::TestTrackList::detach_player_session(const std::string &uuid)
> +{
> + try {
> + m_hubService->detach_session(uuid, media::Player::Client::default_configuration());
> + }
> + catch (std::runtime_error &e) {
> + cerr << "FATAL: Failed to detach the media-hub player session: " << e.what() << endl;
> + }
> +
> + cout << "Detached session " << uuid << endl;
> +}
> +
> +void media::TestTrackList::reattach_player_session(const std::string &uuid)
> +{
> + try {
> + m_hubPlayerSession = m_hubService->reattach_session(uuid, media::Player::Client::default_configuration());
> + }
> + catch (std::runtime_error &e) {
> + cerr << "FATAL: Failed to reattach the media-hub player session: " << e.what() << endl;
> + }
> +
> + try {
> + m_hubTrackList = m_hubPlayerSession->track_list();
> + }
> + catch (std::runtime_error &e) {
> + cerr << "FATAL: Failed to retrieve the current player's TrackList: " << e.what() << endl;
> + }
> +
> + cout << "Re-connected to session " << uuid << endl;
> +}
> +
> +void media::TestTrackList::destroy_player_session(const std::string &uuid)
> {
> // TODO: explicitly add a destroy session to the Service class after ricmm lands his new creation_session
> // that returns a session ID. This will allow me to clear the tracklist after each test.
> @@ -121,8 +163,28 @@
> //destroy_player_session();
> }
>
> +void media::TestTrackList::test_tracklist_resume(const std::string &uri1, const std::string &uri2, const std::string &uuid)
> +{
> + cout << "--> Running test: test_tracklist_resume" << std::endl;
> +
> + add_track(uri1);
> + add_track(uri2);
> +
> + int initial_size = m_hubTrackList->tracks().get().size();
> + cout << "Tracklist size: " << initial_size << endl;
> + detach_player_session(uuid);
> + reattach_player_session(uuid);
> + cout << "Tracklist size: " << m_hubTrackList->tracks().get().size() << endl;
> +
> + m_hubPlayerSession->play();
> +
> + if (initial_size != m_hubTrackList->tracks().get().size())
> + cout << "Tracklist sizes are different, error in resuming" << endl;
> +}
> +
> void media::TestTrackList::test_ensure_tracklist_is_not_empty(const std::string &uri1, const std::string &uri2)
> {
> + //create_new_player_session();
> cout << "--> Running test: test_ensure_tracklist_is_not_empty" << std::endl;
>
> add_track(uri1);
> @@ -335,6 +397,11 @@
> tracklist->test_shuffle(argv[1], argv[2], argv[3]);
> tracklist->test_remove_track(argv[1], argv[2], argv[3]);
> }
> + else if (argc == 5)
> + {
> + std::string uuid = tracklist->create_new_player_session();
> + tracklist->test_tracklist_resume(argv[1], argv[2], uuid);
> + }
> else
> {
> cout << "Can't test TrackList, no track(s) specified." << endl;
>
> === modified file 'tests/test-track-list/test_track_list.h'
> --- tests/test-track-list/test_track_list.h 2015-04-15 18:29:47 +0000
> +++ tests/test-track-list/test_track_list.h 2015-04-15 18:29:47 +0000
> @@ -45,14 +45,19 @@
> TestTrackList();
> ~TestTrackList();
>
> - void create_new_player_session();
> - void destroy_player_session();
> + std::string create_new_player_session();
> + void detach_player_session(const std::string &uuid);
> + void reattach_player_session(const std::string &uuid);
> + void destroy_player_session(const std::string &uuid);
>
> void add_track(const std::string &uri, bool make_current = false);
>
> // Takes in one or two files for playback, adds it/them to the TrackList, and plays
> void test_basic_playback(const std::string &uri1, const std::string &uri2 = std::string{});
>
> + // Takes two uris and confirms that they remain after a detach/reattach
> + void test_tracklist_resume(const std::string &uri1, const std::string &uri2, const std::string &uuid);
> +
> void test_ensure_tracklist_is_not_empty(const std::string &uri1, const std::string &uri2 = std::string{});
>
> // Takes in one or two files for playback, adds it/them to the TrackList, plays and makes sure
>
--
https://code.launchpad.net/~ricmm/media-hub/session-reattach-nonfixed/+merge/256101
Your team Ubuntu Phablet Team is subscribed to branch lp:~phablet-team/media-hub/tracklist-cli.
More information about the Ubuntu-reviews
mailing list