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

[Thomas Voß]

Add an interface media::ClientDeathObserver that abstracts away receiving key-based death notifications for clients associated to media::Player instances server-side
Provide an implementation media::HybrisClientDeathObserver that relies on hybris and ultimately on Android's onBinderDied to receive death notifications.
Adjust media::PlayerStub and media::PlayerImplementation to account for the new interface.
Adjust the CMake setup for tests to link media-hub-service instead of recompiling large parts of the implementation classes.

Add an interface media::ClientDeathObserver that abstracts away receiving key-based death notifications for clients associated to media::Player instances server-side
Provide an implementation media::HybrisClientDeathObserver that relies on hybris and ultimately on Android's onBinderDied to receive death notifications.
Adjust media::PlayerStub and media::PlayerImplementation to account for the new interface.
Adjust the CMake setup for tests to link media-hub-service instead of recompiling large parts of the implementation classes.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:101
http://jenkins.qa.ubuntu.com/job/media-hub-ci/178/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/18/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/18/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/18/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/178/rebuild

review: Needs Fixing (continuous-integration)

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

102. By Thomas Voß on 2014-11-26

Remerge prereq branch.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:102
http://jenkins.qa.ubuntu.com/job/media-hub-ci/191/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/31/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/31/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/31/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/191/rebuild

review: Needs Fixing (continuous-integration)

[Jim Hodapp (jhodapp)]

Looks good, 4 minor comments inline below.

review: Needs Fixing (code)

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

103. By Thomas Voß on 2014-12-09

[ Jim Hodapp ]
* Resubmitting with prerequisite branch (LP: #1331041)
[ Justin McPherson ]
* Resubmitting with prerequisite branch (LP: #1331041)

104. By Thomas Voß on 2014-12-09

Address reviewer comments.

[Jim Hodapp (jhodapp)]

Looks good.

review: Approve (code)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:104
http://jenkins.qa.ubuntu.com/job/media-hub-ci/201/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/41
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/41
        deb: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/41/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/41

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/201/rebuild

review: Approve (continuous-integration)

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

105. By Thomas Voß on 2015-01-26

[ Jim Hodapp ]
* Error reporting all the way up to the app level from the playbin
  pipeline.
[ Ubuntu daily release ]
* New rebuild forced
[ Jim Hodapp ]
* Don't auto-resume playback of videos after a phone call ends. (LP:
  #1411273)
[ Ubuntu daily release ]
* New rebuild forced
[ Ricardo Salveti de Araujo ]
* service_implementation: adding debug for call started/ended signals.
  Make sure account and connection are available when setting up
  account manager (patch from Gustavo Boiko). call_monitor: don't
  check caps when hooking up on/off signals, until bug 1409125 is
  fixed. Enable parallel building . (LP: #1409125)
[ Jim Hodapp ]
* Pause playback when recording begins. (LP: #1398047)
[ Ricardo Salveti de Araujo ]
* call_monitor.cpp: waiting for bridge to be up, and also protecting
  the on_change call (LP: #1408137)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:105
http://jenkins.qa.ubuntu.com/job/media-hub-ci/229/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/69/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/69/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/69/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/229/rebuild

review: Needs Fixing (continuous-integration)

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

106. By Thomas Voß on 2015-01-26

Merge prereq branch.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:106
http://jenkins.qa.ubuntu.com/job/media-hub-ci/238/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/78
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/78
        deb: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/78/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/78/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/238/rebuild

review: Needs Fixing (continuous-integration)

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

107. By Thomas Voß on 2015-01-29

Merge prereq branch.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:107
http://jenkins.qa.ubuntu.com/job/media-hub-ci/259/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/99
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/99
        deb: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/99/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/99

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/259/rebuild

review: Approve (continuous-integration)

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

108. By Thomas Voß on 2015-03-12

* debian/control:
  - Removing pre-depends that are not required
  - Bumping standards-version to 3.9.6
[ Ricardo Salveti de Araujo ]
* Migrating tests to use ogg instead of mp3/avi removed:
  tests/h264.avi tests/test.mp3 added: tests/test-audio-1.ogg
  tests/test-video.ogg tests/test.mp3 renamed: tests/test.ogg =>
  tests/test-audio.ogg

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:108
http://jenkins.qa.ubuntu.com/job/media-hub-ci/314/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/154
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/154
        deb: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/154/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/154

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/314/rebuild

review: Approve (continuous-integration)

=== modified file 'src/core/media/CMakeLists.txt'
--- src/core/media/CMakeLists.txt	2015-03-12 11:40:36 +0000
+++ src/core/media/CMakeLists.txt	2015-03-12 11:40:36 +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	2015-03-12 11:40:36 +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ß <[e-mail]>
+ */
+
+#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	2015-03-12 11:40:36 +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ß <[e-mail]>
+ */
+
+#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 whenever 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	2015-03-12 11:40:36 +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ß <[e-mail]>
+ */
+
+#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())
+    {
+        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	2015-03-12 11:40:36 +0000
@@ -0,0 +1,64 @@
+/*
+ * 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ß <[e-mail]>
+ */
+
+#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.
+// Specific implementation for a hybris-based platform.
+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 whenever 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	2015-03-12 11:40:36 +0000
+++ src/core/media/player_implementation.cpp	2015-03-12 11:40:36 +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	2015-03-12 11:40:36 +0000
+++ src/core/media/player_stub.cpp	2015-03-12 11:40:36 +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))

=== modified file 'tests/unit-tests/CMakeLists.txt'
--- tests/unit-tests/CMakeLists.txt	2014-11-18 20:29:26 +0000
+++ tests/unit-tests/CMakeLists.txt	2015-03-12 11:40:36 +0000
@@ -13,15 +13,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
 )
 
@@ -30,6 +21,7 @@
 
     media-hub-common
     media-hub-client
+    media-hub-service
     call-monitor
     media-hub-test-framework