[Merge] lp:~karni/media-hub/media-hub-engine-refactor into lp:media-hub/stable

Jim Hodapp jim.hodapp at canonical.com
Tue Feb 23 17:02:22 UTC 2016


Review: Needs Fixing code

Looks fantastic Karni, good work so far! Several change suggestions inline below.

Diff comments:

> 
> === added file 'src/core/media/engine/codec.h'
> --- src/core/media/engine/codec.h	1970-01-01 00:00:00 +0000
> +++ src/core/media/engine/codec.h	2016-01-27 19:53:39 +0000
> @@ -0,0 +1,126 @@
> +/*
> + * Copyright © 2016 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: Michal Karnicki <michal.karnicki at canonical.com>
> + */
> +
> +#pragma once

Even though this is completely valid, please use the older technique of #ifndef CODEC_H_ #define CODEC_H_ just so we're consistent throughout the codebase.

> +
> +// TODO karni: replace following with <core/media/engine.h> once engine.h moved to include/core/media/
> +#include "engine.h"
> +
> +#include <core/dbus/codec.h>

This is an internal include to the project, so use "" instead of <>

> +
> +namespace core
> +{
> +namespace dbus
> +{
> +namespace helper
> +{
> +
> +template<>
> +struct TypeMapper<core::ubuntu::media::Engine::Volume>
> +{
> +    constexpr static ArgumentType type_value()
> +    {
> +        return ArgumentType::floating_point;
> +    }
> +    constexpr static bool is_basic_type()
> +    {
> +        return false;
> +    }
> +    constexpr static bool requires_signature()
> +    {
> +        return false;
> +    }
> +
> +    static std::string signature()
> +    {
> +        static const std::string s = TypeMapper<double>::signature();
> +        return s;
> +    }
> +};
> +}
> +
> +template<>
> +struct Codec<core::ubuntu::media::Engine::Volume>
> +{
> +    static void encode_argument(core::dbus::Message::Writer& out, const core::ubuntu::media::Engine::Volume& in)
> +    {
> +        out.push_floating_point(in.value);
> +    }
> +
> +    static void decode_argument(core::dbus::Message::Reader& out, core::ubuntu::media::Engine::Volume& in)
> +    {
> +        in = core::ubuntu::media::Engine::Volume(out.pop_floating_point());
> +    }
> +};
> +
> +namespace helper
> +{
> +template<>
> +struct TypeMapper<core::ubuntu::media::Engine::State>
> +{
> +    constexpr static ArgumentType type_value()
> +    {
> +        return ArgumentType::int16;
> +    }
> +    constexpr static bool is_basic_type()
> +    {
> +        return false;
> +    }
> +    constexpr static bool requires_signature()
> +    {
> +        return false;
> +    }
> +
> +    static std::string signature()
> +    {
> +        static const std::string s = TypeMapper<int16_t>::signature();
> +        return s;
> +    }
> +};
> +}
> +
> +template<>
> +struct Codec<core::ubuntu::media::Engine::State>
> +{
> +    static void encode_argument(core::dbus::Message::Writer& out, const core::ubuntu::media::Engine::State& in)
> +    {
> +        out.push_int16(static_cast<std::int16_t>(in));
> +    }
> +
> +    static void decode_argument(core::dbus::Message::Reader& out, core::ubuntu::media::Engine::State& in)
> +    {
> +        in = static_cast<core::ubuntu::media::Engine::State>(out.pop_int16());
> +    }
> +};
> +
> +template<>
> +struct Codec<std::chrono::duration<long, std::micro > >
> +{
> +    static void encode_argument(core::dbus::Message::Writer& out, const std::chrono::duration<long, std::micro >& in)
> +    {
> +        out.push_int64(static_cast<std::int64_t>(in.count()));
> +    }
> +
> +    static void decode_argument(core::dbus::Message::Reader& out, std::chrono::duration<long, std::micro >& in)
> +    {
> +        in = std::chrono::microseconds{out.pop_int64()};
> +    }
> +};
> +
> +}
> +}
> 
> === added file 'src/core/media/engine/engine.h'
> --- src/core/media/engine/engine.h	1970-01-01 00:00:00 +0000
> +++ src/core/media/engine/engine.h	2016-01-27 19:53:39 +0000
> @@ -0,0 +1,95 @@
> +/*
> + * Copyright © 2016 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: Michal Karnicki <michal.karnicki at canonical.com>
> + */
> +
> +#pragma once

Also change here, see above comment about #pragma once.

> +
> +// XXX karni: core/media/engine.h will be moved to include/core/media/dbus/engine.h
> +#include "../engine.h"
> +#include <core/media/player.h>

Use ""

> +#include <core/media/track.h>

Use ""

