[Merge] lp:~thomas-voss/media-hub/introduce-power-controller-interface into lp:media-hub
Thomas Voß
thomas.voss at canonical.com
Thu Dec 11 13:57:29 UTC 2014
Diff comments:
> === modified file 'src/core/media/CMakeLists.txt'
> --- src/core/media/CMakeLists.txt 2014-11-26 12:42:47 +0000
> +++ src/core/media/CMakeLists.txt 2014-11-26 12:42:47 +0000
> @@ -90,8 +90,12 @@
> hybris_client_death_observer.cpp
> cover_art_resolver.cpp
> engine.cpp
> +
> + power/state_controller.cpp
> +
> recorder_observer.cpp
> hybris_recorder_observer.cpp
> +
> gstreamer/engine.cpp
> gstreamer/playbin.cpp
>
>
> === added file 'src/core/media/external_services.h'
> --- src/core/media/external_services.h 1970-01-01 00:00:00 +0000
> +++ src/core/media/external_services.h 2014-11-26 12:42:47 +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_EXTERNAL_SERVICES_H_
> +#define CORE_UBUNTU_MEDIA_EXTERNAL_SERVICES_H_
> +
> +#include <core/dbus/bus.h>
> +
> +#include <core/dbus/asio/executor.h>
> +
> +#include <boost/asio.hpp>
> +
> +namespace core
> +{
> +namespace ubuntu
> +{
> +namespace media
> +{
> +namespace helper
> +{
> +// A helper struct that bundles:
> +// * a dispatcher, i.e., the io_service
> +// * access to the system and session bus
> +//
> +// In addtion, it allows us to mock out services and
> +// for acceptance testing purposes.
> +struct ExternalServices
> +{
> + ExternalServices(const core::dbus::Bus::Ptr& session, const core::dbus::Bus::Ptr& system)
> + : keep_alive{io_service},
> + session{session},
> + system{system}
> + {
> + }
> +
> + ExternalServices()
> + : ExternalServices
> + {
> + core::dbus::Bus::Ptr{new core::dbus::Bus{core::dbus::WellKnownBus::session}},
> + core::dbus::Bus::Ptr{new core::dbus::Bus{core::dbus::WellKnownBus::system}}
> + }
> + {
> + session->install_executor(core::dbus::asio::make_executor(session, io_service));
> + system->install_executor(core::dbus::asio::make_executor(session, io_service));
> + }
> +
> +
> + void run()
> + {
> + io_service.run();
> + }
> +
> + void stop()
> + {
> + io_service.stop();
> + }
> +
> + boost::asio::io_service io_service;
> + boost::asio::io_service::work keep_alive;
> +
> + core::dbus::Bus::Ptr session;
> + core::dbus::Bus::Ptr system;
> +};
> +}
> +}
> +}
> +}
> +
> +#endif // CORE_UBUNTU_MEDIA_EXTERNAL_SERVICES_H_
>
> === modified file 'src/core/media/player_implementation.cpp'
> --- src/core/media/player_implementation.cpp 2014-11-26 12:42:47 +0000
> +++ src/core/media/player_implementation.cpp 2014-11-26 12:42:47 +0000
> @@ -25,8 +25,6 @@
> #include "engine.h"
> #include "track_list_implementation.h"
>
> -#include "powerd_service.h"
> -#include "unity_screen_service.h"
> #include "gstreamer/engine.h"
>
> #include <memory>
> @@ -52,41 +50,25 @@
> WAKELOCK_CLEAR_INVALID
> };
>
> - Private(PlayerImplementation* parent,
> - const dbus::types::ObjectPath& session_path,
> - const std::shared_ptr<media::Service>& service,
> - PlayerImplementation::PlayerKey key)
> + Private(PlayerImplementation* parent, const media::PlayerImplementation::Configuration& config)
> : parent(parent),
> - service(service),
> + config(config),
> + display_state_lock(config.power_state_controller->display_state_lock()),
> + system_state_lock(config.power_state_controller->system_state_lock()),
> engine(std::make_shared<gstreamer::Engine>()),
> - session_path(session_path),
> track_list(
> new media::TrackListImplementation(
> - session_path.as_string() + "/TrackList",
> + config.session->path().as_string() + "/TrackList",
> engine->meta_data_extractor())),
> - sys_lock_name("media-hub-music-playback"),
> - disp_cookie(-1),
> system_wakelock_count(0),
> display_wakelock_count(0),
> previous_state(Engine::State::stopped),
> - key(key),
> engine_state_change_connection(engine->state().changed().connect(make_state_change_handler()))
> {
> - auto bus = std::shared_ptr<dbus::Bus>(new dbus::Bus(core::dbus::WellKnownBus::system));
> - bus->install_executor(dbus::asio::make_executor(bus));
> -
> - auto stub_service = dbus::Service::use_service(bus, dbus::traits::Service<core::Powerd>::interface_name());
> - powerd_session = stub_service->object_for_path(dbus::types::ObjectPath("/com/canonical/powerd"));
> -
> - auto uscreen_stub_service = dbus::Service::use_service(bus, dbus::traits::Service<core::UScreen>::interface_name());
> - uscreen_session = uscreen_stub_service->object_for_path(dbus::types::ObjectPath("/com/canonical/Unity/Screen"));
> -
> - auto client_death_observer = media::platform_default_client_death_observer();
> -
> - client_death_observer->register_for_death_notifications_with_key(key);
> - client_death_observer->on_client_with_key_died().connect([this](const media::Player::PlayerKey& died)
> + config.client_death_observer->register_for_death_notifications_with_key(config.key);
> + config.client_death_observer->on_client_with_key_died().connect([this](const media::Player::PlayerKey& died)
> {
> - if (died != this->key)
> + if (died != this->config.key)
> return;
>
> on_client_died();
> @@ -169,24 +151,12 @@
> if (parent->is_video_source())
> {
> if (++display_wakelock_count == 1)
> - {
> - auto result = uscreen_session->invoke_method_synchronously<core::UScreen::keepDisplayOn, int>();
> - if (result.is_error())
> - throw std::runtime_error(result.error().print());
> - disp_cookie = result.value();
> - cout << "Requested new display wakelock" << endl;
> - }
> + display_state_lock->request_acquire(media::power::DisplayState::on);
> }
> else
> {
> if (++system_wakelock_count == 1)
> - {
> - auto result = powerd_session->invoke_method_synchronously<core::Powerd::requestSysState, std::string>(sys_lock_name, static_cast<int>(1));
> - if (result.is_error())
> - throw std::runtime_error(result.error().print());
> - sys_cookie = result.value();
> - cout << "Requested new system wakelock" << endl;
> - }
> + system_state_lock->request_acquire(media::power::SystemState::active);
> }
> }
> catch(const std::exception& e)
> @@ -208,20 +178,12 @@
> case wakelock_clear_t::WAKELOCK_CLEAR_SYSTEM:
> // Only actually clear the system wakelock once the count reaches zero
> if (--system_wakelock_count == 0)
> - {
> - cout << "Clearing system wakelock" << endl;
> - powerd_session->invoke_method_synchronously<core::Powerd::clearSysState, void>(sys_cookie);
> - sys_cookie.clear();
> - }
> + system_state_lock->request_release(media::power::SystemState::active);
> break;
> case wakelock_clear_t::WAKELOCK_CLEAR_DISPLAY:
> // Only actually clear the display wakelock once the count reaches zero
> if (--display_wakelock_count == 0)
> - {
> - cout << "Clearing display wakelock" << endl;
> - uscreen_session->invoke_method_synchronously<core::UScreen::removeDisplayOnRequest, void>(disp_cookie);
> - disp_cookie = -1;
> - }
> + display_state_lock->request_release(media::power::DisplayState::on);
> break;
> case wakelock_clear_t::WAKELOCK_CLEAR_INVALID:
> default:
> @@ -275,44 +237,33 @@
> engine->reset();
> }
>
> - PlayerImplementation* parent;
> - std::shared_ptr<Service> service;
> + // Our link back to our parent.
> + media::PlayerImplementation* parent;
> + // We just store the parameters passed on construction.
> + media::PlayerImplementation::Configuration config;
> + media::power::StateController::Lock<media::power::DisplayState>::Ptr display_state_lock;
> + media::power::StateController::Lock<media::power::SystemState>::Ptr system_state_lock;
> +
> std::shared_ptr<Engine> engine;
> - dbus::types::ObjectPath session_path;
> std::shared_ptr<TrackListImplementation> track_list;
> - std::shared_ptr<dbus::Object> powerd_session;
> - std::shared_ptr<dbus::Object> uscreen_session;
> - std::string sys_lock_name;
> - int disp_cookie;
> - std::string sys_cookie;
> std::atomic<int> system_wakelock_count;
> std::atomic<int> display_wakelock_count;
> Engine::State previous_state;
> - PlayerImplementation::PlayerKey key;
> core::Signal<> on_client_disconnected;
> core::Connection engine_state_change_connection;
> };
>
> -media::PlayerImplementation::PlayerImplementation(
> - const std::string& identity,
> - const std::shared_ptr<core::dbus::Bus>& bus,
> - const std::shared_ptr<core::dbus::Object>& session,
> - const std::shared_ptr<Service>& service,
> - PlayerKey key)
> +media::PlayerImplementation::PlayerImplementation(const media::PlayerImplementation::Configuration& config)
> : media::PlayerSkeleton
> {
> media::PlayerSkeleton::Configuration
> {
> - bus,
> - session,
> - identity
> + config.bus,
> + config.session,
> + config.identity
> }
> },
> - d(make_shared<Private>(
> - this,
> - session->path(),
> - service,
> - key))
> + d{std::make_shared<Private>(this, config)}
> {
> // Initialize default values for Player interface properties
> can_play().set(true);
> @@ -452,7 +403,7 @@
> // TODO: Convert this to be a property instead of sync call
> media::Player::PlayerKey media::PlayerImplementation::key() const
> {
> - return d->key;
> + return d->config.key;
> }
>
> media::video::Sink::Ptr media::PlayerImplementation::create_gl_texture_video_sink(std::uint32_t texture_id)
>
> === modified file 'src/core/media/player_implementation.h'
> --- src/core/media/player_implementation.h 2014-11-26 12:42:47 +0000
> +++ src/core/media/player_implementation.h 2014-11-26 12:42:47 +0000
> @@ -21,6 +21,9 @@
>
> #include "player_skeleton.h"
>
> +#include "client_death_observer.h"
> +#include "power/state_controller.h"
> +
> #include <memory>
>
> namespace core
> @@ -35,12 +38,21 @@
> class PlayerImplementation : public PlayerSkeleton
> {
> public:
> - PlayerImplementation(
> - const std::string& identity,
> - const std::shared_ptr<core::dbus::Bus>& bus,
> - const std::shared_ptr<core::dbus::Object>& session,
> - const std::shared_ptr<Service>& service,
> - PlayerKey key);
> + // All creation time arguments go here
> + struct Configuration
> + {
> + std::string identity;
> + std::shared_ptr<core::dbus::Bus> bus;
> + std::shared_ptr<core::dbus::Object> session;
> + std::shared_ptr<Service> service;
> + PlayerKey key;
> +
> + // Functional dependencies
> + ClientDeathObserver::Ptr client_death_observer;
> + power::StateController::Ptr power_state_controller;
> + };
> +
> + PlayerImplementation(const Configuration& configuration);
> ~PlayerImplementation();
>
> virtual std::shared_ptr<TrackList> track_list();
>
> === added directory 'src/core/media/power'
> === added file 'src/core/media/power/state_controller.cpp'
> --- src/core/media/power/state_controller.cpp 1970-01-01 00:00:00 +0000
> +++ src/core/media/power/state_controller.cpp 2014-11-26 12:42:47 +0000
> @@ -0,0 +1,302 @@
> +/*
> + * 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/power/state_controller.h>
> +
> +#include <core/dbus/macros.h>
> +#include <core/dbus/object.h>
> +
> +namespace media = core::ubuntu::media;
> +
> +namespace com { namespace canonical {
> +struct Unity
> +{
> + struct Screen
> + {
> + static const std::string& name()
> + {
> + static std::string s = "com.canonical.Unity.Screen";
> + return s;
> + }
> +
> + static const core::dbus::types::ObjectPath& path()
> + {
> + static core::dbus::types::ObjectPath p{"/com/canonical/Unity/Screen"};
> + return p;
> + }
> +
> + DBUS_CPP_METHOD_DEF(keepDisplayOn, Screen)
> + DBUS_CPP_METHOD_DEF(removeDisplayOnRequest, Screen)
> + };
> +};
> +namespace powerd {
> +struct Interface
> +{
> + static std::string& name()
> + {
> + static std::string s = "com.canonical.powerd";
> + return s;
> + }
> +
> + static const core::dbus::types::ObjectPath& path()
> + {
> + static core::dbus::types::ObjectPath p{"/com/canonical/powerd"};
> + return p;
> + }
> +
> + DBUS_CPP_METHOD_DEF(requestSysState, com::canonical::powerd::Interface)
> + DBUS_CPP_METHOD_DEF(clearSysState, com::canonical::powerd::Interface)
> +};
> +}}}
> +
> +namespace
> +{
> +namespace impl
> +{
> +struct DisplayStateLock : public media::power::StateController::Lock<media::power::DisplayState>,
> + public std::enable_shared_from_this<DisplayStateLock>
> +{
> + // To safe us some typing
> + typedef std::shared_ptr<DisplayStateLock> Ptr;
> +
> + // We postpone releasing the display for this amount of time.
> + static boost::posix_time::seconds timeout_for_release()
> + {
> + return boost::posix_time::seconds{4};
> + }
> +
> + // The invalid cookie marker.
> + static constexpr const std::int32_t the_invalid_cookie{-1};
> +
> + DisplayStateLock(const media::power::StateController::Ptr& parent,
> + boost::asio::io_service& io_service,
> + const core::dbus::Object::Ptr& object)
> + : parent{parent},
> + timeout{io_service},
> + object{object},
> + cookie{the_invalid_cookie}
> + {
> + }
> +
> + // From core::ubuntu::media::power::StateController::Lock<DisplayState>
> + void request_acquire(media::power::DisplayState state) override
> + {
> + if (state == media::power::DisplayState::off)
> + return;
> +
> + std::weak_ptr<DisplayStateLock> wp{shared_from_this()};
> +
> + object->invoke_method_asynchronously_with_callback<com::canonical::Unity::Screen::keepDisplayOn, std::int32_t>([wp, state](const core::dbus::Result<std::int32_t>& result)
> + {
> + if (result.is_error()) // We should at least log the error case here.
> + return;
> +
> + if (auto sp = wp.lock())
> + {
> + sp->cookie = result.value();
> + sp->signals.acquired(state);
> + }
> + });
> + }
> +
> + void request_release(media::power::DisplayState state) override
> + {
> + if (state == media::power::DisplayState::off)
> + return;
> +
> + if (cookie == the_invalid_cookie)
> + return;
> +
> + std::weak_ptr<DisplayStateLock> wp{shared_from_this()};
> +
> + auto current_cookie(cookie);
> +
> + timeout.expires_from_now(timeout_for_release());
> + timeout.async_wait([wp, state, current_cookie](const boost::system::error_code& ec)
> + {
> + if (not ec)
Yup, I adjusted the code to only return early if the operation has been explicitly aborted prior to that.
> + {
> + if (auto sp = wp.lock())
> + {
> + sp->object->invoke_method_asynchronously_with_callback<com::canonical::Unity::Screen::removeDisplayOnRequest, void>([wp, state, current_cookie](const core::dbus::Result<void>& result)
> + {
> + if (result.is_error()) // We should at least log the error case here.
> + return;
> +
> + if (auto sp = wp.lock())
> + {
> + sp->signals.released(state);
> +
> + // We might have issued a different request before and
> + // only call the display state done if the original cookie
> + // corresponds to the one we just gave up.
> + if (sp->cookie == current_cookie)
> + sp->cookie = the_invalid_cookie;
> + }
> +
> + }, current_cookie);
> + }
> + }
> + });
> + }
> +
> + // Emitted whenever the acquire request completes.
> + const core::Signal<media::power::DisplayState>& acquired() const override
> + {
> + return signals.acquired;
> + }
> +
> + // Emitted whenever the release request completes.
> + const core::Signal<media::power::DisplayState>& released() const override
> + {
> + return signals.released;
> + }
> +
> + media::power::StateController::Ptr parent;
> + boost::asio::deadline_timer timeout;
> + core::dbus::Object::Ptr object;
> + std::int32_t cookie;
> +
> + struct
> + {
> + core::Signal<media::power::DisplayState> acquired;
> + core::Signal<media::power::DisplayState> released;
> + } signals;
> +};
> +
> +struct SystemStateLock : public media::power::StateController::Lock<media::power::SystemState>
> +{
> + static constexpr const char* wake_lock_name
> + {
> + "media-hub-playback_lock"
> + };
> +
> + SystemStateLock(const media::power::StateController::Ptr& parent, const core::dbus::Object::Ptr& object)
> + : parent{parent},
> + object{object}
> + {
> + }
> +
> + // Informs the system that the caller would like
> + // the system to stay active.
> + void request_acquire(media::power::SystemState state) override
> + {
> + if (state == media::power::SystemState::suspend)
> + return;
> +
> + std::lock_guard<std::mutex> lg{cookie_store_guard};
> + if (cookie_store.count(state) > 0)
> + return;
> +
> + object->invoke_method_asynchronously_with_callback<com::canonical::powerd::Interface::requestSysState, std::string>([this, state](const core::dbus::Result<std::string>& result)
> + {
> + if (result.is_error()) // TODO(tvoss): We should log the error condition here.
> + return;
> +
> + std::lock_guard<std::mutex> lg{cookie_store_guard};
> +
> + cookie_store[state] = result.value();
> + signals.acquired(state);
> + }, std::string{wake_lock_name}, static_cast<std::int32_t>(state));
> + }
> +
> + // Informs the system that the caller does not
> + // require the system to stay active anymore.
> + void request_release(media::power::SystemState state) override
> + {
> + if (state == media::power::SystemState::suspend)
> + return;
> +
> + std::lock_guard<std::mutex> lg{cookie_store_guard};
> +
> + if (cookie_store.count(state) == 0)
> + return;
> +
> + object->invoke_method_asynchronously_with_callback<com::canonical::powerd::Interface::clearSysState, void>([this, state](const core::dbus::Result<void>& result)
> + {
> + if (result.is_error()) {/*TODO(tvoss): We should log the error condition here.*/}
> +
> + std::lock_guard<std::mutex> lg{cookie_store_guard};
> +
> + cookie_store.erase(state);
> + signals.released(state);
> + }, cookie_store.at(state));
> + }
> +
> + // Emitted whenever the acquire request completes.
> + const core::Signal<media::power::SystemState>& acquired() const override
> + {
> + return signals.acquired;
> + }
> +
> + // Emitted whenever the release request completes.
> + const core::Signal<media::power::SystemState>& released() const override
> + {
> + return signals.released;
> + }
> +
> + std::mutex cookie_store_guard;
> + std::map<media::power::SystemState, std::string> cookie_store;
> + media::power::StateController::Ptr parent;
> + core::dbus::Object::Ptr object;
> + struct
> + {
> + core::Signal<media::power::SystemState> acquired;
> + core::Signal<media::power::SystemState> released;
> + } signals;
> +};
> +
> +struct StateController : public media::power::StateController,
> + public std::enable_shared_from_this<impl::StateController>
> +{
> + StateController(media::helper::ExternalServices& es)
> + : external_services{es},
> + powerd
> + {
> + core::dbus::Service::use_service<com::canonical::powerd::Interface>(external_services.system)
> + ->object_for_path(com::canonical::powerd::Interface::path())
> + },
> + unity_screen
> + {
> + core::dbus::Service::use_service<com::canonical::Unity::Screen>(external_services.system)
> + ->object_for_path(com::canonical::Unity::Screen::path())
> + }
> + {
> + }
> +
> + media::power::StateController::Lock<media::power::SystemState>::Ptr system_state_lock() override
> + {
> + return std::make_shared<impl::SystemStateLock>(shared_from_this(), powerd);
> + }
> +
> + media::power::StateController::Lock<media::power::DisplayState>::Ptr display_state_lock() override
> + {
> + return std::make_shared<impl::DisplayStateLock>(shared_from_this(), external_services.io_service, unity_screen);
> + }
> +
> + media::helper::ExternalServices& external_services;
> + core::dbus::Object::Ptr powerd;
> + core::dbus::Object::Ptr unity_screen;
> +};
> +}
> +}
> +
> +media::power::StateController::Ptr media::power::make_platform_default_state_controller(core::ubuntu::media::helper::ExternalServices& external_services)
> +{
> + return std::make_shared<impl::StateController>(external_services);
> +}
>
> === added file 'src/core/media/power/state_controller.h'
> --- src/core/media/power/state_controller.h 1970-01-01 00:00:00 +0000
> +++ src/core/media/power/state_controller.h 2014-11-26 12:42:47 +0000
> @@ -0,0 +1,102 @@
> +/*
> + * 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_POWER_STATE_CONTROLLER_H_
> +#define CORE_UBUNTU_MEDIA_POWER_STATE_CONTROLLER_H_
> +
> +#include <core/media/external_services.h>
> +
> +#include <core/property.h>
> +
> +#include <memory>
> +
> +namespace core
> +{
> +namespace ubuntu
> +{
> +namespace media
> +{
> +namespace power
> +{
> +// Enumerates all known power states of a display.
> +enum class DisplayState
> +{
> + // The display is off.
> + off = 0,
> + // The display is on.
> + on = 1
> +};
> +
> +// Enumerates known power states of the system.
> +enum class SystemState
> +{
> + // Note that callers will be notified of suspend state changes
> + // but may not request this state.
> + suspend = 0,
> + // The Active state will prevent system suspend
> + active = 1,
> + // Substate of Active with disabled proximity based blanking
> + blank_on_proximity = 2
> +};
> +
> +// Interface that enables observation of the system power state.
> +struct StateController
> +{
> + // To safe us some typing.
> + typedef std::shared_ptr<StateController> Ptr;
> +
> + // When acquired, ensures that the system stays active,
> + // and decreases the reference count when released.
> + template<typename State>
> + struct Lock
> + {
> + // To safe us some typing.
> + typedef std::shared_ptr<Lock> Ptr;
> +
> + Lock() = default;
> + virtual ~Lock() = default;
> +
> + // Informs the system that the caller would like
> + // the system to stay active.
> + virtual void request_acquire(State state) = 0;
> + // Informs the system that the caller does not
> + // require the system to stay active anymore.
> + virtual void request_release(State state) = 0;
> +
> + // Emitted whenever the acquire request completes.
> + virtual const core::Signal<State>& acquired() const = 0;
> + // Emitted whenever the release request completes.
> + virtual const core::Signal<State>& released() const = 0;
> + };
> +
> + StateController() = default;
> + virtual ~StateController() = default;
> +
> + // Returns a power::StateController::Lock<DisplayState> instance.
> + virtual Lock<DisplayState>::Ptr display_state_lock() = 0;
> + // Returns a power::StateController::Lock<SystemState> instance.
> + virtual Lock<SystemState>::Ptr system_state_lock() = 0;
> +};
> +
> +// Creates a StateController instance that connects to the platform default
> +// services to control system and display power states.
> +StateController::Ptr make_platform_default_state_controller(core::ubuntu::media::helper::ExternalServices&);
> +}
> +}
> +}
> +}
> +#endif // CORE_UBUNTU_MEDIA_POWER_STATE_CONTROLLER_H_
>
> === removed file 'src/core/media/powerd_service.h'
> --- src/core/media/powerd_service.h 2014-08-08 14:36:29 +0000
> +++ src/core/media/powerd_service.h 1970-01-01 00:00:00 +0000
> @@ -1,73 +0,0 @@
> -/*
> - * Copyright (C) 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/>.
> - *
> - * Author: Ricardo Mendoza <ricardo.mendoza at canonical.com>
> - */
> -
> -#include <core/dbus/dbus.h>
> -#include <core/dbus/fixture.h>
> -#include <core/dbus/object.h>
> -#include <core/dbus/property.h>
> -#include <core/dbus/service.h>
> -#include <core/dbus/interfaces/properties.h>
> -#include <core/dbus/types/stl/tuple.h>
> -#include <core/dbus/types/stl/vector.h>
> -
> -#include <core/dbus/asio/executor.h>
> -
> -#include <string>
> -#include <vector>
> -#include <chrono>
> -
> -namespace core
> -{
> -
> -struct Powerd
> -{
> - static std::string& name()
> - {
> - static std::string s = "com.canonical.powerd";
> - return s;
> - }
> -
> - struct requestSysState
> - {
> - static std::string name()
> - {
> - static std::string s = "requestSysState";
> - return s;
> - }
> -
> - static const std::chrono::milliseconds default_timeout() { return std::chrono::seconds{1}; }
> -
> - typedef Powerd Interface;
> - };
> -
> - struct clearSysState
> - {
> - static std::string name()
> - {
> - static std::string s = "clearSysState";
> - return s;
> - }
> -
> - static const std::chrono::milliseconds default_timeout() { return std::chrono::seconds{1}; }
> -
> - typedef Powerd Interface;
> - };
> -
> -};
> -
> -}
>
> === modified file 'src/core/media/server/server.cpp'
> --- src/core/media/server/server.cpp 2014-04-04 14:31:43 +0000
> +++ src/core/media/server/server.cpp 2014-11-26 12:42:47 +0000
> @@ -20,24 +20,127 @@
> #include <core/media/player.h>
> #include <core/media/track_list.h>
>
> -#include <hybris/media/media_codec_layer.h>
> -
> #include "core/media/service_implementation.h"
>
> +#include <core/posix/signal.h>
> +
> #include <iostream>
>
> namespace media = core::ubuntu::media;
>
> using namespace std;
>
> +#if defined(MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER)
> +#include <hybris/media/media_codec_layer.h>
> +
> +namespace
> +{
> +// All platform-specific initialization routines go here.
> +void platform_init()
> +{
> + decoding_service_init();
> +}
> +}
> +#else // MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER
> +namespace
> +{
> +// All platform-specific initialization routines go here.
> +void platform_init()
> +{
> + // Consciously left empty
> +}
> +}
> +#endif // MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER
> +
> int main()
> {
> - // Init hybris-level DecodingService
> - decoding_service_init();
> - cout << "Starting DecodingService..." << endl;
> -
> - auto service = std::make_shared<media::ServiceImplementation>();
> - service->run();
> + auto trap = core::posix::trap_signals_for_all_subsequent_threads(
> + {
> + core::posix::Signal::sig_int,
> + core::posix::Signal::sig_term
> + });
> +
> + trap->signal_raised().connect([trap](core::posix::Signal)
> + {
> + trap->stop();
> + });
> +
> + // Init platform-specific functionality.
> + platform_init();
> +
> + // We keep track of our state.
> + bool shutdown_requested{false};
> +
> + // Our helper for connecting to external services.
> + core::ubuntu::media::helper::ExternalServices external_services;
> +
> + std::thread external_services_worker
> + {
> + [&shutdown_requested, &external_services]()
> + {
> + while (not shutdown_requested)
> + {
> + try
> + {
> + external_services.io_service.run();
> + }
> + catch (const std::exception& e)
> + {
> + std::cerr << "Error while executing the underlying io_service: " << e.what() << std::endl;
> + }
> + catch (...)
> + {
> + std::cerr << "Error while executing the underlying io_service." << std::endl;
> + }
> + }
> + }
> + };
> +
> + // We assemble the configuration for executing the service now.
> + media::ServiceImplementation::Configuration service_config
> + {
> + external_services
> + };
> +
> + auto service = std::make_shared<media::ServiceImplementation>(service_config);
> +
> + std::thread service_worker
> + {
> + [&shutdown_requested, service]()
> + {
> + while (not shutdown_requested)
> + {
> + try
> + {
> + service->run();
> + }
> + catch (const std::exception& e)
> + {
> + std::cerr << "Recoverable error while executing the service: " << e.what() << std::endl;
> + }
> + catch (...)
> + {
> + std::cerr << "Recoverable error while executing the service." << std::endl;
> + }
> + }
> + }
> + };
> +
> + // We block on waiting for signals telling us to gracefully shutdown.
> + trap->run();
> +
> + // Inform our workers that we should shutdown gracefully
> + shutdown_requested = true;
> +
> + // And stop execution of helper and actual service.
> + external_services.stop();
> + service->stop();
> +
> + if (external_services_worker.joinable())
I split out the stopping and adjusted the ordering.
> + external_services_worker.join();
> +
> + if (service_worker.joinable())
> + service_worker.join();
>
> return 0;
> }
>
> === modified file 'src/core/media/service_implementation.cpp'
> --- src/core/media/service_implementation.cpp 2014-11-26 12:42:47 +0000
> +++ src/core/media/service_implementation.cpp 2014-11-26 12:42:47 +0000
> @@ -22,6 +22,7 @@
>
> #include "service_implementation.h"
>
> +#include "client_death_observer.h"
> #include "indicator_power_service.h"
> #include "call-monitor/call_monitor.h"
> #include "player_configuration.h"
> @@ -40,7 +41,6 @@
> #include <pulse/pulseaudio.h>
>
> #include "util/timeout.h"
> -#include "unity_screen_service.h"
>
> namespace media = core::ubuntu::media;
>
> @@ -48,10 +48,13 @@
>
> struct media::ServiceImplementation::Private
> {
> - Private()
> - : resume_key(std::numeric_limits<std::uint32_t>::max()),
> - keep_alive(io_service),
> - disp_cookie(0),
> + Private(const ServiceImplementation::Configuration& configuration)
> + : configuration(configuration),
> + resume_key(std::numeric_limits<std::uint32_t>::max()),
> + power_state_controller(media::power::make_platform_default_state_controller(configuration.external_services)),
> + display_state_lock(power_state_controller->display_state_lock()),
> + client_death_observer(media::platform_default_client_death_observer()),
> + recorder_observer(media::make_platform_default_recorder_observer()),
> pulse_mainloop_api(nullptr),
> pulse_context(nullptr),
> headphones_connected(false),
> @@ -59,13 +62,6 @@
> primary_idx(-1),
> call_monitor(new CallMonitor)
> {
> - bus = std::shared_ptr<dbus::Bus>(new dbus::Bus(core::dbus::WellKnownBus::session));
> - bus->install_executor(dbus::asio::make_executor(bus, io_service));
> - worker = std::move(std::thread([this]()
> - {
> - bus->run();
> - }));
> -
> // Spawn pulse watchdog
> pulse_mainloop = nullptr;
> pulse_worker = std::move(std::thread([this]()
> @@ -129,19 +125,11 @@
>
> // Connect the property change signal that will allow media-hub to take appropriate action
> // when the battery level reaches critical
> - auto stub_service = dbus::Service::use_service(bus, "com.canonical.indicator.power");
> + auto stub_service = dbus::Service::use_service(configuration.external_services.session, "com.canonical.indicator.power");
> indicator_power_session = stub_service->object_for_path(dbus::types::ObjectPath("/com/canonical/indicator/power/Battery"));
> power_level = indicator_power_session->get_property<core::IndicatorPower::PowerLevel>();
> is_warning = indicator_power_session->get_property<core::IndicatorPower::IsWarning>();
>
> - // Obtain session with Unity.Screen so that we request state when doing recording
> - auto bus = std::shared_ptr<dbus::Bus>(new dbus::Bus(core::dbus::WellKnownBus::system));
> - bus->install_executor(dbus::asio::make_executor(bus));
> -
> - auto uscreen_stub_service = dbus::Service::use_service(bus, dbus::traits::Service<core::UScreen>::interface_name());
> - uscreen_session = uscreen_stub_service->object_for_path(dbus::types::ObjectPath("/com/canonical/Unity/Screen"));
> -
> - recorder_observer = media::make_platform_default_recorder_observer();
> recorder_observer->recording_state().changed().connect([this](media::RecordingState state)
> {
> media_recording_state_changed(state);
> @@ -159,40 +147,16 @@
> pulse_mainloop = nullptr;
> }
>
> - bus->stop();
> -
> - if (worker.joinable())
> - worker.join();
> -
> if (pulse_worker.joinable())
> pulse_worker.join();
> }
>
> void media_recording_state_changed(media::RecordingState state)
> {
> - if (uscreen_session == nullptr)
> - return;
> -
> if (state == media::RecordingState::started)
> - {
> - if (disp_cookie > 0)
> - return;
> -
> - auto result = uscreen_session->invoke_method_synchronously<core::UScreen::keepDisplayOn, int>();
> - if (result.is_error())
> - throw std::runtime_error(result.error().print());
> - disp_cookie = result.value();
> - }
> + display_state_lock->request_acquire(media::power::DisplayState::on);
> else if (state == media::RecordingState::stopped)
> - {
> - if (disp_cookie != -1)
> - {
> - timeout(4000, true, [this](){
> - this->uscreen_session->invoke_method_synchronously<core::UScreen::removeDisplayOnRequest, void>(this->disp_cookie);
> - this->disp_cookie = -1;
> - });
> - }
> - }
> + display_state_lock->request_release(media::power::DisplayState::off);
> }
>
> pa_threaded_mainloop *mainloop()
> @@ -438,18 +402,16 @@
> }
> }
>
> + 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;
> - std::thread worker;
> - dbus::Bus::Ptr bus;
> - boost::asio::io_service io_service;
> - boost::asio::io_service::work keep_alive;
> + media::Player::PlayerKey resume_key;
> std::shared_ptr<dbus::Object> indicator_power_session;
> std::shared_ptr<core::dbus::Property<core::IndicatorPower::PowerLevel>> power_level;
> std::shared_ptr<core::dbus::Property<core::IndicatorPower::IsWarning>> is_warning;
> - int disp_cookie;
> - std::shared_ptr<dbus::Object> uscreen_session;
> + media::power::StateController::Ptr power_state_controller;
> + media::power::StateController::Lock<media::power::DisplayState>::Ptr display_state_lock;
> + media::ClientDeathObserver::Ptr client_death_observer;
> media::RecorderObserver::Ptr recorder_observer;
> // Pulse-specific
> pa_mainloop_api *pulse_mainloop_api;
> @@ -470,7 +432,7 @@
> std::list<media::Player::PlayerKey> paused_sessions;
> };
>
> -media::ServiceImplementation::ServiceImplementation() : d(new Private())
> +media::ServiceImplementation::ServiceImplementation(const Configuration& configuration) : d(new Private(configuration))
> {
> d->power_level->changed().connect([this](const core::IndicatorPower::PowerLevel::ValueType &level)
> {
> @@ -513,8 +475,16 @@
> std::shared_ptr<media::Player> media::ServiceImplementation::create_session(
> const media::Player::Configuration& conf)
> {
> - auto player = std::make_shared<media::PlayerImplementation>(
> - conf.identity, conf.bus, conf.session, shared_from_this(), conf.key);
> + auto player = std::make_shared<media::PlayerImplementation>(media::PlayerImplementation::Configuration
> + {
> + conf.identity,
> + conf.bus,
> + conf.session,
> + shared_from_this(),
> + conf.key,
> + d->client_death_observer,
> + d->power_state_controller
> + });
>
> auto key = conf.key;
> player->on_client_disconnected().connect([this, key]()
> @@ -524,7 +494,7 @@
> // remove_player_for_key can destroy the player instance which in turn
> // destroys the "on_client_disconnected" signal whose destructor will wait
> // until all dispatches are done
> - d->io_service.post([this, key]()
> + d->configuration.external_services.io_service.post([this, key]()
> {
> remove_player_for_key(key);
> });
>
> === modified file 'src/core/media/service_implementation.h'
> --- src/core/media/service_implementation.h 2014-10-31 07:49:33 +0000
> +++ src/core/media/service_implementation.h 2014-11-26 12:42:47 +0000
> @@ -20,6 +20,7 @@
> #define CORE_UBUNTU_MEDIA_SERVICE_IMPLEMENTATION_H_
>
> #include "service_skeleton.h"
> +#include "external_services.h"
>
> namespace core
> {
> @@ -27,13 +28,18 @@
> {
> namespace media
> {
> -
> class Player;
>
> class ServiceImplementation : public ServiceSkeleton
> {
> public:
> - ServiceImplementation ();
> + // All creation time arguments go here.
> + struct Configuration
> + {
> + helper::ExternalServices& external_services;
> + };
> +
> + ServiceImplementation (const Configuration& configuration);
> ~ServiceImplementation ();
>
> std::shared_ptr<Player> create_session(const Player::Configuration&);
>
> === removed file 'src/core/media/unity_screen_service.h'
> --- src/core/media/unity_screen_service.h 2014-06-16 22:28:53 +0000
> +++ src/core/media/unity_screen_service.h 1970-01-01 00:00:00 +0000
> @@ -1,72 +0,0 @@
> -/*
> - * Copyright (C) 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/>.
> - *
> - * Author: Alberto Aguirre <alberto.aguirre at canonical.com>
> - */
> -
> -#include <core/dbus/dbus.h>
> -#include <core/dbus/fixture.h>
> -#include <core/dbus/object.h>
> -#include <core/dbus/property.h>
> -#include <core/dbus/service.h>
> -#include <core/dbus/interfaces/properties.h>
> -#include <core/dbus/types/stl/tuple.h>
> -#include <core/dbus/types/stl/vector.h>
> -
> -#include <core/dbus/asio/executor.h>
> -
> -#include <string>
> -#include <vector>
> -#include <chrono>
> -
> -namespace core
> -{
> -
> -struct UScreen
> -{
> - static std::string& name()
> - {
> - static std::string s = "com.canonical.Unity.Screen";
> - return s;
> - }
> -
> - struct keepDisplayOn
> - {
> - static std::string name()
> - {
> - static std::string s = "keepDisplayOn";
> - return s;
> - }
> -
> - static const std::chrono::milliseconds default_timeout() { return std::chrono::seconds{1}; }
> -
> - typedef UScreen Interface;
> - };
> -
> - struct removeDisplayOnRequest
> - {
> - static std::string name()
> - {
> - static std::string s = "removeDisplayOnRequest";
> - return s;
> - }
> -
> - static const std::chrono::milliseconds default_timeout() { return std::chrono::seconds{1}; }
> -
> - typedef UScreen Interface;
> - };
> -};
> -
> -}
>
> === modified file 'tests/CMakeLists.txt'
> --- tests/CMakeLists.txt 2014-02-14 08:12:35 +0000
> +++ tests/CMakeLists.txt 2014-11-26 12:42:47 +0000
> @@ -50,5 +50,5 @@
> "COM_UBUNTU_MEDIA_SERVICE_AUDIO_SINK_NAME=fakesink"
> )
>
> -add_subdirectory(acceptance-tests)
> +# add_subdirectory(acceptance-tests)
> add_subdirectory(unit-tests)
>
--
https://code.launchpad.net/~thomas-voss/media-hub/introduce-power-controller-interface/+merge/242905
Your team Ubuntu Phablet Team is subscribed to branch lp:media-hub.
More information about the Ubuntu-reviews
mailing list