[Merge] lp:~ricmm/media-hub/session-reattach-nonfixed into lp:media-hub
Jim Hodapp
jim.hodapp at canonical.com
Tue Apr 14 19:16:37 UTC 2015
Review: Needs Fixing code
Looks good, a few things to fix.
Diff comments:
> === modified file 'CMakeLists.txt'
> --- CMakeLists.txt 2015-01-23 13:00:36 +0000
> +++ CMakeLists.txt 2015-04-14 02:49:06 +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-14 02:49:06 +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-14 02:49:06 +0000
> +++ include/core/media/player.h 2015-04-14 02:49:06 +0000
> @@ -126,6 +126,9 @@
> Player& operator=(const Player&) = delete;
> bool operator==(const Player&) const = delete;
>
> + virtual std::string uuid() const = 0;
> + 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-14 02:49:06 +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-14 02:49:06 +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-14 02:49:06 +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-14 02:49:06 +0000
> +++ src/core/media/player_implementation.cpp 2015-04-14 02:49:06 +0000
> @@ -521,6 +521,19 @@
> }
>
> template<typename Parent>
> +std::string media::PlayerImplementation<Parent>::uuid() const
> +{
> + // No impl for now, as not needed internally.
> +}
> +
> +template<typename Parent>
> +void media::PlayerImplementation<Parent>::reconnect()
> +{
> + // TODO: Reconnect all signals that would've expired on session dettachment
Is there some reason not to add this code now?
> + 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-14 02:49:06 +0000
> +++ src/core/media/player_implementation.h 2015-04-14 02:49:06 +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-14 02:49:06 +0000
> +++ src/core/media/player_stub.cpp 2015-04-14 02:49:06 +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-14 02:49:06 +0000
> +++ src/core/media/player_stub.h 2015-04-14 02:49:06 +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 = "");
>
> ~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-14 02:49:06 +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-14 02:49:06 +0000
> +++ src/core/media/service_skeleton.cpp 2015-04-14 02:49:06 +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();
>
> 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,169 @@
> }
> }
>
> + 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);
> + if (std::get<1>(info) && (std::get<2>(info) == msg->sender())) { // Player is attached
Add a comment stating what the get<1> and get<2> fields are for easier reference.
> + 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);
> +
> + // Signal player reconnection
This comment doesn't describe what the code is doing.
> + 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 +327,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 +342,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 +425,7 @@
> impl->access_bus()->send(reply);
> }
>
> + media::apparmor::ubuntu::RequestContextResolver::Ptr request_context_resolver;
> media::ServiceSkeleton* impl;
> dbus::Object::Ptr object;
>
> @@ -238,6 +433,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-14 02:49:06 +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-14 02:49:06 +0000
> @@ -58,16 +58,51 @@
> 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>();
> -
> - if (op.is_error())
> - throw std::runtime_error("Problem creating session: " + op.error());
> -
> - return std::shared_ptr<media::Player>(new media::PlayerStub
> - {
> - shared_from_this(),
> - access_service()->object_for_path(op.value())
> - });
> + std::tuple<dbus::types::ObjectPath, std::string>>();
> +
> + if (op.is_error())
> + throw std::runtime_error("Problem creating session: " + op.error());
> +
> + return std::shared_ptr<media::Player>(new media::PlayerStub
> + {
> + shared_from_this(),
> + 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 creating session: " + op.error());
Error text should be "Problem destroying session: "
> }
>
> 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-14 02:49:06 +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-14 02:49:06 +0000
> +++ tests/test-track-list/test_track_list.cpp 2015-04-14 02:49:06 +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,6 +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;
> +
> + //create_new_player_session();
> +
> + 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;
> +
> + //destroy_player_session();
> +}
> void media::TestTrackList::test_has_next_track(const std::string &uri1, const std::string &uri2)
> {
> cout << "--> Running test: test_has_next_track" << std::endl;
> @@ -318,6 +382,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-14 02:49:06 +0000
> +++ tests/test-track-list/test_track_list.h 2015-04-14 02:49:06 +0000
> @@ -45,14 +45,18 @@
> 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);
> // Takes in one or two files for playback, adds it/them to the TrackList, plays and makes sure
> // that the Player advances the TrackList
> void test_has_next_track(const std::string &uri1, const std::string &uri2);
>
--
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