> +
> +#include <core/dbus/bus.h>
> +#include <core/dbus/macros.h>
> +#include <core/dbus/object.h>
> +#include <core/dbus/skeleton.h>
> +#include <core/dbus/stub.h>
> +
> +#include <core/property.h>
> +
> +#include <memory>
> +
> +namespace core
> +{
> +namespace media
> +{
> +namespace engine
> +{
> +
> +namespace engine = core::media::engine;
> +namespace media = core::ubuntu::media;
> +
> +struct Engine
> +{
> +    typedef engine::Engine Interface;
> +
> +    static const std::string& name()
> +    {
> +        static const std::string s{"core.ubuntu.media.Service.Engine"};
> +        return s;
> +    }
> +
> +    DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(OpenResourceForUri, Engine, 5000)
> +    DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(OpenResourceForUriExtended, Engine, 5000)
> +    DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(CreateVideoSink, Engine, 5000)
> +    DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(Play, Engine, 5000)
> +    DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(Stop, Engine, 5000)
> +    DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(Pause, Engine, 5000)
> +    DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(SeekTo, Engine, 5000)
> +    DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(Reset, Engine, 5000)
> +
> +    struct Properties
> +    {
> +        typedef std::tuple<media::Track::UriType, media::Track::MetaData> MetaDataTuple;
> +
> +        DBUS_CPP_READABLE_PROPERTY_DEF(State, Engine, media::Engine::State)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(IsVideoSource, Engine, bool)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(IsAudioSource, Engine, bool)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(Position, Engine, uint64_t)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(Duration, Engine, uint64_t)
> +        DBUS_CPP_WRITABLE_PROPERTY_DEF(Volume, Engine, media::Engine::Volume)
> +        DBUS_CPP_WRITABLE_PROPERTY_DEF(AudioStreamRole, Engine, media::Player::AudioStreamRole)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(Orientation, Engine, media::Player::Orientation)
> +        DBUS_CPP_WRITABLE_PROPERTY_DEF(Lifetime, Engine, media::Player::Lifetime)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(TrackMetaData, Engine, MetaDataTuple)
> +    };
> +
> +    struct Signals
> +    {
> +        DBUS_CPP_SIGNAL_DEF(SeekedTo, Engine, uint64_t)
> +        DBUS_CPP_SIGNAL_DEF(AboutToFinish, Engine, void)
> +        DBUS_CPP_SIGNAL_DEF(EndOfStream, Engine, void)
> +        DBUS_CPP_SIGNAL_DEF(ClientDisconnected, Engine, void)
> +        DBUS_CPP_SIGNAL_DEF(PlaybackStatusChanged, Engine, media::Player::PlaybackStatus)
> +        DBUS_CPP_SIGNAL_DEF(VideoDimensionChanged, Engine, media::video::Dimensions)
> +        DBUS_CPP_SIGNAL_DEF(Error, Engine, media::Player::Error)
> +    };
> +};
> +
> +}
> +}
> +}
> 
> === added file 'src/core/media/engine/engine_skeleton.cpp'
> --- src/core/media/engine/engine_skeleton.cpp	1970-01-01 00:00:00 +0000
> +++ src/core/media/engine/engine_skeleton.cpp	2016-01-27 19:53:39 +0000
> @@ -0,0 +1,652 @@
> +/**
> + * Copyright (C) 2016 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: Michal Karnicki <michal.karnicki at canonical.com>
> + */
> +
> +#include "engine_skeleton.h"
> +
> +#include "codec.h"
> +#include "engine_traits.h"
> +#include "../apparmor/ubuntu.h"
> +#include "../codec.h"
> +#include "../engine.h"
> +#include "../property_stub.h"
> +#include "../the_session_bus.h"
> +
> +#include <core/dbus/asio/executor.h>
> +#include <core/dbus/interfaces/properties.h>
> +#include <core/dbus/object.h>
> +#include <core/dbus/property.h>
> +#include <core/dbus/stub.h>
> +#include <core/dbus/types/variant.h>
> +
> +namespace dbus = core::dbus;
> +namespace engine = core::media::engine;
> +namespace media = core::ubuntu::media;
> +
> +namespace
> +{
> +const std::vector<std::string>& the_empty_array_of_invalidated_properties()
> +{
> +    static const std::vector<std::string> v; return v;
> +}
> +}
> +
> +struct engine::EngineSkeleton::Private
> +{
> +    Private(
> +        const std::shared_ptr<media::Engine> engine,
> +        const std::shared_ptr<core::dbus::Bus>& bus,
> +        const std::shared_ptr<core::dbus::Object>& object
> +        ) : impl(engine),
> +            bus(bus),
> +            object(object),
> +            properties_changed(
> +                object->get_signal<core::dbus::interfaces::Properties::Signals::PropertiesChanged>()
> +            ),
> +            properties
> +            {
> +                object->get_property<engine::Engine::Properties::State>(),
> +                object->get_property<engine::Engine::Properties::IsVideoSource>(),
> +                object->get_property<engine::Engine::Properties::IsAudioSource>(),
> +                object->get_property<engine::Engine::Properties::Position>(),
> +                object->get_property<engine::Engine::Properties::Duration>(),
> +                object->get_property<engine::Engine::Properties::Volume>(),
> +                object->get_property<engine::Engine::Properties::AudioStreamRole>(),
> +                object->get_property<engine::Engine::Properties::Orientation>(),
> +                object->get_property<engine::Engine::Properties::Lifetime>(),
> +                object->get_property<engine::Engine::Properties::TrackMetaData>()
> +            },
> +            connections
> +            {
> +                properties.state->changed().connect([this](media::Engine::State value)
> +                {
> +                    on_property_value_changed<engine::Engine::Properties::State>(value);
> +                }),
> +                properties.is_video_source->changed().connect([this](bool value)
> +                {
> +                    on_property_value_changed<engine::Engine::Properties::IsVideoSource>(value);
> +                }),
> +                properties.is_audio_source->changed().connect([this](bool value)
> +                {
> +                    on_property_value_changed<engine::Engine::Properties::IsAudioSource>(value);
> +                }),
> +                properties.position->changed().connect([this](uint64_t value)
> +                {
> +                    on_property_value_changed<engine::Engine::Properties::Position>(value);
> +                }),
> +                properties.duration->changed().connect([this](uint64_t value)
> +                {
> +                    on_property_value_changed<engine::Engine::Properties::Duration>(value);
> +                }),
> +                properties.volume->changed().connect([this](Volume value)
> +                {
> +                    impl->volume().set(value);
> +                }),
> +                properties.audio_stream_role->changed().connect([this](media::Player::AudioStreamRole role)
> +                {
> +                    impl->audio_stream_role().set(role);
> +                }),
> +                properties.orientation->changed().connect([this](media::Player::Orientation value)
> +                {
> +                    on_property_value_changed<engine::Engine::Properties::Orientation>(value);
> +                }),
> +                properties.lifetime->changed().connect([this](media::Player::Lifetime value)
> +                {
> +                    std::cout << "lifetime, not working" << std::endl;
> +                    impl->lifetime().set(value);
> +                }),
> +                properties.track_meta_data->changed().connect([this](std::tuple<media::Track::UriType, media::Track::MetaData> value)

Is there a reason why you're not passing in the tuple by reference instead of by value (i.e. a copy)? : std::tuple<media::Track::UriType, media::Track::MetaData> value

> +                {
> +                    std::cout << "track_meta_data, not working" << std::endl;
> +                    on_property_value_changed<engine::Engine::Properties::TrackMetaData>(value);
> +                })
> +            },
> +            signals
> +            {
> +                object->get_signal<engine::Engine::Signals::AboutToFinish>(),
> +                object->get_signal<engine::Engine::Signals::SeekedTo>(),
> +                object->get_signal<engine::Engine::Signals::EndOfStream>(),
> +                object->get_signal<engine::Engine::Signals::ClientDisconnected>(),
> +                object->get_signal<engine::Engine::Signals::PlaybackStatusChanged>(),
> +                object->get_signal<engine::Engine::Signals::VideoDimensionChanged>(),
> +                object->get_signal<engine::Engine::Signals::Error>()
> +            }
> +    {
> +        // Hook-up properties from engine implementation to the skeleton.
> +
> +        impl->state().changed().connect([this](media::Engine::State value)
> +        {
> +            // (*properties.state.get()).set(state);
> +            on_property_value_changed<engine::Engine::Properties::State>(value);
> +        });
> +        impl->is_video_source().changed().connect([this](bool value)
> +        {
> +            on_property_value_changed<engine::Engine::Properties::IsVideoSource>(value);
> +        });
> +        impl->is_audio_source().changed().connect([this](bool value)
> +        {
> +            on_property_value_changed<engine::Engine::Properties::IsAudioSource>(value);
> +        });
> +        impl->position().changed().connect([this](uint64_t value)
> +        {
> +            on_property_value_changed<engine::Engine::Properties::Position>(value);
> +        });
> +        impl->duration().changed().connect([this](uint64_t value)
> +        {
> +            on_property_value_changed<engine::Engine::Properties::Duration>(value);
> +        });
> +        impl->orientation().changed().connect([this](media::Player::Orientation value)
> +        {
> +            on_property_value_changed<engine::Engine::Properties::Orientation>(value);
> +        });
> +        impl->track_meta_data().changed().connect([this](std::tuple<media::Track::UriType, media::Track::MetaData> value)

Same here, see above comment at line 440.

> +        {
> +            on_property_value_changed<engine::Engine::Properties::TrackMetaData>(value);
> +        });
> +
> +        // Hook-up signals from engine implementation to the skeleton.
> +
> +        impl->about_to_finish_signal().connect([this](){
> +            signals.about_to_finish();
> +        });
> +        impl->seeked_to_signal().connect([this](uint64_t ts){
> +            signals.seeked_to(ts);
> +        });
> +        impl->client_disconnected_signal().connect([this](){
> +            signals.client_disconnected();
> +        });
> +        impl->end_of_stream_signal().connect([this](){
> +            signals.end_of_stream();
> +        });
> +        impl->playback_status_changed_signal().connect([this](media::Player::PlaybackStatus status){
> +            signals.playback_status_changed(status);
> +        });
> +        impl->video_dimension_changed_signal().connect([this](media::video::Dimensions dimensions){

Same as line 440.

> +            signals.video_dimension_changed(dimensions);
> +        });
> +        impl->error_signal().connect([this](media::Player::Error error){
> +            signals.error(error);
> +        });
> +    }
> +
> +    void handle_open_resource_for_uri(const core::dbus::Message::Ptr& msg) const {
> +        try {
> +            core::ubuntu::media::Track::UriType uri;
> +            bool do_pipeline_reset{false};
> +            msg->reader() >> uri >> do_pipeline_reset;
> +            bool result = impl->open_resource_for_uri(uri, do_pipeline_reset);
> +            auto reply = core::dbus::Message::make_method_return(msg);
> +            reply->writer() << result;
> +            bus->send(reply);
> +        }
> +        catch (...)
> +        {
> +            // TODO karni: Find appropriate dbus error title, as this will throw as well!

Take a look at what I did for the UriNotFound that just landed into trunk. You can do something similar and piggy back off of what I did. If you add a new error type, make sure to catch it and do something useful with it in qtubuntu-media.

Also, which exceptions are you trying to catch here? I see that you're trying to catch them all, but are there any specific ones that you could catch that would provide better specific context for the error that you send over dbus?

> +            auto error = core::dbus::Message::make_error(msg, "Error", "Invocation of OpenResourceForUri failed");
> +            bus->send(error);
> +            return;
> +        }
> +    }
> +
> +    void handle_create_video_sink(const core::dbus::Message::Ptr& msg) const {
> +        try {
> +            std::uint32_t texture_id;
> +            msg->reader() >> texture_id;
> +            impl->create_video_sink(texture_id);
> +            auto reply = core::dbus::Message::make_method_return(msg);
> +            bus->send(reply);
> +        }
> +        catch (...)
> +        {
> +            auto error = core::dbus::Message::make_error(msg, "Error", "Invocation of CreateVideoSink failed");

See the comment on line 526.

> +            bus->send(error);
> +            return;
> +        }
> +    }
> +
> +    void handle_play(const core::dbus::Message::Ptr& msg) const
> +    {
> +        try
> +        {
> +            bool result = impl->play();
> +            auto reply = core::dbus::Message::make_method_return(msg);
> +            reply->writer() << result;
> +            bus->send(reply);
> +        }
> +        catch (...)
> +        {
> +            auto error = core::dbus::Message::make_error(msg, "Error", "Invocation of Play failed");

See the comment on line 526.

> +            bus->send(error);
> +            return;
> +        }
> +    }
> +
> +    void handle_stop(const core::dbus::Message::Ptr& msg)
> +    {
> +        try
> +        {
> +            bool result = impl->stop();
> +            auto reply = core::dbus::Message::make_method_return(msg);
> +            reply->writer() << result;
> +            bus->send(reply);
> +        }
> +        catch (...)
> +        {
> +            auto error = core::dbus::Message::make_error(msg, "Error", "Invocation of Stop failed");

See the comment on line 526.

> +            bus->send(error);
> +            return;
> +        }
> +    }
> +
> +    void handle_pause(const core::dbus::Message::Ptr& msg)
> +    {
> +        try
> +        {
> +            bool result = impl->pause();
> +            auto reply = core::dbus::Message::make_method_return(msg);
> +            reply->writer() << result;
> +            bus->send(reply);
> +        }
> +        catch (...)
> +        {
> +            auto error = core::dbus::Message::make_error(msg, "Error", "Invocation of Pause failed");

See the comment on line 526.

> +            bus->send(error);
> +            return;
> +        }
> +    }
> +
> +    void handle_seek_to(const core::dbus::Message::Ptr& msg)
> +    {
> +        try
> +        {
> +            std::chrono::microseconds ts;
> +            msg->reader() >> ts;
> +            bool result = impl->seek_to(ts);
> +            auto reply = core::dbus::Message::make_method_return(msg);
> +            reply->writer() << result;
> +            bus->send(reply);
> +        }
> +        catch (...)
> +        {
> +            auto error = core::dbus::Message::make_error(msg, "Error", "Invocation of SeekTo failed");

See the comment on line 526.

> +            bus->send(error);
> +            return;
> +        }
> +    }
> +
> +    void handle_reset(const core::dbus::Message::Ptr& msg)
> +    {
> +        try
> +        {
> +            impl->reset();
> +            auto reply = core::dbus::Message::make_method_return(msg);
> +            bus->send(reply);
> +        }
> +        catch (...)
> +        {
> +            auto error = core::dbus::Message::make_error(msg, "Error", "Invocation of Reset failed");

See the comment on line 526.

> +            bus->send(error);
> +            return;
> +        }
> +    }
> +
> +    template<typename Property>
> +    void on_property_value_changed(const typename Property::ValueType& value)
> +    {
> +        std::map<std::string, core::dbus::types::Variant> dict
> +        {
> +            {
> +                Property::name(),
> +                core::dbus::types::Variant::encode(value)
> +            }
> +        };
> +
> +        properties_changed->emit(
> +                std::make_tuple(
> +                    dbus::traits::Service<engine::Engine>::interface_name(),
> +                    dict,
> +                    the_empty_array_of_invalidated_properties()));
> +    }
> +
> +    const media::Engine::Ptr impl;
> +    const dbus::Bus::Ptr bus;
> +    const dbus::Object::Ptr object;
> +    core::dbus::Signal
> +    <
> +        core::dbus::interfaces::Properties::Signals::PropertiesChanged,
> +        core::dbus::interfaces::Properties::Signals::PropertiesChanged::ArgumentType
> +    >::Ptr properties_changed;
> +
> +    // TODO karni: Our functional dependencies.
> +    // apparmor::ubuntu::RequestContextResolver::Ptr request_context_resolver;

Do you just not have a use for these yet?

> +    // apparmor::ubuntu::RequestAuthenticator::Ptr request_authenticator;
> +
> +    struct
> +    {
> +        std::shared_ptr<core::dbus::Property<engine::Engine::Properties::State>> state;
> +        std::shared_ptr<core::dbus::Property<engine::Engine::Properties::IsVideoSource>> is_video_source;
> +        std::shared_ptr<core::dbus::Property<engine::Engine::Properties::IsAudioSource>> is_audio_source;
> +        std::shared_ptr<core::dbus::Property<engine::Engine::Properties::Position>> position;
> +        std::shared_ptr<core::dbus::Property<engine::Engine::Properties::Duration>> duration;
> +        std::shared_ptr<core::dbus::Property<engine::Engine::Properties::Volume>> volume;
> +        std::shared_ptr<core::dbus::Property<engine::Engine::Properties::AudioStreamRole>> audio_stream_role;
> +        std::shared_ptr<core::dbus::Property<engine::Engine::Properties::Orientation>> orientation;
> +        std::shared_ptr<core::dbus::Property<engine::Engine::Properties::Lifetime>> lifetime;
> +        std::shared_ptr<core::dbus::Property<engine::Engine::Properties::TrackMetaData>> track_meta_data;
> +    } properties;
> +
> +    struct
> +    {
> +        core::ScopedConnection state;
> +        core::ScopedConnection is_video_source;
> +        core::ScopedConnection is_audio_source;
> +        core::ScopedConnection position;
> +        core::ScopedConnection duration;
> +        core::ScopedConnection volume;
> +        core::ScopedConnection audio_stream_role;
> +        core::ScopedConnection orientation;
> +        core::ScopedConnection lifetime;
> +        core::ScopedConnection track_meta_data;
> +    } connections;
> +
> +    struct Signals
> +    {
> +        typedef core::dbus::Signal<engine::Engine::Signals::AboutToFinish, engine::Engine::Signals::AboutToFinish::ArgumentType> DBusAboutToFinishSignal;
> +        typedef core::dbus::Signal<engine::Engine::Signals::SeekedTo, engine::Engine::Signals::SeekedTo::ArgumentType> DBusSeekedToSignal;
> +        typedef core::dbus::Signal<engine::Engine::Signals::EndOfStream, engine::Engine::Signals::EndOfStream::ArgumentType> DBusEndOfStreamSignal;
> +        typedef core::dbus::Signal<engine::Engine::Signals::ClientDisconnected, engine::Engine::Signals::ClientDisconnected::ArgumentType> DBusClientDisconnectedSignal;
> +        typedef core::dbus::Signal<engine::Engine::Signals::PlaybackStatusChanged, engine::Engine::Signals::PlaybackStatusChanged::ArgumentType> DBusPlaybackStatusChangedSignal;
> +        typedef core::dbus::Signal<engine::Engine::Signals::VideoDimensionChanged, engine::Engine::Signals::VideoDimensionChanged::ArgumentType> DBusVideoDimensionChangedSignal;
> +        typedef core::dbus::Signal<engine::Engine::Signals::Error, engine::Engine::Signals::Error::ArgumentType> DBusErrorSignal;
> +
> +        Signals(const std::shared_ptr<DBusAboutToFinishSignal>& remote_about_to_finish,
> +                const std::shared_ptr<DBusSeekedToSignal>& remote_seeked_to,
> +                const std::shared_ptr<DBusEndOfStreamSignal>& remote_end_of_stream,
> +                const std::shared_ptr<DBusClientDisconnectedSignal>& remote_client_disconnected,
> +                const std::shared_ptr<DBusPlaybackStatusChangedSignal>& remote_playback_status_changed,
> +                const std::shared_ptr<DBusVideoDimensionChangedSignal>& remote_video_dimension_changed,
> +                const std::shared_ptr<DBusErrorSignal>& remote_error)
> +        {
> +            about_to_finish.connect([remote_about_to_finish]()
> +            {
> +                remote_about_to_finish->emit();
> +            });
> +
> +            seeked_to.connect([remote_seeked_to](std::uint64_t value)
> +            {
> +                remote_seeked_to->emit(value);
> +            });
> +
> +            end_of_stream.connect([remote_end_of_stream]()
> +            {
> +                remote_end_of_stream->emit();
> +            });
> +
> +            client_disconnected.connect([remote_client_disconnected]()
> +            {
> +                remote_client_disconnected->emit();
> +            });
> +
> +            playback_status_changed.connect([remote_playback_status_changed](const media::Player::PlaybackStatus& status)
> +            {
> +                remote_playback_status_changed->emit(status);
> +            });
> +
> +            video_dimension_changed.connect([remote_video_dimension_changed](const media::video::Dimensions& dimensions)
> +            {
> +                remote_video_dimension_changed->emit(dimensions);
> +            });
> +
> +            error.connect([remote_error](const media::Player::Error& e)
> +            {
> +                remote_error->emit(e);
> +            });
> +        }
> +
> +        core::Signal<void> about_to_finish;
> +        core::Signal<uint64_t> seeked_to;
> +        core::Signal<void> end_of_stream;
> +        core::Signal<void> client_disconnected;
> +        core::Signal<media::Player::PlaybackStatus> playback_status_changed;
> +        core::Signal<media::video::Dimensions> video_dimension_changed;
> +        core::Signal<media::Player::Error> error;
> +    } signals;
> +};
> +
> +engine::EngineSkeleton::EngineSkeleton(
> +    const std::shared_ptr<media::Engine>& engine,
> +    const std::shared_ptr<core::dbus::Bus>& bus,
> +    const std::shared_ptr<core::dbus::Object>& object
> +    ) : d(new Private(engine, bus, object)) // this instead of engine?

Where is the engine instance coming from? In theory passing this or engine would be the same, but if they're not then you still need to pass in engine to this constructor and pass it down to Private.

