[Merge] lp:~thomas-voss/media-hub/introduce-client-death-observer-interface into lp:media-hub

Thomas Voß thomas.voss at canonical.com
Tue Dec 9 16:05:13 UTC 2014



Diff comments:

> === modified file 'src/core/media/CMakeLists.txt'
> --- src/core/media/CMakeLists.txt	2014-11-26 12:41:08 +0000
> +++ src/core/media/CMakeLists.txt	2014-11-26 12:41:08 +0000
> @@ -86,6 +86,8 @@
>      ${MEDIA_HUB_HEADERS}
>      ${MPRIS_HEADERS}
>  
> +    client_death_observer.cpp
> +    hybris_client_death_observer.cpp
>      cover_art_resolver.cpp
>      engine.cpp
>      gstreamer/engine.cpp
> 
> === added file 'src/core/media/client_death_observer.cpp'
> --- src/core/media/client_death_observer.cpp	1970-01-01 00:00:00 +0000
> +++ src/core/media/client_death_observer.cpp	2014-11-26 12:41:08 +0000
> @@ -0,0 +1,42 @@
> +/*
> + * Copyright © 2014 Canonical Ltd.
> + *
> + * This program is free software: you can redistribute it and/or modify it
> + * under the terms of the GNU Lesser General Public License version 3,
> + * as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + *
> + * Authored by: Thomas Voß <thomas.voss at canonical.com>
> + */
> +
> +#include <core/media/client_death_observer.h>
> +
> +namespace media = core::ubuntu::media;
> +
> +#if defined(MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER)
> +#include <core/media/hybris_client_death_observer.h>
> +
> +// Accesses the default client death observer implementation for the platform.
> +media::ClientDeathObserver::Ptr media::platform_default_client_death_observer()
> +{
> +    return media::HybrisClientDeathObserver::create();
> +}
> +#else  // MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER
> +// Just throws a std::logic_error as we have not yet defined a default way to
> +// identify client death changes. One possible way of implementing the interface
> +// would be to listen to dbus name changes and react accordingly.
> +media::ClientDeathObserver::Ptr media::platform_default_client_death_observer()
> +{
> +    throw std::logic_error
> +    {
> +        "No platform-specific death observer implementation known."
> +    };
> +}
> +#endif // MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER
> 
> === added file 'src/core/media/client_death_observer.h'
> --- src/core/media/client_death_observer.h	1970-01-01 00:00:00 +0000
> +++ src/core/media/client_death_observer.h	2014-11-26 12:41:08 +0000
> @@ -0,0 +1,62 @@
> +/*
> + * Copyright © 2014 Canonical Ltd.
> + *
> + * This program is free software: you can redistribute it and/or modify it
> + * under the terms of the GNU Lesser General Public License version 3,
> + * as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + *
> + * Authored by: Thomas Voß <thomas.voss at canonical.com>
> + */
> +
> +#ifndef CORE_UBUNTU_MEDIA_CLIENT_DEATH_OBSERVER_H_
> +#define CORE_UBUNTU_MEDIA_CLIENT_DEATH_OBSERVER_H_
> +
> +#include <core/media/player.h>
> +
> +#include <core/signal.h>
> +
> +#include <memory>
> +
> +namespace core
> +{
> +namespace ubuntu
> +{
> +namespace media
> +{
> +// Models functionality to be notified whenever a client
> +// of the service goes away, and thus allows us to clean
> +// up in that case.
> +struct ClientDeathObserver
> +{
> +    // To save us some typing.
> +    typedef std::shared_ptr<ClientDeathObserver> Ptr;
> +
> +    ClientDeathObserver() = default;
> +    ClientDeathObserver(const ClientDeathObserver&) = delete;
> +    virtual ~ClientDeathObserver() = default;
> +
> +    ClientDeathObserver& operator=(const ClientDeathObserver&) = delete;
> +
> +    // Registers the client with the given key for death notifications.
> +    virtual void register_for_death_notifications_with_key(const Player::PlayerKey&) = 0;
> +
> +    // Emitted whenver a client dies, reporting the key under which the
> +    // respective client was known.
> +    virtual const core::Signal<Player::PlayerKey>& on_client_with_key_died() const = 0;
> +};
> +
> +// Accesses the default client death observer implementation for the platform.
> +ClientDeathObserver::Ptr platform_default_client_death_observer();
> +}
> +}
> +}
> +
> +#endif // CORE_UBUNTU_MEDIA_CLIENT_DEATH_OBSERVER_H_
> 
> === added file 'src/core/media/hybris_client_death_observer.cpp'
> --- src/core/media/hybris_client_death_observer.cpp	1970-01-01 00:00:00 +0000
> +++ src/core/media/hybris_client_death_observer.cpp	2014-11-26 12:41:08 +0000
> @@ -0,0 +1,84 @@
> +/*
> + * Copyright © 2014 Canonical Ltd.
> + *
> + * This program is free software: you can redistribute it and/or modify it
> + * under the terms of the GNU Lesser General Public License version 3,
> + * as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + *
> + * Authored by: Thomas Voß <thomas.voss at canonical.com>
> + */
> +
> +#include <core/media/hybris_client_death_observer.h>
> +
> +namespace media = core::ubuntu::media;
> +
> +#if defined(MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER)
> +#include <hybris/media/media_codec_layer.h>
> +
> +namespace
> +{
> +typedef std::pair<media::Player::PlayerKey, std::weak_ptr<media::HybrisClientDeathObserver>> Holder;
> +}
> +
> +void media::HybrisClientDeathObserver::on_client_died_cb(void* context)
> +{
> +    auto holder = static_cast<Holder*>(context);
> +
> +    if (not holder)
> +        return;
> +
> +    // We check if we are still alive or if we already got killed.
> +    if (auto sp = holder->second.lock())

Yup, in addition: On destruction, when the HybrisClientDeathObserver instance goes away, this makes sure that we do not call into the dead object and that we do not keep it alive for longer than necessary.

> +    {
> +        sp->client_with_key_died(holder->first);
> +    }
> +
> +    // And with that, we have reached end of life for our holder object.
> +    delete holder;
> +}
> +
> +// Creates an instance of the HybrisClientDeathObserver or throws
> +// if the underlying platform does not support it.
> +media::ClientDeathObserver::Ptr media::HybrisClientDeathObserver::create()
> +{
> +    return media::ClientDeathObserver::Ptr{new media::HybrisClientDeathObserver{}};
> +}
> +
> +media::HybrisClientDeathObserver::HybrisClientDeathObserver()
> +{
> +}
> +
> +media::HybrisClientDeathObserver::~HybrisClientDeathObserver()
> +{
> +}
> +
> +void media::HybrisClientDeathObserver::register_for_death_notifications_with_key(const media::Player::PlayerKey& key)
> +{
> +    decoding_service_set_client_death_cb(&media::HybrisClientDeathObserver::on_client_died_cb, key, new Holder{key, shared_from_this()});
> +}
> +
> +const core::Signal<media::Player::PlayerKey>& media::HybrisClientDeathObserver::on_client_with_key_died() const
> +{
> +    return client_with_key_died;
> +}
> +#else  // MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER
> +// Creates an instance of the HybrisClientDeathObserver or throws
> +// if the underlying platform does not support it.
> +media::ClientDeathObserver::Ptr media::HybrisClientDeathObserver::create()
> +{
> +    throw std::logic_error
> +    {
> +        "Hybris-based death observer implementation not supported on this platform."
> +    };
> +}
> +#endif // MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER
> +
> +
> 
> === added file 'src/core/media/hybris_client_death_observer.h'
> --- src/core/media/hybris_client_death_observer.h	1970-01-01 00:00:00 +0000
> +++ src/core/media/hybris_client_death_observer.h	2014-11-26 12:41:08 +0000
> @@ -0,0 +1,63 @@
> +/*
> + * Copyright © 2014 Canonical Ltd.
> + *
> + * This program is free software: you can redistribute it and/or modify it
> + * under the terms of the GNU Lesser General Public License version 3,
> + * as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + *
> + * Authored by: Thomas Voß <thomas.voss at canonical.com>
> + */
> +
> +#ifndef CORE_UBUNTU_MEDIA_HYBRIS_CLIENT_DEATH_OBSERVER_H_
> +#define CORE_UBUNTU_MEDIA_HYBRIS_CLIENT_DEATH_OBSERVER_H_
> +
> +#include <core/media/client_death_observer.h>
> +
> +namespace core
> +{
> +namespace ubuntu
> +{
> +namespace media
> +{
> +// Models functionality to be notified whenever a client
> +// of the service goes away, and thus allows us to clean
> +// up in that case.
> +class HybrisClientDeathObserver : public ClientDeathObserver,
> +                                  public std::enable_shared_from_this<HybrisClientDeathObserver>
> +{
> +public:
> +    // Our static callback that we inject to the hybris world.
> +    static void on_client_died_cb(void* context);
> +
> +    // Creates an instance of the HybrisClientDeathObserver or throws
> +    // if the underlying platform does not support it.
> +    static ClientDeathObserver::Ptr create();
> +
> +    // Make std::unique_ptr happy for forward declared Private internals.
> +    ~HybrisClientDeathObserver();
> +
> +    // Registers the client with the given key for death notifications.
> +    void register_for_death_notifications_with_key(const Player::PlayerKey&) override;
> +
> +    // Emitted whenver a client dies, reporting the key under which the
> +    // respective client was known.
> +    const core::Signal<Player::PlayerKey>& on_client_with_key_died() const override;
> +
> +private:
> +    HybrisClientDeathObserver();
> +
> +    core::Signal<media::Player::PlayerKey> client_with_key_died;
> +};
> +}
> +}
> +}
> +
> +#endif // CORE_UBUNTU_MEDIA_HYBRIS_CLIENT_DEATH_OBSERVER_H_
> 
> === modified file 'src/core/media/player_implementation.cpp'
> --- src/core/media/player_implementation.cpp	2014-11-26 12:41:08 +0000
> +++ src/core/media/player_implementation.cpp	2014-11-26 12:41:08 +0000
> @@ -21,10 +21,10 @@
>  
>  #include <unistd.h>
>  
> +#include "client_death_observer.h"
>  #include "engine.h"
>  #include "track_list_implementation.h"
>  
> -#include <hybris/media/media_codec_layer.h>
>  #include "powerd_service.h"
>  #include "unity_screen_service.h"
>  #include "gstreamer/engine.h"
> @@ -81,7 +81,16 @@
>          auto uscreen_stub_service = dbus::Service::use_service(bus, dbus::traits::Service<core::UScreen>::interface_name());
>          uscreen_session = uscreen_stub_service->object_for_path(dbus::types::ObjectPath("/com/canonical/Unity/Screen"));
>  
> -        decoding_service_set_client_death_cb(&Private::on_client_died_cb, key, static_cast<void*>(this));
> +        auto client_death_observer = media::platform_default_client_death_observer();
> +
> +        client_death_observer->register_for_death_notifications_with_key(key);
> +        client_death_observer->on_client_with_key_died().connect([this](const media::Player::PlayerKey& died)
> +        {
> +            if (died != this->key)
> +                return;
> +
> +            on_client_died();
> +        });
>      }
>  
>      ~Private()
> @@ -261,15 +270,6 @@
>          };
>      }
>  
> -    static void on_client_died_cb(void *context)
> -    {
> -        if (context)
> -        {
> -            Private *p = static_cast<Private*>(context);
> -            p->on_client_died();
> -        }
> -    }
> -
>      void on_client_died()
>      {
>          engine->reset();
> 
> === modified file 'src/core/media/player_stub.cpp'
> --- src/core/media/player_stub.cpp	2014-11-26 12:41:08 +0000
> +++ src/core/media/player_stub.cpp	2014-11-26 12:41:08 +0000
> @@ -33,10 +33,6 @@
>  #include <core/dbus/property.h>
>  #include <core/dbus/types/object_path.h>
>  
> -// Hybris
> -#include <hybris/media/media_codec_layer.h>
> -#include <hybris/media/surface_texture_client_hybris.h>
> -
>  #include <limits>
>  
>  #define UNUSED __attribute__((unused))
> @@ -83,7 +79,6 @@
>                      object->get_signal<mpris::Player::Signals::VideoDimensionChanged>()
>                  }
>      {
> -        decoding_session = decoding_service_create_session(key);
>      }
>  
>      ~Private()
> @@ -93,8 +88,6 @@
>      std::shared_ptr<Service> parent;
>      std::shared_ptr<TrackList> track_list;
>  
> -    DSSessionWrapperHybris decoding_session;
> -
>      dbus::Object::Ptr object;
>      media::Player::PlayerKey key;
>      struct
> 
> === modified file 'tests/unit-tests/CMakeLists.txt'
> --- tests/unit-tests/CMakeLists.txt	2014-11-12 19:10:24 +0000
> +++ tests/unit-tests/CMakeLists.txt	2014-11-26 12:41:08 +0000
> @@ -8,15 +8,6 @@
>      test-gstreamer-engine
>  
>      libmedia-mock.cpp
> -    ${CMAKE_SOURCE_DIR}/src/core/media/cover_art_resolver.cpp
> -    ${CMAKE_SOURCE_DIR}/src/core/media/engine.cpp
> -    ${CMAKE_SOURCE_DIR}/src/core/media/gstreamer/engine.cpp
> -    ${CMAKE_SOURCE_DIR}/src/core/media/player_skeleton.cpp
> -    ${CMAKE_SOURCE_DIR}/src/core/media/player_implementation.cpp
> -    ${CMAKE_SOURCE_DIR}/src/core/media/service_skeleton.cpp
> -    ${CMAKE_SOURCE_DIR}/src/core/media/service_implementation.cpp
> -    ${CMAKE_SOURCE_DIR}/src/core/media/track_list_skeleton.cpp
> -    ${CMAKE_SOURCE_DIR}/src/core/media/track_list_implementation.cpp
>      test-gstreamer-engine.cpp
>  )
>  
> @@ -25,6 +16,7 @@
>  
>      media-hub-common
>      media-hub-client
> +    media-hub-service
>      call-monitor
>      media-hub-test-framework
>  
> 


-- 
https://code.launchpad.net/~thomas-voss/media-hub/introduce-client-death-observer-interface/+merge/242884
Your team Ubuntu Phablet Team is subscribed to branch lp:media-hub.



More information about the Ubuntu-reviews mailing list