[Merge] lp:~justinmcp/media-hub/proxy-player into lp:media-hub

[Justin McPherson]

Diff comments:

> 
> === added file 'src/core/media/managed_player_implementation.h'
> --- src/core/media/managed_player_implementation.h	1970-01-01 00:00:00 +0000
> +++ src/core/media/managed_player_implementation.h	2015-08-24 00:47:56 +0000
> @@ -0,0 +1,89 @@
> +/*
> + * Copyright © 2013-2015 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: Justin McPherson <justin.mcpherson at canonical.com>
> + */
> +
> +#ifndef CORE_UBUNTU_MANAGED_MEDIA_PLAYER_IMPLEMENTATION_H_
> +#define CORE_UBUNTU_MANAGED_MEDIA_PLAYER_IMPLEMENTATION_H_
> +
> +#include <core/media/player.h>
> +
> +#include "apparmor/ubuntu.h"
> +#include "client_death_observer.h"
> +#include "power/state_controller.h"
> +
> +#include <memory>
> +
> +namespace core
> +{
> +namespace ubuntu
> +{
> +namespace media
> +{
> +class Engine;
> +class Service;
> +
> +template<typename Parent>
> +class ManagedPlayerImplementation : public Parent

I am not so sure this is a good idea. By inheriting from PlayerImplementation, all the engine(gstreamer) code is brought along, which (among other things), builds two pipelines, creates/"installs" properties and sets getters that will may have side effects and most likely return inappropriate information or invoke exception behaviour. Even if we accept this, it would still put the same maintenance burden on future committers, as behaviour changes to engine/"standard" player implementation would also need to be tested for the proxy.

It seems more like that there is a need to change the way the Players implementations are parameterised. What do you think?

> +{
> +public:
> +    // All creation time arguments go here
> +    struct Configuration
> +    {
> +        // All creation time configuration options of the Parent class.
> +        typename Parent::Configuration parent;
> +        // The unique key identifying the player instance.
> +        Player::PlayerKey key;
> +        // Functional dependencies
> +        ClientDeathObserver::Ptr client_death_observer;
> +        power::StateController::Ptr power_state_controller;
> +    };
> +
> +    ManagedPlayerImplementation(const Configuration& configuration);
> +    ~ManagedPlayerImplementation();
> +
> +    virtual std::string uuid() const;
> +    virtual void reconnect();
> +    virtual void abandon();
> +
> +    virtual std::shared_ptr<TrackList> track_list();
> +    virtual Player::PlayerKey key() const;
> +
> +    virtual video::Sink::Ptr create_gl_texture_video_sink(std::uint32_t texture_id);
> +
> +    virtual bool open_uri(const Track::UriType& uri);
> +    virtual bool open_uri(const Track::UriType& uri, const Player::HeadersType& headers);
> +    virtual void next();
> +    virtual void previous();
> +    virtual void play();
> +    virtual void pause();
> +    virtual void stop();
> +    virtual void seek_to(const std::chrono::microseconds& offset);
> +
> +    const core::Signal<>& on_client_disconnected() const;
> +
> +protected:
> +    void emit_playback_status_changed(const Player::PlaybackStatus &status);
> +
> +private:
> +    struct Private;
> +    std::shared_ptr<Private> d;
> +};
> +
> +}
> +}
> +}
> +#endif // CORE_UBUNTU_MANAGED_MEDIA_PLAYER_IMPLEMENTATION_H_


-- 
https://code.launchpad.net/~justinmcp/media-hub/proxy-player/+merge/268862
Your team Ubuntu Phablet Team is subscribed to branch lp:media-hub.