> +{
> +    auto open_resource_for_uri = std::bind(&Private::handle_open_resource_for_uri, d, std::placeholders::_1);

Just a note that I'll only add once, but auto does not automatically bring const with it. So if you mean for any of these to be const, which I believe you do, add const auto.

> +    d->object->install_method_handler<engine::Engine::OpenResourceForUri>(open_resource_for_uri);
> +
> +    auto create_video_sink = std::bind(&Private::handle_create_video_sink, d, std::placeholders::_1);
> +    d->object->install_method_handler<engine::Engine::CreateVideoSink>(create_video_sink);
> +
> +    auto play = std::bind(&Private::handle_play, d, std::placeholders::_1);
> +    d->object->install_method_handler<engine::Engine::Play>(play);
> +
> +    auto stop = std::bind(&Private::handle_stop, d, std::placeholders::_1);
> +    d->object->install_method_handler<engine::Engine::Stop>(stop);
> +
> +    auto pause = std::bind(&Private::handle_pause, d, std::placeholders::_1);
> +    d->object->install_method_handler<engine::Engine::Pause>(pause);
> +
> +    auto seek_to = std::bind(&Private::handle_seek_to, d, std::placeholders::_1);
> +    d->object->install_method_handler<engine::Engine::SeekTo>(seek_to);
> +
> +    auto reset = std::bind(&Private::handle_reset, d, std::placeholders::_1);
> +    d->object->install_method_handler<engine::Engine::Reset>(reset);
> +}
> +
> +engine::EngineSkeleton::~EngineSkeleton()
> +{
> +    d->object->uninstall_method_handler<engine::Engine::OpenResourceForUri>();
> +    d->object->uninstall_method_handler<engine::Engine::CreateVideoSink>();
> +    d->object->uninstall_method_handler<engine::Engine::Play>();
> +    d->object->uninstall_method_handler<engine::Engine::Stop>();
> +    d->object->uninstall_method_handler<engine::Engine::Pause>();
> +    d->object->uninstall_method_handler<engine::Engine::SeekTo>();
> +    d->object->uninstall_method_handler<engine::Engine::Reset>();
> +}
> +
> +const std::shared_ptr<media::Engine::MetaDataExtractor>& engine::EngineSkeleton::meta_data_extractor() const
> +{
> +    throw std::logic_error{"not implemented"};
> +}
> +
> +bool engine::EngineSkeleton::open_resource_for_uri(
> +    const media::Track::UriType& uri,
> +    bool do_pipeline_reset)
> +{
> +    return d->impl->open_resource_for_uri(uri, do_pipeline_reset);
> +}
> +
> +bool engine::EngineSkeleton::open_resource_for_uri(
> +    const media::Track::UriType& uri,
> +    const media::Player::HeadersType& headers)
> +{
> +    return d->impl->open_resource_for_uri(uri, headers);
> +}
> +
> +void engine::EngineSkeleton::create_video_sink(std::uint32_t texture_id)
> +{
> +    d->impl->create_video_sink(texture_id);
> +}
> +
> +bool engine::EngineSkeleton::play()
> +{
> +    return d->impl->play();
> +}
> +
> +bool engine::EngineSkeleton::stop()
> +{
> +    return d->impl->stop();
> +}
> +
> +bool engine::EngineSkeleton::pause()
> +{
> +    return d->impl->pause();
> +}
> +
> +bool engine::EngineSkeleton::seek_to(const std::chrono::microseconds& ts)
> +{
> +    return d->impl->seek_to(ts);
> +}
> +
> +void engine::EngineSkeleton::reset()
> +{
> +    d->impl->reset();
> +}
> +
> +// TODO karni: complete engine.h interface implementation
> +
> +const core::Property<media::Engine::State>& engine::EngineSkeleton::state() const
> +{
> +    return *d->properties.state;
> +}
> +
> +const core::Property<bool>& engine::EngineSkeleton::is_video_source() const
> +{
> +    return *d->properties.is_video_source;
> +}
> +
> +const core::Property<bool>& engine::EngineSkeleton::is_audio_source() const
> +{
> +    return *d->properties.is_audio_source;
> +}
> +
> +const core::Property<uint64_t>& engine::EngineSkeleton::position() const
> +{
> +    return *d->properties.position;
> +}
> +
> +const core::Property<uint64_t>& engine::EngineSkeleton::duration() const
> +{
> +    return *d->properties.duration;
> +}
> +
> +const core::Property<media::Engine::Volume>& engine::EngineSkeleton::volume() const
> +{
> +    return *d->properties.volume;
> +}
> +
> +const core::Property<media::Player::AudioStreamRole>& engine::EngineSkeleton::audio_stream_role() const
> +{
> +    return *d->properties.audio_stream_role;
> +}
> +const core::Property<media::Player::Orientation>& engine::EngineSkeleton::orientation() const
> +{
> +    return *d->properties.orientation;
> +}
> +
> +const core::Property<media::Player::Lifetime>& engine::EngineSkeleton::lifetime() const
> +{
> +    return *d->properties.lifetime;
> +}
> +
> +const core::Property<std::tuple<media::Track::UriType, media::Track::MetaData>>& engine::EngineSkeleton::track_meta_data() const
> +{
> +    return *d->properties.track_meta_data;
> +}
> +
> +core::Property<media::Engine::Volume>& engine::EngineSkeleton::volume()
> +{
> +    return *d->properties.volume;
> +}
> +
> +core::Property<media::Player::AudioStreamRole>& engine::EngineSkeleton::audio_stream_role()
> +{
> +    return *d->properties.audio_stream_role;
> +}
> +
> +core::Property<media::Player::Lifetime>& engine::EngineSkeleton::lifetime()
> +{
> +    return *d->properties.lifetime;
> +}
> +
> +const core::Signal<void>& engine::EngineSkeleton::about_to_finish_signal() const
> +{
> +    return d->signals.about_to_finish;
> +}
> +
> +core::Signal<void>& engine::EngineSkeleton::about_to_finish_signal()
> +{
> +    return d->signals.about_to_finish;
> +}
> +
> +const core::Signal<uint64_t>& engine::EngineSkeleton::seeked_to_signal() const
> +{
> +    return d->signals.seeked_to;
> +}
> +
> +core::Signal<uint64_t>& engine::EngineSkeleton::seeked_to_signal()
> +{
> +    return d->signals.seeked_to;
> +}
> +
> +const core::Signal<void>& engine::EngineSkeleton::client_disconnected_signal() const
> +{
> +    return d->signals.client_disconnected;
> +}
> +
> +core::Signal<void>& engine::EngineSkeleton::client_disconnected_signal()
> +{
> +    return d->signals.client_disconnected;
> +}
> +
> +const core::Signal<void>& engine::EngineSkeleton::end_of_stream_signal() const
> +{
> +    return d->signals.end_of_stream;
> +}
> +
> +core::Signal<void>& engine::EngineSkeleton::end_of_stream_signal()
> +{
> +    return d->signals.end_of_stream;
> +}
> +
> +const core::Signal<media::Player::PlaybackStatus>& engine::EngineSkeleton::playback_status_changed_signal() const
> +{
> +    return d->signals.playback_status_changed;
> +}
> +
> +core::Signal<media::Player::PlaybackStatus>& engine::EngineSkeleton::playback_status_changed_signal()
> +{
> +    return d->signals.playback_status_changed;
> +}
> +
> +const core::Signal<media::video::Dimensions>& engine::EngineSkeleton::video_dimension_changed_signal() const
> +{
> +    return d->signals.video_dimension_changed;
> +}
> +
> +core::Signal<media::video::Dimensions>& engine::EngineSkeleton::video_dimension_changed_signal()
> +{
> +    return d->signals.video_dimension_changed;
> +}
> +
> +const core::Signal<media::Player::Error>& engine::EngineSkeleton::error_signal() const
> +{
> +    return d->signals.error;
> +}
> +
> +core::Signal<media::Player::Error>& engine::EngineSkeleton::error_signal()
> +{
> +    return d->signals.error;
> +}
> 
> === added file 'src/core/media/engine/engine_skeleton.h'
> --- src/core/media/engine/engine_skeleton.h	1970-01-01 00:00:00 +0000
> +++ src/core/media/engine/engine_skeleton.h	2016-01-27 19:53:39 +0000
> @@ -0,0 +1,101 @@
> +/**
> + * Copyright (C) 2016 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: Michal Karnicki <michal.karnicki at canonical.com>
> + */
> +
> +#pragma once

Use old method.

> +
> +#include <memory>
> +
> +#include <core/dbus/skeleton.h>
> +#include <core/dbus/types/object_path.h>
> +
> +#include "engine_traits.h"
> +#include "../engine.h" // TODO karni: <core/media/engine.h>
> +#include "../apparmor/ubuntu.h"
> +
> +namespace core
> +{
> +namespace media
> +{
> +namespace engine
> +{
> +class Service;
> +
> +namespace media = core::ubuntu::media;
> +namespace engine = core::media::engine;
> +
> +class EngineSkeleton : public media::Engine
> +{
> +public:
> +    explicit EngineSkeleton(
> +    	const std::shared_ptr<media::Engine>& engine,
> +    	const std::shared_ptr<core::dbus::Bus>& bus,
> +    	const std::shared_ptr<core::dbus::Object>& object);
> +    virtual ~EngineSkeleton();
> +
> +    virtual const std::shared_ptr<media::Engine::MetaDataExtractor>& meta_data_extractor() const;
> +
> +    virtual bool open_resource_for_uri(const media::Track::UriType& uri, bool do_pipeline_reset);
> +    virtual bool open_resource_for_uri(const media::Track::UriType& uri, const media::Player::HeadersType&);
> +    // Throws media::Player::Error::OutOfProcessBufferStreamingNotSupported if the implementation does not
> +    // support this feature.
> +    virtual void create_video_sink(uint32_t texture_id);
> +
> +    virtual bool play();
> +    virtual bool stop() ;
> +    virtual bool pause();
> +    virtual bool seek_to(const std::chrono::microseconds& ts);
> +    virtual void reset();
> +
> +    virtual const core::Property<media::Engine::State>& state() const;
> +    virtual const core::Property<bool>& is_video_source() const;
> +    virtual const core::Property<bool>& is_audio_source() const;
> +    virtual const core::Property<uint64_t>& position() const;
> +    virtual const core::Property<uint64_t>& duration() const;
> +    virtual const core::Property<media::Engine::Volume>& volume() const;
> +    virtual const core::Property<media::Player::AudioStreamRole>& audio_stream_role() const;
> +    virtual const core::Property<media::Player::Orientation>& orientation() const;
> +    virtual const core::Property<media::Player::Lifetime>& lifetime() const;
> +    virtual const core::Property<std::tuple<media::Track::UriType, media::Track::MetaData>>& track_meta_data() const;
> +
> +    virtual core::Property<media::Engine::Volume>& volume();
> +    virtual core::Property<media::Player::AudioStreamRole>& audio_stream_role();
> +    virtual core::Property<media::Player::Lifetime>& lifetime();
> +
> +    virtual const core::Signal<void>& about_to_finish_signal() const;
> +    virtual const core::Signal<uint64_t>& seeked_to_signal() const;
> +    virtual const core::Signal<void>& client_disconnected_signal() const;
> +    virtual const core::Signal<void>& end_of_stream_signal() const;
> +    virtual const core::Signal<media::Player::PlaybackStatus>& playback_status_changed_signal() const;
> +    virtual const core::Signal<media::video::Dimensions>& video_dimension_changed_signal() const;
> +    virtual const core::Signal<media::Player::Error>& error_signal() const;
> +
> +    virtual core::Signal<void>& about_to_finish_signal();
> +    virtual core::Signal<uint64_t>& seeked_to_signal();
> +    virtual core::Signal<void>& client_disconnected_signal();
> +    virtual core::Signal<void>& end_of_stream_signal();
> +    virtual core::Signal<media::Player::PlaybackStatus>& playback_status_changed_signal();
> +    virtual core::Signal<media::video::Dimensions>& video_dimension_changed_signal();
> +    virtual core::Signal<media::Player::Error>& error_signal();
> +
> +private:
> +    struct Private;
> +    std::shared_ptr<Private> d;
> +};
> +}
> +}
> +}
> 
> === added file 'src/core/media/engine/engine_stub.cpp'
> --- src/core/media/engine/engine_stub.cpp	1970-01-01 00:00:00 +0000
> +++ src/core/media/engine/engine_stub.cpp	2016-01-27 19:53:39 +0000
> @@ -0,0 +1,365 @@
> +/*
> + * Copyright © 2016 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: Michal Karnicki <michal.karnicki at canonical.com>
> + */
> +
> +#include "engine_stub.h"
> +
> +#include "codec.h" // Volume

Is the volume comment necessary?

> +#include "engine.h"
> +// #include "engine_traits.h"
> +#include "../property_stub.h"
> +#include "../the_session_bus.h"
> +#include "../codec.h"
> +
> +#include <core/dbus/property.h>
> +#include <core/dbus/types/object_path.h>
> +
> +namespace dbus = core::dbus;
> +namespace engine = core::media::engine;
> +namespace media = core::ubuntu::media;
> +
> +struct engine::EngineStub::Private
> +{
> +    Private(const std::shared_ptr<core::dbus::Bus>& bus,
> +            const std::shared_ptr<core::dbus::Object>& object
> +            ) : bus(bus),
> +                object(object),
> +                properties
> +                {
> +                    object->get_property<engine::Engine::Properties::State>(),
> +                    object->get_property<engine::Engine::Properties::IsVideoSource>(),
> +                    object->get_property<engine::Engine::Properties::IsAudioSource>(),
> +                    object->get_property<engine::Engine::Properties::Position>(),
> +                    object->get_property<engine::Engine::Properties::Duration>(),
> +                    object->get_property<engine::Engine::Properties::Volume>(),
> +                    object->get_property<engine::Engine::Properties::AudioStreamRole>(),
> +                    object->get_property<engine::Engine::Properties::Orientation>(),
> +                    object->get_property<engine::Engine::Properties::Lifetime>(),
> +                    object->get_property<engine::Engine::Properties::TrackMetaData>()
> +                },
> +                signals
> +                {
> +                    object->get_signal<engine::Engine::Signals::SeekedTo>(),
> +                    object->get_signal<engine::Engine::Signals::AboutToFinish>(),
> +                    object->get_signal<engine::Engine::Signals::EndOfStream>(),
> +                    object->get_signal<engine::Engine::Signals::ClientDisconnected>(),
> +                    object->get_signal<engine::Engine::Signals::PlaybackStatusChanged>(),
> +                    object->get_signal<engine::Engine::Signals::VideoDimensionChanged>(),
> +                    object->get_signal<engine::Engine::Signals::Error>()
> +                }
> +    {
> +    }
> +
> +    ~Private()
> +    {
> +    }
> +
> +    std::shared_ptr<core::dbus::Bus> bus;
> +    std::shared_ptr<core::dbus::Object> object;
> +
> +    struct Properties
> +    {
> +        std::shared_ptr<core::dbus::Property<engine::Engine::Properties::State>> state;
> +        std::shared_ptr<core::dbus::Property<engine::Engine::Properties::IsVideoSource>> is_video_source;
> +        std::shared_ptr<core::dbus::Property<engine::Engine::Properties::IsAudioSource>> is_audio_source;
> +        std::shared_ptr<core::dbus::Property<engine::Engine::Properties::Position>> position;
> +        std::shared_ptr<core::dbus::Property<engine::Engine::Properties::Duration>> duration;
> +        std::shared_ptr<core::dbus::Property<engine::Engine::Properties::Volume>> volume;
> +        std::shared_ptr<core::dbus::Property<engine::Engine::Properties::AudioStreamRole>> audio_stream_role;
> +        std::shared_ptr<core::dbus::Property<engine::Engine::Properties::Orientation>> orientation;
> +        std::shared_ptr<core::dbus::Property<engine::Engine::Properties::Lifetime>> lifetime;
> +        std::shared_ptr<core::dbus::Property<engine::Engine::Properties::TrackMetaData>> track_meta_data;
> +    } properties;
> +
> +    struct Signals
> +    {
> +        typedef core::dbus::Signal<engine::Engine::Signals::SeekedTo, engine::Engine::Signals::SeekedTo::ArgumentType> DBusSeekedToSignal;
> +        typedef core::dbus::Signal<engine::Engine::Signals::AboutToFinish, engine::Engine::Signals::AboutToFinish::ArgumentType> DBusAboutToFinishSignal;
> +        typedef core::dbus::Signal<engine::Engine::Signals::EndOfStream, engine::Engine::Signals::EndOfStream::ArgumentType> DBusEndOfStreamSignal;
> +        typedef core::dbus::Signal<engine::Engine::Signals::ClientDisconnected, engine::Engine::Signals::ClientDisconnected::ArgumentType> DBusClientDisconnectedSignal;
> +        typedef core::dbus::Signal<engine::Engine::Signals::PlaybackStatusChanged, engine::Engine::Signals::PlaybackStatusChanged::ArgumentType> DBusPlaybackStatusChangedSignal;
> +        typedef core::dbus::Signal<engine::Engine::Signals::VideoDimensionChanged, engine::Engine::Signals::VideoDimensionChanged::ArgumentType> DBusVideoDimensionChangedSignal;
> +        typedef core::dbus::Signal<engine::Engine::Signals::Error, engine::Engine::Signals::Error::ArgumentType> DBusErrorSignal;
> +
> +        Signals(const std::shared_ptr<DBusSeekedToSignal>& seeked_to,
> +                const std::shared_ptr<DBusAboutToFinishSignal>& about_to_finish,
> +                const std::shared_ptr<DBusEndOfStreamSignal>& end_of_stream,
> +                const std::shared_ptr<DBusClientDisconnectedSignal> client_disconnected,
> +                const std::shared_ptr<DBusPlaybackStatusChangedSignal>& playback_status_changed,
> +                const std::shared_ptr<DBusVideoDimensionChangedSignal>& video_dimension_changed,
> +                const std::shared_ptr<DBusErrorSignal>& error)
> +            : seeked_to(),
> +              about_to_finish(),
> +              end_of_stream(),
> +              client_disconnected(),
> +              playback_status_changed(),
> +              video_dimension_changed(),
> +              error(),
> +              dbus
> +              {
> +                  seeked_to,
> +                  about_to_finish,
> +                  end_of_stream,
> +                  client_disconnected,
> +                  playback_status_changed,
> +                  video_dimension_changed,
> +                  error
> +              }
> +        {
> +            dbus.seeked_to->connect([this](std::uint64_t value)
> +            {
> +                std::cout << "SeekedTo signal arrived via the bus." << std::endl;
> +                this->seeked_to(value);
> +            });
> +
> +            dbus.about_to_finish->connect([this]()
> +            {
> +                std::cout << "AboutToFinish signal arrived via the bus." << std::endl;
> +                this->about_to_finish();
> +            });
> +
> +            dbus.end_of_stream->connect([this]()
> +            {
> +                std::cout << "EndOfStream signal arrived via the bus." << std::endl;
> +                this->end_of_stream();
> +            });
> +
> +            dbus.client_disconnected->connect([this]()
> +            {
> +                std::cout << "ClientDisconnected signal arrived via the bus." << std::endl;
> +                this->client_disconnected();
> +            });
> +
> +            dbus.playback_status_changed->connect([this](const media::Player::PlaybackStatus& status)

Make sure all of the functions where you pass a PlaybackStatus instance are passed consistently. I've seen you pass by value and here you're passing by reference.

> +            {
> +                std::cout << "PlaybackStatusChanged signal arrived via the bus (Status: " << status << ")" << std::endl;
> +                this->playback_status_changed(status);
> +            });
> +
> +            dbus.video_dimension_changed->connect([this](const media::video::Dimensions dimensions)

Pass by reference, not by value.

> +            {
> +                std::cout << "VideoDimensionChanged signal arrived via the bus." << std::endl;
> +                this->video_dimension_changed(dimensions);
> +            });
> +
> +            dbus.error->connect([this](const media::Player::Error& e)
> +            {
> +                std::cout << "Error signal arrived via the bus (Error: " << e << ")" << std::endl;
> +                this->error(e);
> +            });
> +        }
> +
> +        core::Signal<uint64_t> seeked_to;
> +        core::Signal<void> about_to_finish;
> +        core::Signal<void> end_of_stream;
> +        core::Signal<void> client_disconnected;
> +        core::Signal<media::Player::PlaybackStatus> playback_status_changed;
> +        core::Signal<media::video::Dimensions> video_dimension_changed;
> +        core::Signal<media::Player::Error> error;
> +
> +        struct DBus
> +        {
> +            std::shared_ptr<DBusSeekedToSignal> seeked_to;
> +            std::shared_ptr<DBusAboutToFinishSignal> about_to_finish;
> +            std::shared_ptr<DBusEndOfStreamSignal> end_of_stream;
> +            std::shared_ptr<DBusClientDisconnectedSignal> client_disconnected;
> +            std::shared_ptr<DBusPlaybackStatusChangedSignal> playback_status_changed;
> +            std::shared_ptr<DBusVideoDimensionChangedSignal> video_dimension_changed;
> +            std::shared_ptr<DBusErrorSignal> error;
> +        } dbus;
> +    } signals;
> +};
> +
> +engine::EngineStub::EngineStub(
> +    const std::shared_ptr<core::dbus::Bus>& bus,
> +    const std::shared_ptr<core::dbus::Object>& object
> +    ) : d(new Private{bus, object})
> +{
> +}
> +
> +engine::EngineStub::~EngineStub()
> +{
> +}
> +
> +const std::shared_ptr<media::Engine::MetaDataExtractor>& engine::EngineStub::meta_data_extractor() const
> +{
> +    throw std::logic_error{"not implemented"};
> +}
> +
> +const core::Property<media::Engine::State>& engine::EngineStub::state() const
> +{
> +    return *d->properties.state;
> +}
> +
> +bool engine::EngineStub::open_resource_for_uri(
> +    const media::Track::UriType& uri,
> +    bool do_pipeline_reset)
> +{
> +    auto result = d->object->invoke_method_synchronously<engine::Engine::OpenResourceForUri, bool>(uri, do_pipeline_reset);
> +    if (result.is_error())
> +        throw std::runtime_error(result.error().print());
> +    return result.value();
> +}
> +
> +bool engine::EngineStub::open_resource_for_uri(
> +    const media::Track::UriType& uri,
> +    const media::Player::HeadersType& headers)
> +{
> +    auto result = d->object->invoke_method_synchronously<engine::Engine::OpenResourceForUriExtended, bool>(uri, headers);
> +    if (result.is_error())
> +        throw std::runtime_error(result.error().print());
> +    return result.value();
> +}
> +
> +void engine::EngineStub::create_video_sink(std::uint32_t texture_id)
> +{
> +    d->object->invoke_method_synchronously<engine::Engine::CreateVideoSink, void>(texture_id);
> +}
> +
> +bool engine::EngineStub::play()
> +{
> +    auto result = d->object->invoke_method_synchronously<engine::Engine::Play, bool>();
> +    if (result.is_error())
> +        throw std::runtime_error(result.error().print());
> +    return result.value();
> +}
> +
> +bool engine::EngineStub::stop()
> +{
> +    auto result = d->object->invoke_method_synchronously<engine::Engine::Stop, bool>();
> +    if (result.is_error())
> +        throw std::runtime_error(result.error().print());
> +    return result.value();
> +}
> +
> +bool engine::EngineStub::pause()
> +{
> +    auto result = d->object->invoke_method_synchronously<engine::Engine::Pause, bool>();
> +    if (result.is_error())
> +        throw std::runtime_error(result.error().print());
> +    return result.value();
> +}
> +
> +bool engine::EngineStub::seek_to(const std::chrono::microseconds& ts)
> +{
> +    auto result = d->object->invoke_method_synchronously<engine::Engine::SeekTo, bool>(ts);
> +    if (result.is_error())
> +        throw std::runtime_error(result.error().print());
> +    return result.value();
> +}
> +
> +void engine::EngineStub::reset()
> +{
> +    auto result = d->object->invoke_method_synchronously<engine::Engine::Reset, void>();
> +    if (result.is_error())
> +        throw std::runtime_error(result.error().print());
> +}
> +
> +const core::Property<bool>& engine::EngineStub::is_video_source() const
> +{
> +    return *d->properties.is_video_source;
> +}
> +
> +const core::Property<bool>& engine::EngineStub::is_audio_source() const
> +{
> +    return *d->properties.is_audio_source;
> +}
> +
> +const core::Property<uint64_t>& engine::EngineStub::position() const
> +{
> +    return *d->properties.position;
> +}
> +
> +const core::Property<uint64_t>& engine::EngineStub::duration() const
> +{
> +    return *d->properties.duration;
> +}
> +
> +const core::Property<media::Engine::Volume>& engine::EngineStub::volume() const
> +{
> +    return *d->properties.volume;
> +}
> +
> +const core::Property<media::Player::AudioStreamRole>& engine::EngineStub::audio_stream_role() const
> +{
> +    return *d->properties.audio_stream_role;
> +}
> +
> +const core::Property<media::Player::Orientation>& engine::EngineStub::orientation() const
> +{
> +    return *d->properties.orientation;
> +}
> +
> +const core::Property<media::Player::Lifetime>& engine::EngineStub::lifetime() const
> +{
> +    return *d->properties.lifetime;
> +}
> +
> +const core::Property<std::tuple<media::Track::UriType, media::Track::MetaData>>& engine::EngineStub::track_meta_data() const
> +{
> +    return *d->properties.track_meta_data;
> +}
> +
> +core::Property<media::Engine::Volume>& engine::EngineStub::volume()
> +{
> +    return *d->properties.volume;
> +}
> +
> +core::Property<media::Player::AudioStreamRole>& engine::EngineStub::audio_stream_role()
> +{
> +    return *d->properties.audio_stream_role;
> +}
> +
> +core::Property<media::Player::Lifetime>& engine::EngineStub::lifetime()
> +{
> +    return *d->properties.lifetime;
> +}
> +
> +const core::Signal<void>& engine::EngineStub::about_to_finish_signal() const
> +{
> +    return d->signals.about_to_finish;
> +}
> +
> +const core::Signal<uint64_t>& engine::EngineStub::seeked_to_signal() const
> +{
> +    return d->signals.seeked_to;
> +}
> +
> +const core::Signal<void>& engine::EngineStub::client_disconnected_signal() const
> +{
> +    return d->signals.client_disconnected;
> +}
> +
> +const core::Signal<void>& engine::EngineStub::end_of_stream_signal() const
> +{
> +    return d->signals.end_of_stream;
> +}
> +
> +const core::Signal<media::Player::PlaybackStatus>& engine::EngineStub::playback_status_changed_signal() const
> +{
> +    return d->signals.playback_status_changed;
> +}
> +
> +const core::Signal<media::video::Dimensions>& engine::EngineStub::video_dimension_changed_signal() const
> +{
> +    return d->signals.video_dimension_changed;
> +}
> +
> +const core::Signal<media::Player::Error>& engine::EngineStub::error_signal() const
> +{
> +    return d->signals.error;
> +}
> 
> === added file 'src/core/media/engine/engine_stub.h'
> --- src/core/media/engine/engine_stub.h	1970-01-01 00:00:00 +0000
> +++ src/core/media/engine/engine_stub.h	2016-01-27 19:53:39 +0000
> @@ -0,0 +1,91 @@
> +/*
> + * Copyright © 2016 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: Michal Karnicki <michal.karnicki at canonical.com>
> + */
> +
> +#pragma once

Use old method.

> +
> +#include "../engine.h" // TODO karni: <core/media/engine.h>
> +
> +#include <core/media/player.h>
> +#include <core/media/track.h>
> +
> +#include <core/dbus/stub.h>
> +
> +#include <memory>
> +
> +namespace core
> +{
> +namespace media
> +{
> +namespace engine
> +{
> +namespace media = core::ubuntu::media;
> +
> +class EngineStub : public media::Engine
> +{
> +public:
> +    explicit EngineStub(
> +        const std::shared_ptr<core::dbus::Bus>& bus,
> +        const std::shared_ptr<core::dbus::Object>& object);
> +
> +    virtual ~EngineStub();
> +
> +    virtual const std::shared_ptr<media::Engine::MetaDataExtractor>& meta_data_extractor() const;
> +
> +    virtual const core::Property<media::Engine::State>& state() const;
> +
> +    virtual bool open_resource_for_uri(const media::Track::UriType& uri, bool do_pipeline_reset);
> +    virtual bool open_resource_for_uri(const media::Track::UriType& uri, const media::Player::HeadersType&);
> +    // Throws media::Player::Error::OutOfProcessBufferStreamingNotSupported if the implementation does not
> +    // support this feature.
> +    virtual void create_video_sink(uint32_t texture_id);
> +
> +    virtual bool play() override;
> +    virtual bool stop() override;
> +    virtual bool pause() override;
> +    virtual bool seek_to(const std::chrono::microseconds& ts) override;
> +    virtual void reset() override;
> +
> +    virtual const core::Property<bool>& is_video_source() const;
> +    virtual const core::Property<bool>& is_audio_source() const;
> +    virtual const core::Property<uint64_t>& position() const;
> +    virtual const core::Property<uint64_t>& duration() const;
> +    virtual const core::Property<media::Engine::Volume>& volume() const;
> +    virtual const core::Property<media::Player::AudioStreamRole>& audio_stream_role() const;
> +    virtual const core::Property<media::Player::Orientation>& orientation() const;
> +    virtual const core::Property<media::Player::Lifetime>& lifetime() const;
> +    virtual const core::Property<std::tuple<media::Track::UriType, media::Track::MetaData>>& track_meta_data() const;
> +
> +    virtual core::Property<media::Engine::Volume>& volume();
> +    virtual core::Property<media::Player::AudioStreamRole>& audio_stream_role();
> +    virtual core::Property<media::Player::Lifetime>& lifetime();
> +
> +    virtual const core::Signal<void>& about_to_finish_signal() const;
> +    virtual const core::Signal<uint64_t>& seeked_to_signal() const;
> +    virtual const core::Signal<void>& client_disconnected_signal() const;
> +    virtual const core::Signal<void>& end_of_stream_signal() const;
> +    virtual const core::Signal<media::Player::PlaybackStatus>& playback_status_changed_signal() const;
> +    virtual const core::Signal<media::video::Dimensions>& video_dimension_changed_signal() const;
> +    virtual const core::Signal<media::Player::Error>& error_signal() const;
> +
> +private:
> +    struct Private;
> +    std::unique_ptr<Private> d;
> +};
> +}
> +}
> +}
> 
> === added file 'tests/unit-tests/test-dbus-engine.cpp'
> --- tests/unit-tests/test-dbus-engine.cpp	1970-01-01 00:00:00 +0000
> +++ tests/unit-tests/test-dbus-engine.cpp	2016-01-27 19:53:39 +0000
> @@ -0,0 +1,1322 @@
> +/*
> + * Copyright © 2016 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: Michal Karnicki <michal.karnicki at canonical.com>
> + */
> +
> +#include "core/media/engine/codec.h"
> +#include "core/media/engine/engine_traits.h"
> +#include "core/media/engine/engine_skeleton.h"
> +#include "core/media/engine/engine_stub.h"
> +#include "core/media/engine.h"
> +
> +#include <core/media/player.h>
> +#include <core/media/track.h>
> +
> +#include <core/dbus/asio/executor.h>
> +#include <core/dbus/dbus.h>
> +#include <core/dbus/fixture.h>
> +#include <core/dbus/interfaces/properties.h>
> +#include <core/dbus/object.h>
> +#include <core/dbus/property.h>
> +#include <core/dbus/service.h>
> +
> +#include "sig_term_catcher.h"
> +#include "test_data.h"
> +
> +#include <core/testing/cross_process_sync.h>
> +#include <core/testing/fork_and_run.h>
> +
> +#include <gmock/gmock.h>
> +#include <gtest/gtest.h>
> +
> +#include <memory>
> +#include <system_error>
> +#include <thread>
> +
> +namespace dbus = core::dbus;
> +namespace engine = core::media::engine;
> +namespace media = core::ubuntu::media;
> +
> +namespace
> +{
> +using ::testing::_;
> +using ::testing::Return;
> +
> +auto session_bus_config_file =
> +    dbus::Fixture::default_session_bus_config_file();
> +    // core::testing::session_bus_configuration_file();
> +
> +auto system_bus_config_file =
> +    dbus::Fixture::default_system_bus_config_file();
> +    // core::testing::system_bus_configuration_file();
> +
> +class MockEngine : public media::Engine {
> +public:
> +    MockEngine()
> +    {
> +        ON_CALL(*this, open_resource_for_uri(_, true))
> +            .WillByDefault(Return(true));
> +        ON_CALL(*this, create_video_sink(_))
> +            .WillByDefault(Return());
> +
> +        ON_CALL(*this, play())
> +            .WillByDefault(Return(true));
> +        ON_CALL(*this, stop())
> +            .WillByDefault(Return(true));
> +        ON_CALL(*this, pause())
> +            .WillByDefault(Return(true));
> +        ON_CALL(*this, seek_to(_))
> +            .WillByDefault(Return(true));
> +
> +        properties.volume.changed().connect([this](Volume volume)
> +        {
> +            std::cout << "mock engine volume changed to: " << volume.value << std::endl;
> +        });
> +        properties.audio_stream_role.changed().connect([this](media::Player::AudioStreamRole role)
> +        {
> +            std::cout << "mock engine audio_stream_role changed to: " << role << std::endl;
> +        });
> +        properties.lifetime.changed().connect([this](media::Player::Lifetime lifetime)
> +        {
> +            std::cout << "mock engine lifetime changed to: " << lifetime << std::endl;
> +        });
> +    }
> +
> +    ~MockEngine()
> +    {
> +    }
> +
> +    MOCK_CONST_METHOD0(meta_data_extractor, std::shared_ptr<media::Engine::MetaDataExtractor>&());
> +
> +    MOCK_METHOD2(open_resource_for_uri, bool(const media::Track::UriType& uri, bool do_pipeline_reset));
> +    MOCK_METHOD2(open_resource_for_uri, bool(const media::Track::UriType& uri, const media::Player::HeadersType& headers));
> +
> +    MOCK_METHOD1(create_video_sink, void(uint32_t texture_id));
> +
> +    MOCK_METHOD0(play, bool());
> +    MOCK_METHOD0(pause, bool());
> +    MOCK_METHOD0(stop, bool());
> +    MOCK_METHOD1(seek_to, bool(const std::chrono::microseconds& ts));
> +    MOCK_METHOD0(reset, void());
> +
> +/*
> +property            set by:
> +
> +state               server
> +is_video_source     server
> +is_audio_source     server
> +position            server
> +duration            server
> +volume               client
> +audio_stream_role    client
> +orientation         server
> +lifetime             client
> +track_meta_data     server
> +*/
> +
> +    // MOCK_CONST_METHOD0(state, core::Property<media::Engine::State>&());

Are these commented out for a reason?

> +    // MOCK_CONST_METHOD0(is_video_source, core::Property<bool>&());
> +    // MOCK_CONST_METHOD0(is_audio_source, core::Property<bool>&());
> +    // MOCK_CONST_METHOD0(position, core::Property<uint64_t>&());
> +    // MOCK_CONST_METHOD0(duration, core::Property<uint64_t>&());
> +    // MOCK_CONST_METHOD0(volume, core::Property<media::Engine::Volume>&());
> +    // MOCK_CONST_METHOD0(audio_stream_role, core::Property<media::Player::AudioStreamRole>&());
> +    // MOCK_CONST_METHOD0(orientation, core::Property<media::Player::Orientation>&());
> +    // MOCK_CONST_METHOD0(lifetime, core::Property<media::Player::Lifetime>&());
> +    // MOCK_CONST_METHOD0(track_meta_data, core::Property<std::tuple<media::Track::UriType, media::Track::MetaData>>&());
> +
> +    // MOCK_METHOD0(volume, core::Property<media::Engine::Volume>&());
> +    // MOCK_METHOD0(audio_stream_role, core::Property<media::Player::AudioStreamRole>&());
> +    // MOCK_METHOD0(lifetime, core::Property<media::Player::Lifetime>&());
> +
> +    const core::Property<media::Engine::State>& state() const
> +    {
> +        return properties.state;
> +    }
> +
> +    core::Property<media::Engine::State>& state()
> +    {
> +        return properties.state;
> +    }
> +
> +    const core::Property<bool>& is_video_source() const
> +    {
> +        return properties.is_video_source;
> +    }
> +
> +    core::Property<bool>& is_video_source()
> +    {
> +        return properties.is_video_source;
> +    }
> +
> +    const core::Property<bool>& is_audio_source() const
> +    {
> +        return properties.is_audio_source;
> +    }
> +
> +    core::Property<bool>& is_audio_source()
> +    {
> +        return properties.is_audio_source;
> +    }
> +
> +    const core::Property<uint64_t>& position() const
> +    {
> +        return properties.position;
> +    }
> +
> +    core::Property<uint64_t>& position()
> +    {
> +        return properties.position;
> +    }
> +
> +    const core::Property<uint64_t>& duration() const
> +    {
> +        return properties.duration;
> +    }
> +
> +    core::Property<uint64_t>& duration()
> +    {
> +        return properties.duration;
> +    }
> +
> +    const core::Property<Volume>& volume() const
> +    {
> +        return properties.volume;
> +    }
> +
> +    core::Property<Volume>& volume()
> +    {
> +        return properties.volume;
> +    }
> +
> +    const core::Property<media::Player::AudioStreamRole>& audio_stream_role() const
> +    {
> +        return properties.audio_stream_role;
> +    }
> +
> +    core::Property<media::Player::AudioStreamRole>& audio_stream_role()
> +    {
> +        return properties.audio_stream_role;
> +    }    
> +
> +    const core::Property<media::Player::Orientation>& orientation() const
> +    {
> +        return properties.orientation;
> +    }
> +
> +    core::Property<media::Player::Orientation>& orientation()
> +    {
> +        return properties.orientation;
> +    }
> +
> +    const core::Property<media::Player::Lifetime>& lifetime() const
> +    {
> +        return properties.lifetime;
> +    }
> +
> +    core::Property<media::Player::Lifetime>& lifetime()
> +    {
> +        return properties.lifetime;
> +    }
> +
> +    const core::Property<std::tuple<media::Track::UriType, media::Track::MetaData>>& track_meta_data() const
> +    {
> +        return properties.track_meta_data;
> +    }
> +
> +    core::Property<std::tuple<media::Track::UriType, media::Track::MetaData>>& track_meta_data()
> +    {
> +        return properties.track_meta_data;
> +    }
> +
> +    struct
> +    {
> +        // std::shared_ptr<core::dbus::Property<engine::Engine::Properties::State>> state;
> +        core::Property<media::Engine::State> state;
> +        // std::shared_ptr<core::dbus::Property<engine::Engine::Properties::IsVideoSource>> is_video_source;
> +        core::Property<bool> is_video_source;
> +        // std::shared_ptr<core::dbus::Property<engine::Engine::Properties::IsAudioSource>> is_audio_source;
> +        core::Property<bool> is_audio_source;
> +        // std::shared_ptr<core::dbus::Property<engine::Engine::Properties::Position>> position;
> +        core::Property<uint64_t> position;
> +        // std::shared_ptr<core::dbus::Property<engine::Engine::Properties::Duration>> duration;
> +        core::Property<uint64_t> duration;
> +        // std::shared_ptr<core::dbus::Property<engine::Engine::Properties::Volume>> volume;
> +        core::Property<Volume> volume;
> +        // std::shared_ptr<core::dbus::Property<engine::Engine::Properties::AudioStreamRole>> audio_stream_role;
> +        core::Property<media::Player::AudioStreamRole> audio_stream_role;
> +        // std::shared_ptr<core::dbus::Property<engine::Engine::Properties::Orientation>> orientation;
> +        core::Property<media::Player::Orientation> orientation;
> +        // std::shared_ptr<core::dbus::Property<engine::Engine::Properties::Lifetime>> lifetime;
> +        core::Property<media::Player::Lifetime> lifetime;
> +        // std::shared_ptr<core::dbus::Property<engine::Engine::Properties::TrackMetaData>> track_meta_data;
> +        core::Property<std::tuple<media::Track::UriType, media::Track::MetaData>> track_meta_data;
> +
> +    } properties;
> +
> +    const core::Signal<void>& about_to_finish_signal() const
> +    {
> +        return signals.about_to_finish;
> +    }
> +
> +    const core::Signal<uint64_t>& seeked_to_signal() const
> +    {
> +        return signals.seeked_to;
> +    }
> +
> +    const core::Signal<void>& end_of_stream_signal() const
> +    {
> +        return signals.end_of_stream;
> +    }
> +
> +    const core::Signal<void>& client_disconnected_signal() const
> +    {
> +        return signals.client_disconnected;
> +    }
> +
> +    const core::Signal<media::Player::PlaybackStatus>& playback_status_changed_signal() const
> +    {
> +        return signals.playback_status_changed;
> +    }
> +
> +    const core::Signal<media::video::Dimensions>& video_dimension_changed_signal() const
> +    {
> +        return signals.video_dimension_changed;
> +    }
> +
> +    const core::Signal<media::Player::Error>& error_signal() const
> +    {
> +        return signals.error;
> +    }
> +
> +    core::Signal<void>& about_to_finish_signal()
> +    {
> +        return signals.about_to_finish;
> +    }
> +
> +    core::Signal<uint64_t>& seeked_to_signal()
> +    {
> +        return signals.seeked_to;
> +    }
> +
> +    core::Signal<void>& end_of_stream_signal()
> +    {
> +        return signals.end_of_stream;
> +    }
> +
> +    core::Signal<void>& client_disconnected_signal()
> +    {
> +        return signals.client_disconnected;
> +    }
> +
> +    core::Signal<media::Player::PlaybackStatus>& playback_status_changed_signal()
> +    {
> +        return signals.playback_status_changed;
> +    }
> +
> +    core::Signal<media::video::Dimensions>& video_dimension_changed_signal()
> +    {
> +        return signals.video_dimension_changed;
> +    }
> +
> +    core::Signal<media::Player::Error>& error_signal()
> +    {
> +        return signals.error;
> +    }
> +
> +    struct Signals
> +    {
> +        core::Signal<void> about_to_finish;
> +        core::Signal<uint64_t> seeked_to;
> +        core::Signal<void> end_of_stream;
> +        core::Signal<void> client_disconnected;
> +        core::Signal<media::Player::PlaybackStatus> playback_status_changed;
> +        core::Signal<media::video::Dimensions> video_dimension_changed;
> +        core::Signal<media::Player::Error> error;
> +    } signals;
> +};
> +
> +struct DBusEngine : public dbus::testing::Fixture
> +{
> +    void test_method_forward(
> +        std::function<void(std::shared_ptr<engine::EngineStub>&)> test_client,
> +        std::function<void(std::shared_ptr<MockEngine>)> test_serice);
> +
> +    void test_signal_callback(
> +        std::function<void(
> +            const std::shared_ptr<media::Engine>&)> emit_signal,
> +        std::function<void(
> +            const std::shared_ptr<dbus::Bus>& bus,
> +            const std::shared_ptr<engine::EngineStub>& stub,
> +            bool &signalled)> connect_signal);
> +
> +    void test_property_forward_from_engine(
> +        std::function<void(
> +            const std::shared_ptr<media::Engine>&)> change_property,
> +        std::function<void(
> +            const std::shared_ptr<dbus::Bus>& bus,
> +            const std::shared_ptr<engine::EngineStub>& stub,
> +            bool &property_changed)> check_property);
> +
> +    void test_property_forward_to_engine(
> +        std::function<void(
> +            const std::shared_ptr<engine::EngineStub>& stub)> change_property,
> +        std::function<void(
> +            const std::shared_ptr<dbus::Bus>& bus,
> +            const std::shared_ptr<media::Engine>& engine,
> +            bool& changed,
> +            core::testing::CrossProcessSync& property_has_changed)> track_property);
> +};
> +
> +void DBusEngine::test_method_forward(
> +    std::function<void(std::shared_ptr<engine::EngineStub>&)> test_client,
> +    std::function<void(std::shared_ptr<MockEngine>)> test_service)
> +{
> +    const std::string service_name{"com.ubuntu.media.Service.Engine"};
> +    const std::string path{"/com/ubuntu/media/Service/Engine"};
> +
> +    core::testing::CrossProcessSync barrier;
> +    core::testing::CrossProcessSync stop_barrier;
> +
> +    auto client = core::posix::fork([this, service_name, path, &barrier, &stop_barrier, &test_client]()
> +    {
> +        barrier.wait_for_signal_ready_for(std::chrono::milliseconds{500});
> +
> +        auto bus = session_bus();
> +
> +        auto service = dbus::Service::use_service(bus, service_name);
> +        auto object = service->object_for_path(dbus::types::ObjectPath{path});
> +        auto stub = std::shared_ptr<engine::EngineStub>(new engine::EngineStub(bus, object));
> +
> +        test_client(stub);
> +
> +        stop_barrier.try_signal_ready_for(std::chrono::milliseconds{500});
> +
> +        return ::testing::Test::HasFailure()
> +            ? core::posix::exit::Status::failure
> +            : core::posix::exit::Status::success;
> +    }, core::posix::StandardStream::empty);
> +    
> +    auto service = core::posix::fork([this, service_name, path, &barrier, &stop_barrier, &test_service]()
> +    {
> +        core::testing::SigTermCatcher sc;
> +
> +        auto bus = session_bus();
> +        auto executor = dbus::asio::make_executor(bus);
> +        bus->install_executor(executor);
> +        std::thread t{[bus](){ bus->run(); }};
> +
> +        MockEngine* mock_engine = new ::testing::NiceMock<MockEngine>();
> +        auto mock_ptr = std::shared_ptr<MockEngine>(mock_engine);
> +        auto engine_ptr = std::shared_ptr<media::Engine>(mock_ptr);
> +        test_service(mock_ptr);
> +
> +        dbus::Bus::RequestNameFlag flags{dbus::Bus::RequestNameFlag::do_not_queue};
> +        auto service = dbus::Service::add_service(bus, service_name, flags);
> +        auto object = service->add_object_for_path(dbus::types::ObjectPath{path});
> +        auto skeleton = std::shared_ptr<core::media::engine::EngineSkeleton>(
> +            new core::media::engine::EngineSkeleton(engine_ptr, bus, object));
> +
> +        barrier.try_signal_ready_for(std::chrono::milliseconds{500});
> +
> +        stop_barrier.wait_for_signal_ready_for(std::chrono::milliseconds{500});
> +        bus->stop();
> +        sc.wait_for_signal();
> +
> +        if (t.joinable())
> +            t.join();
> +
> +        return ::testing::Test::HasFailure()
> +            ? core::posix::exit::Status::failure
> +            : core::posix::exit::Status::success;
> +    }, core::posix::StandardStream::empty);
> +    
> +    auto client_result = client.wait_for(core::posix::wait::Flags::untraced);
> +
> +    EXPECT_EQ(core::posix::wait::Result::Status::exited, client_result.status);
> +    EXPECT_EQ(core::posix::exit::Status::success, client_result.detail.if_exited.status);
> +
> +    service.send_signal_or_throw(core::posix::Signal::sig_term);
> +    auto service_result = service.wait_for(core::posix::wait::Flags::untraced);
> +
> +    EXPECT_EQ(core::posix::wait::Result::Status::exited, service_result.status);
> +    EXPECT_EQ(core::posix::exit::Status::success, service_result.detail.if_exited.status);
> +}
> +
> +void DBusEngine::test_signal_callback(
> +    std::function<void(
> +        const std::shared_ptr<media::Engine>&)> emit_signal,
> +    std::function<void(
> +        const std::shared_ptr<dbus::Bus>& bus,
> +        const std::shared_ptr<engine::EngineStub>& stub,
> +        bool &signalled)> connect_signal)
> +{
> +const std::string service_name{"com.ubuntu.media.Service.Engine"};
> +    const std::string path{"/com/ubuntu/media/Service/Engine"};
> +
> +    core::testing::CrossProcessSync server_is_running;
> +    core::testing::CrossProcessSync client_has_setup_signals_and_connections;
> +
> +    auto client = [
> +        this, &service_name, &path,
> +        &server_is_running, &client_has_setup_signals_and_connections,
> +        &connect_signal]()
> +    {
> +        auto bus = session_bus();
> +        auto executor = dbus::asio::make_executor(bus);
> +        bus->install_executor(executor);
> +        std::thread t{[bus](){ bus->run(); }};
> +
> +        EXPECT_EQ(std::uint32_t(1),
> +                  server_is_running.wait_for_signal_ready_for(std::chrono::milliseconds{500}));
> +
> +        auto service = dbus::Service::use_service(bus, service_name);
> +        auto object = service->object_for_path(dbus::types::ObjectPath{path});
> +        auto stub = std::shared_ptr<engine::EngineStub>(new engine::EngineStub(bus, object));
> +
> +        bool signalled{false};
> +        connect_signal(bus, stub, signalled);
> +
> +        client_has_setup_signals_and_connections.try_signal_ready_for(std::chrono::milliseconds{500});
> +
> +        if (t.joinable())
> +            t.join();
> +
> +        EXPECT_TRUE(signalled);
> +
> +        return ::testing::Test::HasFailure()
> +            ? core::posix::exit::Status::failure
> +            : core::posix::exit::Status::success;
> +    };
> +
> +    auto service = [
> +        this, &service_name, &path,
> +        &server_is_running, &client_has_setup_signals_and_connections,
> +        &emit_signal]()
> +    {
> +        core::testing::SigTermCatcher sc;
> +
> +        auto bus = session_bus();
> +        auto executor = dbus::asio::make_executor(bus);
> +        bus->install_executor(executor);
> +        std::thread t{[bus](){ bus->run(); }};
> +
> +        MockEngine* mock_engine = new ::testing::NiceMock<MockEngine>();
> +        auto mock_ptr = std::shared_ptr<MockEngine>(mock_engine);
> +        auto engine_ptr = std::shared_ptr<media::Engine>(mock_ptr);
> +
> +        dbus::Bus::RequestNameFlag flags{dbus::Bus::RequestNameFlag::do_not_queue};
> +        auto service = dbus::Service::add_service(bus, service_name, flags);
> +        auto object = service->add_object_for_path(dbus::types::ObjectPath{path});
> +        auto skeleton = std::shared_ptr<core::media::engine::EngineSkeleton>(
> +            new core::media::engine::EngineSkeleton(engine_ptr, bus, object));
> +
> +        server_is_running.try_signal_ready_for(std::chrono::milliseconds{1000});
> +        EXPECT_EQ(std::uint32_t(1),
> +                  client_has_setup_signals_and_connections.wait_for_signal_ready_for(
> +                      std::chrono::milliseconds{500}));
> +
> +        emit_signal(engine_ptr);
> +
> +        sc.wait_for_signal();
> +
> +        bus->stop();
> +
> +        if (t.joinable())
> +            t.join();
> +
> +        return ::testing::Test::HasFailure()
> +            ? core::posix::exit::Status::failure
> +            : core::posix::exit::Status::success;
> +    };
> +
> +    EXPECT_EQ(core::testing::ForkAndRunResult::empty, core::testing::fork_and_run(service, client));
> +}
> +
> +void DBusEngine::test_property_forward_from_engine(
> +    std::function<void(
> +        const std::shared_ptr<media::Engine>&)> change_property,
> +    std::function<void(
> +        const std::shared_ptr<dbus::Bus>& bus,
> +        const std::shared_ptr<engine::EngineStub>& stub,
> +        bool &property_changed)> track_property)
> +{
> +    const std::string service_name{"com.ubuntu.media.Service.Engine"};
> +    const std::string path{"/com/ubuntu/media/Service/Engine"};
> +
> +    core::testing::CrossProcessSync server_is_running;
> +    core::testing::CrossProcessSync client_has_setup_properties;
> +
> +    auto client = [
> +        this, &service_name, &path,
> +        &server_is_running, &client_has_setup_properties,
> +        &track_property]()
> +    {
> +        auto bus = session_bus();
> +        auto executor = dbus::asio::make_executor(bus);
> +        bus->install_executor(executor);
> +        std::thread t{[bus](){ bus->run(); }};
> +
> +        EXPECT_EQ(std::uint32_t(1),
> +                  server_is_running.wait_for_signal_ready_for(std::chrono::milliseconds{500}));
> +
> +        auto service = dbus::Service::use_service(bus, service_name);
> +        auto object = service->object_for_path(dbus::types::ObjectPath{path});
> +        auto stub = std::shared_ptr<engine::EngineStub>(new engine::EngineStub(bus, object));
> +
> +        bool changed{false};
> +        track_property(bus, stub, changed);
> +
> +        client_has_setup_properties.try_signal_ready_for(std::chrono::milliseconds{500});
> +
> +        if (t.joinable())
> +            t.join();
> +
> +        EXPECT_TRUE(changed);
> +
> +        return ::testing::Test::HasFailure()
> +            ? core::posix::exit::Status::failure
> +            : core::posix::exit::Status::success;
> +    };
> +
> +    auto service = [
> +        this, &service_name, &path,
> +        &server_is_running, &client_has_setup_properties,
> +        &change_property]()
> +    {
> +        core::testing::SigTermCatcher sc;
> +
> +        auto bus = session_bus();
> +        auto executor = dbus::asio::make_executor(bus);
> +        bus->install_executor(executor);
> +        std::thread t{[bus](){ bus->run(); }};
> +
> +        MockEngine* mock_engine = new ::testing::NiceMock<MockEngine>();
> +        auto mock_ptr = std::shared_ptr<MockEngine>(mock_engine);
> +        auto engine_ptr = std::shared_ptr<media::Engine>(mock_ptr);
> +
> +        dbus::Bus::RequestNameFlag flags{dbus::Bus::RequestNameFlag::do_not_queue};
> +        auto service = dbus::Service::add_service(bus, service_name, flags);
> +        auto object = service->add_object_for_path(dbus::types::ObjectPath{path});
> +        auto skeleton = std::shared_ptr<core::media::engine::EngineSkeleton>(
> +            new core::media::engine::EngineSkeleton(engine_ptr, bus, object));
> +
> +        server_is_running.try_signal_ready_for(std::chrono::milliseconds{1000});
> +        EXPECT_EQ(std::uint32_t(1),
> +                  client_has_setup_properties.wait_for_signal_ready_for(
> +                      std::chrono::milliseconds{500}));
> +
> +        change_property(engine_ptr);
> +
> +        sc.wait_for_signal();
> +
> +        bus->stop();
> +
> +        if (t.joinable())
> +            t.join();
> +
> +        return ::testing::Test::HasFailure()
> +            ? core::posix::exit::Status::failure
> +            : core::posix::exit::Status::success;
> +    };
> +
> +    EXPECT_EQ(core::testing::ForkAndRunResult::empty, core::testing::fork_and_run(service, client));
> +}
> +
> +void DBusEngine::test_property_forward_to_engine(
> +    std::function<void(
> +        const std::shared_ptr<engine::EngineStub>& stub)> change_property,
> +    std::function<void(
> +        const std::shared_ptr<dbus::Bus>& bus,
> +        const std::shared_ptr<media::Engine>& engine,
> +        bool& changed,
> +        core::testing::CrossProcessSync&)> track_property)
> +{
> +    const std::string service_name{"com.ubuntu.media.Service.Engine"};
> +    const std::string path{"/com/ubuntu/media/Service/Engine"};
> +
> +    core::testing::CrossProcessSync server_is_running;
> +    core::testing::CrossProcessSync property_has_changed;
> +
> +    auto client = [
> +        this, &service_name, &path,
> +        &server_is_running, &property_has_changed,
> +        &change_property]()
> +    {
> +        auto bus = session_bus();
> +        auto executor = dbus::asio::make_executor(bus);
> +        bus->install_executor(executor);
> +        std::thread t{[bus](){ bus->run(); }};
> +
> +        EXPECT_EQ(std::uint32_t(1),
> +                  server_is_running.wait_for_signal_ready_for(std::chrono::milliseconds{500}));
> +
> +        auto service = dbus::Service::use_service(bus, service_name);
> +        auto object = service->object_for_path(dbus::types::ObjectPath{path});
> +        auto stub = std::shared_ptr<engine::EngineStub>(new engine::EngineStub(bus, object));
> +
> +        change_property(stub);
> +
> +        EXPECT_EQ(std::uint32_t(1),
> +            property_has_changed.wait_for_signal_ready_for(std::chrono::milliseconds{500}));
> +        bus->stop();
> +
> +        if (t.joinable())
> +            t.join();
> +
> +        return ::testing::Test::HasFailure()
> +            ? core::posix::exit::Status::failure
> +            : core::posix::exit::Status::success;
> +    };
> +
> +    auto service = [
> +        this, &service_name, &path,
> +        &server_is_running, &property_has_changed,
> +        &track_property]()
> +    {
> +        core::testing::SigTermCatcher sc;
> +
> +        auto bus = session_bus();
> +        auto executor = dbus::asio::make_executor(bus);
> +        bus->install_executor(executor);
> +        std::thread t{[bus](){ bus->run(); }};
> +
> +        MockEngine* mock_engine = new ::testing::NiceMock<MockEngine>();
> +        auto mock_ptr = std::shared_ptr<MockEngine>(mock_engine);
> +        auto engine_ptr = std::shared_ptr<media::Engine>(mock_ptr);
> +
> +        dbus::Bus::RequestNameFlag flags{dbus::Bus::RequestNameFlag::do_not_queue};
> +        auto service = dbus::Service::add_service(bus, service_name, flags);
> +        auto object = service->add_object_for_path(dbus::types::ObjectPath{path});
> +        auto skeleton = std::shared_ptr<core::media::engine::EngineSkeleton>(
> +            new core::media::engine::EngineSkeleton(engine_ptr, bus, object));
> +
> +        bool changed{false};
> +        track_property(bus, engine_ptr, changed, property_has_changed);
> +
> +        server_is_running.try_signal_ready_for(std::chrono::milliseconds{1000});
> +
> +        sc.wait_for_signal();
> +
> +        EXPECT_EQ(true, changed);
> +
> +        bus->stop();
> +
> +        if (t.joinable())
> +            t.join();
> +
> +        return ::testing::Test::HasFailure()
> +            ? core::posix::exit::Status::failure
> +            : core::posix::exit::Status::success;
> +    };
> +
> +    EXPECT_EQ(core::testing::ForkAndRunResult::empty, core::testing::fork_and_run(service, client));
> +}
> +}
> +
> +namespace
> +{   
> +template<typename T>
> +struct Expectation
> +{
> +    Expectation(const T& expected_value) : expected_value(expected_value)
> +    {
> +    }
> +    
> +    bool satisfied() const
> +    {
> +        return triggered && current_value == expected_value;
> +    }
> +
> +    bool triggered = false;
> +    T expected_value;
> +    T current_value;
> +};
> +}
> +
> +TEST_F(DBusEngine, InvokingPlayForwardsCallToEngineImplementation)
> +{
> +    auto test_client = [](const std::shared_ptr<engine::EngineStub> &stub) {
> +        bool result{false};
> +        EXPECT_NO_THROW(result = stub->play());
> +        EXPECT_EQ(true, result);
> +    };
> +
> +    auto test_service = [](const std::shared_ptr<MockEngine> &mock_ptr) {
> +        EXPECT_CALL(*(mock_ptr.get()), play()).Times(1);
> +    };
> +
> +    test_method_forward(test_client, test_service);
> +}
> +
> +TEST_F(DBusEngine, InvokingStopForwardsCallToEngineImplementation)
> +{
> +    auto test_client = [](const std::shared_ptr<engine::EngineStub> &stub) {
> +        bool result{false};
> +        EXPECT_NO_THROW(result = stub->stop());
> +        EXPECT_EQ(true, result);
> +    };
> +    
> +    auto test_service = [](const std::shared_ptr<MockEngine> &mock_ptr) {
> +        EXPECT_CALL(*(mock_ptr.get()), stop()).Times(1);
> +    };
> +
> +    test_method_forward(test_client, test_service);
> +}
> +
> +TEST_F(DBusEngine, InvokingPauseForwardsCallToEngineImplementation)
> +{
> +    auto test_client = [](const std::shared_ptr<engine::EngineStub> &stub) {
> +        bool result{false};
> +        EXPECT_NO_THROW(result = stub->pause());
> +        EXPECT_EQ(true, result);
> +    };
> +    
> +    auto test_service = [](const std::shared_ptr<MockEngine> &mock_ptr) {
> +        EXPECT_CALL(*(mock_ptr.get()), pause()).Times(1);
> +    };
> +
> +    test_method_forward(test_client, test_service);
> +}
> +
> +TEST_F(DBusEngine, InvokingSeekToForwardsCallToEngineImplementation)
> +{
> +    const std::chrono::microseconds ts{42};
> +
> +    auto test_client = [&ts](const std::shared_ptr<engine::EngineStub> &stub) {
> +        bool result{false};
> +        EXPECT_NO_THROW(result = stub->seek_to(ts));
> +        EXPECT_EQ(true, result);
> +    };
> +    
> +    auto test_service = [&ts](const std::shared_ptr<MockEngine> &mock_ptr) {
> +        EXPECT_CALL(*(mock_ptr.get()), seek_to(ts)).Times(1);
> +    };
> +
> +    test_method_forward(test_client, test_service);
> +}
> +
> +TEST_F(DBusEngine, InvokingResetForwardsCallToEngineImplementation)
> +{
> +    auto test_client = [](const std::shared_ptr<engine::EngineStub> &stub) {
> +        EXPECT_NO_THROW(stub->reset());
> +    };
> +    
> +    auto test_service = [](const std::shared_ptr<MockEngine> &mock_ptr) {
> +        EXPECT_CALL(*(mock_ptr.get()), reset()).Times(1);
> +    };
> +
> +    test_method_forward(test_client, test_service);
> +}
> +
> +// TODO karni: this test is failing (method call throws)
> +TEST_F(DBusEngine, InvokingOpenResourceForUriForwardsCallToEngineImplementation)
> +{
> +    const core::ubuntu::media::Track::UriType& uri{"provider://fake/track/uri/123"};
> +    bool do_pipeline_reset = true;
> +
> +    auto test_client = [&uri, &do_pipeline_reset](const std::shared_ptr<engine::EngineStub> &stub) {
> +        bool result{false};
> +        EXPECT_NO_THROW(result = stub->open_resource_for_uri(uri, do_pipeline_reset));
> +        EXPECT_EQ(true, result);
> +    };
> +    
> +    auto test_service = [&uri, &do_pipeline_reset](const std::shared_ptr<MockEngine> &mock_ptr) {
> +        EXPECT_CALL(*(mock_ptr.get()), open_resource_for_uri(uri, do_pipeline_reset)).Times(1);
> +    };
> +
> +    test_method_forward(test_client, test_service);
> +}
> +
> +TEST_F(DBusEngine, InvokingCreateVideoSinkForwardsCallToEngineImplementation)
> +{
> +    uint32_t texture_id{42};
> +
> +    auto test_client = [&texture_id](const std::shared_ptr<engine::EngineStub> &stub) {
> +        EXPECT_NO_THROW(stub->create_video_sink(texture_id));
> +    };
> +    
> +    auto test_service = [&texture_id](const std::shared_ptr<MockEngine> &mock_ptr) {
> +        EXPECT_CALL(*(mock_ptr.get()), create_video_sink(texture_id)).Times(1);
> +    };
> +
> +    test_method_forward(test_client, test_service);
> +}
> +
> +TEST_F(DBusEngine, EmittingAboutToFinishSignalForwardedToStub)
> +{
> +    auto emit_signal = [](const std::shared_ptr<media::Engine>& engine)
> +    {
> +        core::Signal<void>& about_to_finish =
> +            ((MockEngine*)(engine.get()))->about_to_finish_signal();
> +        about_to_finish();
> +    };
> +
> +    auto connect_signal = [](
> +        const std::shared_ptr<dbus::Bus>& bus,
> +        const std::shared_ptr<engine::EngineStub>& stub,
> +        bool &signalled)
> +    {
> +        stub->about_to_finish_signal().connect([&bus, &signalled]()
> +        {
> +            signalled = true;
> +            bus->stop();
> +        });
> +    };
> +
> +    test_signal_callback(emit_signal, connect_signal);
> +}
> +
> +TEST_F(DBusEngine, EmittingSeekedToSignalForwardedToStub)
> +{
> +    uint64_t value = 42;
> +
> +    auto emit_signal = [&value](const std::shared_ptr<media::Engine>& engine)
> +    {
> +        core::Signal<uint64_t>& seeked_to =
> +            ((MockEngine*)(engine.get()))->seeked_to_signal();
> +        seeked_to(value);
> +    };
> +
> +    auto connect_signal = [&value](
> +        const std::shared_ptr<dbus::Bus>& bus,
> +        const std::shared_ptr<engine::EngineStub>& stub,
> +        bool &signalled)
> +    {
> +        stub->seeked_to_signal().connect([&value, &bus, &signalled](uint64_t ts)
> +        {
> +            signalled = true;
> +            EXPECT_EQ(value, ts);
> +            bus->stop();
> +        });
> +    };
> +
> +    test_signal_callback(emit_signal, connect_signal);
> +}
> +
> +TEST_F(DBusEngine, EmittingClientDisconnectedSignalForwardedToStub)
> +{
> +    auto emit_signal = [](const std::shared_ptr<media::Engine>& engine)
> +    {
> +        core::Signal<void>& client_disconnected =
> +            ((MockEngine*)(engine.get()))->client_disconnected_signal();
> +        client_disconnected();
> +    };
> +
> +    auto connect_signal = [](
> +        const std::shared_ptr<dbus::Bus>& bus,
> +        const std::shared_ptr<engine::EngineStub>& stub,
> +        bool &signalled)
> +    {
> +        stub->client_disconnected_signal().connect([&bus, &signalled]()
> +        {
> +            signalled = true;
> +            bus->stop();
> +        });
> +    };
> +
> +    test_signal_callback(emit_signal, connect_signal);
> +}
> +
> +TEST_F(DBusEngine, EmittingEndOfStreamSignalForwardedToStub)
> +{
> +    auto emit_signal = [](const std::shared_ptr<media::Engine>& engine)
> +    {
> +        core::Signal<void>& end_of_stream =
> +            ((MockEngine*)(engine.get()))->end_of_stream_signal();
> +        end_of_stream();
> +    };
> +
> +    auto connect_signal = [](
> +        const std::shared_ptr<dbus::Bus>& bus,
> +        const std::shared_ptr<engine::EngineStub>& stub,
> +        bool &signalled)
> +    {
> +        stub->end_of_stream_signal().connect([&bus, &signalled]()
> +        {
> +            signalled = true;
> +            bus->stop();
> +        });
> +    };
> +
> +    test_signal_callback(emit_signal, connect_signal);
> +}
> +
> +TEST_F(DBusEngine, EmittingPlayerStatusChangedSignalForwardedToStub)
> +{
> +    media::Player::PlaybackStatus value{media::Player::PlaybackStatus::playing};
> +
> +    auto emit_signal = [&value](const std::shared_ptr<media::Engine>& engine)
> +    {
> +        core::Signal<media::Player::PlaybackStatus>& playback_status_changed =
> +            ((MockEngine*)(engine.get()))->playback_status_changed_signal();
> +        playback_status_changed(value);
> +    };
> +
> +    auto connect_signal = [&value](
> +        const std::shared_ptr<dbus::Bus>& bus,
> +        const std::shared_ptr<engine::EngineStub>& stub,
> +        bool &signalled)
> +    {
> +        stub->playback_status_changed_signal().connect([&value, &bus, &signalled](
> +            media::Player::PlaybackStatus status)
> +        {
> +            signalled = true;
> +            EXPECT_EQ(value, status);
> +            bus->stop();
> +        });
> +    };
> +
> +    test_signal_callback(emit_signal, connect_signal);
> +}
> +
> +TEST_F(DBusEngine, EmittingVideoDimensionChangedSignalForwardedToStub)
> +{
> +    namespace video = core::ubuntu::media::video;
> +    video::Width width{640};
> +    video::Height height{480};
> +    video::Dimensions value = std::tuple<video::Height, video::Width> {
> +        height, width
> +    };
> +
> +    auto emit_signal = [&value](const std::shared_ptr<media::Engine>& engine)
> +    {
> +        core::Signal<video::Dimensions>& video_dimension_changed =
> +            ((MockEngine*)(engine.get()))->video_dimension_changed_signal();
> +        video_dimension_changed(value);
> +    };
> +
> +    auto connect_signal = [&value](
> +        const std::shared_ptr<dbus::Bus>& bus,
> +        const std::shared_ptr<engine::EngineStub>& stub,
> +        bool &signalled)
> +    {
> +        stub->video_dimension_changed_signal().connect(
> +            [&value, &bus, &signalled](video::Dimensions dimensions)
> +        {
> +            signalled = true;
> +            EXPECT_EQ(value, dimensions);
> +            bus->stop();
> +        });
> +    };
> +
> +    test_signal_callback(emit_signal, connect_signal);
> +}
> +
> +TEST_F(DBusEngine, EmittingErrorSignalForwardedToStub)
> +{
> +    using Error = media::Player::Error;
> +
> +    Error error{Error::network_error};
> +
> +    auto emit_signal = [&error](const std::shared_ptr<media::Engine>& engine)
> +    {
> +        core::Signal<media::Player::Error>& error_signal =
> +            ((MockEngine*)(engine.get()))->error_signal();
> +        error_signal(error);
> +    };
> +
> +    auto connect_signal = [&error](
> +        const std::shared_ptr<dbus::Bus>& bus,
> +        const std::shared_ptr<engine::EngineStub>& stub,
> +        bool &signalled)
> +    {
> +        stub->error_signal().connect([&error, &bus, &signalled](Error value)
> +        {
> +            signalled = true;
> +            EXPECT_NE(Error::no_error, value);
> +            EXPECT_EQ(error, value);
> +            bus->stop();
> +        });
> +    };
> +
> +    test_signal_callback(emit_signal, connect_signal);
> +}
> +
> +// Properties signalling value changed from skeleton to stub.
> +
> +TEST_F(DBusEngine, IsStatePropertyChangeForwarded)
> +{
> +    media::Engine::State expected_value = media::Engine::State::playing;
> +
> +    auto change_property = [&expected_value](const std::shared_ptr<media::Engine>& engine)
> +    {
> +        core::Property<media::Engine::State>& state =
> +            ((MockEngine*)(engine.get()))->state();
> +        state.set(expected_value);
> +    };
> +
> +    auto track_property = [&expected_value](
> +        const std::shared_ptr<dbus::Bus>& bus,
> +        const std::shared_ptr<engine::EngineStub>& stub,
> +        bool &changed)
> +    {
> +        stub->state().changed().connect([&bus, &changed, &expected_value](media::Engine::State value){
> +            changed = true;
> +            EXPECT_EQ(expected_value, value);
> +            bus->stop();
> +        });
> +    };
> +
> +    test_property_forward_from_engine(change_property, track_property);
> +}
> +
> +TEST_F(DBusEngine, IsVideoSourcePropertyChangeForwarded)
> +{
> +    bool expected_value = true;
> +
> +    auto change_property = [&expected_value](const std::shared_ptr<media::Engine>& engine)
> +    {
> +        core::Property<bool>& is_video_source =
> +            ((MockEngine*)(engine.get()))->is_video_source();
> +        is_video_source.set(expected_value);
> +    };
> +
> +    auto track_property = [&expected_value](
> +        const std::shared_ptr<dbus::Bus>& bus,
> +        const std::shared_ptr<engine::EngineStub>& stub,
> +        bool &changed)
> +    {
> +        stub->is_video_source().changed().connect([&bus, &changed, &expected_value](bool value){
> +            changed = true;
> +            EXPECT_EQ(expected_value, value);
> +            bus->stop();
> +        });
> +    };
> +
> +    test_property_forward_from_engine(change_property, track_property);
> +}
> +
> +TEST_F(DBusEngine, IsAudioSourcePropertyChangeForwarded)
> +{
> +    bool expected_value = true;
> +
> +    auto change_property = [&expected_value](const std::shared_ptr<media::Engine>& engine)
> +    {
> +        core::Property<bool>& is_audio_source =
> +            ((MockEngine*)(engine.get()))->is_audio_source();
> +        is_audio_source.set(expected_value);
> +    };
> +
> +    auto track_property = [&expected_value](
> +        const std::shared_ptr<dbus::Bus>& bus,
> +        const std::shared_ptr<engine::EngineStub>& stub,
> +        bool &changed)
> +    {
> +        stub->is_audio_source().changed().connect([&bus, &changed, &expected_value](bool value){
> +            changed = true;
> +            EXPECT_EQ(expected_value, value);
> +            bus->stop();
> +        });
> +    };
> +
> +    test_property_forward_from_engine(change_property, track_property);
> +}
> +
> +TEST_F(DBusEngine, PositionPropertyChangeForwarded)
> +{
> +    uint64_t expected_value = 1024;
> +
> +    auto change_property = [&expected_value](const std::shared_ptr<media::Engine>& engine)
> +    {
> +        core::Property<uint64_t>& position =
> +            ((MockEngine*)(engine.get()))->position();
> +        position.set(expected_value);
> +    };
> +
> +    auto track_property = [&expected_value](
> +        const std::shared_ptr<dbus::Bus>& bus,
> +        const std::shared_ptr<engine::EngineStub>& stub,
> +        bool &changed)
> +    {
> +        stub->position().changed().connect([&bus, &changed, &expected_value](uint64_t value){
> +            changed = true;
> +            EXPECT_EQ(expected_value, value);
> +            bus->stop();
> +        });
> +    };
> +
> +    test_property_forward_from_engine(change_property, track_property);
> +}
> +
> +TEST_F(DBusEngine, DurationPropertyChangeForwarded)
> +{
> +    uint64_t expected_value = 2048;
> +
> +    auto change_property = [&expected_value](const std::shared_ptr<media::Engine>& engine)
> +    {
> +        core::Property<uint64_t>& duration =
> +            ((MockEngine*)(engine.get()))->duration();
> +        duration.set(expected_value);
> +    };
> +
> +    auto track_property = [&expected_value](
> +        const std::shared_ptr<dbus::Bus>& bus,
> +        const std::shared_ptr<engine::EngineStub>& stub,
> +        bool &changed)
> +    {
> +        stub->duration().changed().connect([&bus, &changed, &expected_value](uint64_t value){
> +            changed = true;
> +            EXPECT_EQ(expected_value, value);
> +            bus->stop();
> +        });
> +    };
> +
> +    test_property_forward_from_engine(change_property, track_property);
> +}
> +
> +TEST_F(DBusEngine, OrientationPropertyChangeForwardedFromEngine)
> +{
> +    media::Player::Orientation expected_value{core::ubuntu::media::Player::Orientation::rotate90};
> +
> +    auto change_property = [&expected_value](const std::shared_ptr<media::Engine>& engine)
> +    {
> +        core::Property<media::Player::Orientation>& orientation =
> +            ((MockEngine*)(engine.get()))->orientation();
> +        orientation.set(expected_value);
> +    };
> +
> +    auto track_property = [&expected_value](
> +        const std::shared_ptr<dbus::Bus>& bus,
> +        const std::shared_ptr<engine::EngineStub>& stub,
> +        bool &changed)
> +    {
> +        stub->orientation().changed().connect([&bus, &changed, &expected_value](media::Player::Orientation value){
> +            changed = true;
> +            EXPECT_EQ(expected_value, value);
> +            bus->stop();
> +        });
> +    };
> +
> +    test_property_forward_from_engine(change_property, track_property);
> +}
> +
> +TEST_F(DBusEngine, DISABLED_TrackMetaDataPropertyChangeForwardedFromEngine)
> +{
> +    std::tuple<media::Track::UriType, media::Track::MetaData> expected_value;
> +
> +    auto change_property = [&expected_value](const std::shared_ptr<media::Engine>& engine)
> +    {
> +        core::Property<std::tuple<media::Track::UriType, media::Track::MetaData>>& track_meta_data =
> +            ((MockEngine*)(engine.get()))->track_meta_data();
> +        track_meta_data.set(expected_value);
> +    };
> +
> +    auto track_property = [&expected_value](
> +        const std::shared_ptr<dbus::Bus>& bus,
> +        const std::shared_ptr<engine::EngineStub>& stub,
> +        bool &changed)
> +    {
> +        stub->track_meta_data().changed().connect([&bus, &changed, &expected_value]
> +                (std::tuple<media::Track::UriType, media::Track::MetaData> value){
> +            changed = true;
> +            EXPECT_EQ(expected_value, value);
> +            bus->stop();
> +        });
> +    };
> +
> +    test_property_forward_from_engine(change_property, track_property);
> +}
> +
> +// Properties signalling value changed from stub to skeleton.
> +
> +TEST_F(DBusEngine, VolumePropertyChangeForwardedToEngine)
> +{
> +    const media::Engine::Volume expected_value{0.5};
> +
> +    auto change_property = [&expected_value](
> +        const std::shared_ptr<engine::EngineStub>& stub)
> +    {
> +        EXPECT_NO_THROW(stub->volume().set(expected_value));
> +    };
> +
> +    auto track_property = [&expected_value](
> +        const std::shared_ptr<dbus::Bus>& bus,
> +        const std::shared_ptr<media::Engine>& engine,
> +        bool &changed,
> +        core::testing::CrossProcessSync& property_has_changed)
> +    {
> +        core::Property<media::Engine::Volume>& volume = ((MockEngine*)(engine.get()))->volume();
> +        volume.changed().connect([&bus, &expected_value, &changed, &property_has_changed] (media::Engine::Volume value)
> +        {
> +            changed = true;
> +            EXPECT_EQ(expected_value, value);
> +            property_has_changed.try_signal_ready_for(std::chrono::milliseconds{500});
> +            bus->stop();
> +        });
> +    };
> +
> +    test_property_forward_to_engine(change_property, track_property);
> +}
> +
> +TEST_F(DBusEngine, AudioStreamRolePropertyChangeForwardedToEngine)
> +{
> +    media::Player::AudioStreamRole expected_value{media::Player::AudioStreamRole::multimedia};
> +
> +    auto change_property = [&expected_value](
> +        const std::shared_ptr<engine::EngineStub>& stub)
> +    {
> +        EXPECT_NO_THROW(stub->audio_stream_role().set(expected_value));
> +    };
> +
> +    auto track_property = [&expected_value](
> +        const std::shared_ptr<dbus::Bus>& bus,
> +        const std::shared_ptr<media::Engine>& engine,
> +        bool &changed,
> +        core::testing::CrossProcessSync& property_has_changed)
> +    {
> +        core::Property<media::Player::AudioStreamRole>& audio_stream_role = ((MockEngine*)(engine.get()))->audio_stream_role();
> +        audio_stream_role.changed().connect([&bus, &expected_value, &changed, &property_has_changed] (media::Player::AudioStreamRole value)
> +        {
> +            changed = true;
> +            EXPECT_EQ(expected_value, value);
> +            property_has_changed.try_signal_ready_for(std::chrono::milliseconds{500});
> +            bus->stop();
> +        });
> +    };
> +
> +    test_property_forward_to_engine(change_property, track_property);
> +}
> +
> +TEST_F(DBusEngine, DISABLED_LifetimePropertyChangeForwardedToEngine)

Why is this test disabled?

> +{
> +    media::Player::Lifetime expected_value{media::Player::Lifetime::normal};
> +
> +    auto change_property = [&expected_value](
> +        const std::shared_ptr<engine::EngineStub>& stub)
> +    {
> +        EXPECT_NO_THROW(stub->lifetime().set(expected_value));
> +    };
> +
> +    auto track_property = [&expected_value](
> +        const std::shared_ptr<dbus::Bus>& bus,
> +        const std::shared_ptr<media::Engine>& engine,
> +        bool &changed,
> +        core::testing::CrossProcessSync& property_has_changed)
> +    {
> +        core::Property<media::Player::Lifetime>& lifetime = ((MockEngine*)(engine.get()))->lifetime();
> +        lifetime.changed().connect([&bus, &expected_value, &changed, &property_has_changed] (media::Player::Lifetime value)
> +        {
> +            std::cout << "XXX not reached" << std::flush << std::endl;
> +            changed = true;
> +            EXPECT_EQ(expected_value, value);
> +            property_has_changed.try_signal_ready_for(std::chrono::milliseconds{500});
> +            bus->stop();
> +        });
> +    };
> +
> +    test_property_forward_to_engine(change_property, track_property);
> +}


-- 
https://code.launchpad.net/~karni/media-hub/media-hub-engine-refactor/+merge/282286
Your team Ubuntu Phablet Team is subscribed to branch lp:media-hub/stable.



More information about the Ubuntu-reviews mailing list