Merge lp:~jhodapp/media-hub/fix-music-next into lp:media-hub

[Jim Hodapp]

Allow music to advance to the next song when the device is not charging

Allow music to advance to the next song when the device is not charging

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:49
http://jenkins.qa.ubuntu.com/job/media-hub-ci/82/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-utopic-amd64-ci/24
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-utopic-armhf-ci/23
        deb: http://jenkins.qa.ubuntu.com/job/media-hub-utopic-armhf-ci/23/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-utopic-i386-ci/22

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

review: Approve (continuous-integration)

[lp:~jhodapp/media-hub/fix-music-next]

50. By Jim Hodapp on 2014-08-11

Make sure the display wakelock is cleared after video playback stops.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:50
http://jenkins.qa.ubuntu.com/job/media-hub-ci/83/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-utopic-amd64-ci/25
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-utopic-armhf-ci/24/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-utopic-i386-ci/23

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

review: Needs Fixing (continuous-integration)

[Manuel de la Peña (mandel)]

review: Approve

[lp:~jhodapp/media-hub/fix-music-next]

51. By Jim Hodapp on 2014-08-12

Rework the system and display wakelock logic. Should take care of the edge case issues.

52. By Jim Hodapp on 2014-08-12

Make sure a display or system wakelock are not held if the client disconnects during playback

53. By Jim Hodapp on 2014-08-13

Remove some debugging statements and improve the code comments

=== modified file 'README'
--- README	2014-01-13 21:51:14 +0000
+++ README	2014-08-13 01:41:13 +0000
@@ -6,6 +6,10 @@
 cmake ..
 make
 
+To Run From Console:
+--------------------
+CORE_UBUNTU_MEDIA_SERVICE_VIDEO_SINK_NAME=mirsink CORE_UBUNTU_MEDIA_SERVICE_AUDIO_SINK_NAME=pulsesink src/core/media/media-hub-server
+
 To Run Unit Tests:
 ------------------
 

=== modified file 'src/core/media/CMakeLists.txt'
--- src/core/media/CMakeLists.txt	2014-04-04 14:31:43 +0000
+++ src/core/media/CMakeLists.txt	2014-08-13 01:41:13 +0000
@@ -1,5 +1,4 @@
 pkg_check_modules(PC_GSTREAMER_1_0 REQUIRED gstreamer-1.0)
-
 include_directories(${PC_GSTREAMER_1_0_INCLUDE_DIRS} ${HYBRIS_MEDIA_CFLAGS})
 
 add_library(

=== modified file 'src/core/media/engine.h'
--- src/core/media/engine.h	2014-04-25 17:53:00 +0000
+++ src/core/media/engine.h	2014-08-13 01:41:13 +0000
@@ -118,6 +118,7 @@
 
     virtual const core::Signal<void>& about_to_finish_signal() const = 0;
     virtual const core::Signal<uint64_t>& seeked_to_signal() const = 0;
+    virtual const core::Signal<void>& client_disconnected_signal() const = 0;
     virtual const core::Signal<void>& end_of_stream_signal() const = 0;
     virtual const core::Signal<core::ubuntu::media::Player::PlaybackStatus>& playback_status_changed_signal() const = 0;
 };

=== modified file 'src/core/media/gstreamer/engine.cpp'
--- src/core/media/gstreamer/engine.cpp	2014-04-25 17:53:00 +0000
+++ src/core/media/gstreamer/engine.cpp	2014-08-13 01:41:13 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright © 2013 Canonical Ltd.
+ * Copyright © 2013-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,
@@ -175,6 +175,11 @@
         seeked_to(value);
     }
 
+    void on_client_disconnected()
+    {
+        client_disconnected();
+    }
+
     void on_end_of_stream()
     {
         end_of_stream();
@@ -214,6 +219,11 @@
                       &Private::on_seeked_to,
                       this,
                       std::placeholders::_1))),
+          client_disconnected_connection(
+              playbin.signals.client_disconnected.connect(
+                  std::bind(
+                      &Private::on_client_disconnected,
+                      this))),
           on_end_of_stream_connection(
               playbin.signals.on_end_of_stream.connect(
                   std::bind(
@@ -236,10 +246,12 @@
     core::ScopedConnection on_tag_available_connection;
     core::ScopedConnection on_volume_changed_connection;
     core::ScopedConnection on_seeked_to_connection;
+    core::ScopedConnection client_disconnected_connection;
     core::ScopedConnection on_end_of_stream_connection;
 
     core::Signal<void> about_to_finish;
     core::Signal<uint64_t> seeked_to;
+    core::Signal<void> client_disconnected;
     core::Signal<void> end_of_stream;
     core::Signal<media::Player::PlaybackStatus> playback_status_changed;
 };
@@ -383,6 +395,11 @@
     return d->seeked_to;
 }
 
+const core::Signal<void>& gstreamer::Engine::client_disconnected_signal() const
+{
+    return d->client_disconnected;
+}
+
 const core::Signal<void>& gstreamer::Engine::end_of_stream_signal() const
 {
     return d->end_of_stream;

=== modified file 'src/core/media/gstreamer/engine.h'
--- src/core/media/gstreamer/engine.h	2014-04-25 17:53:00 +0000
+++ src/core/media/gstreamer/engine.h	2014-08-13 01:41:13 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright © 2013 Canonical Ltd.
+ * Copyright © 2013-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,
@@ -53,6 +53,7 @@
 
     const core::Signal<void>& about_to_finish_signal() const;
     const core::Signal<uint64_t>& seeked_to_signal() const;
+    const core::Signal<void>& client_disconnected_signal() const;
     const core::Signal<void>& end_of_stream_signal() const;
     const core::Signal<core::ubuntu::media::Player::PlaybackStatus>& playback_status_changed_signal() const;
 

=== modified file 'src/core/media/gstreamer/playbin.h'
--- src/core/media/gstreamer/playbin.h	2014-04-25 21:23:10 +0000
+++ src/core/media/gstreamer/playbin.h	2014-08-13 01:41:13 +0000
@@ -116,6 +116,8 @@
         // in a state that is ready for the next client that connects to the
         // service
         reset_pipeline();
+        // Signal to the Player class that the client side has disconnected
+        signals.client_disconnected();
     }
 
     void reset_pipeline()
@@ -411,6 +413,7 @@
         core::Signal<uint64_t> on_seeked_to;
         core::Signal<void> on_end_of_stream;
         core::Signal<media::Player::PlaybackStatus> on_playback_status_changed;
+        core::Signal<void> client_disconnected;
     } signals;
 };
 }

=== modified file 'src/core/media/player_implementation.cpp'
--- src/core/media/player_implementation.cpp	2014-06-24 17:03:27 +0000
+++ src/core/media/player_implementation.cpp	2014-08-13 01:41:13 +0000
@@ -13,9 +13,11 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
  * Authored by: Thomas Voß <[e-mail]>
+ *              Jim Hodapp <[e-mail]>
  */
 
 #include "player_implementation.h"
+#include "util/timeout.h"
 
 #include <unistd.h>
 
@@ -28,6 +30,7 @@
 
 #include <exception>
 #include <iostream>
+#include <mutex>
 
 #define UNUSED __attribute__((unused))
 
@@ -38,6 +41,14 @@
 
 struct media::PlayerImplementation::Private
 {
+    enum class wakelock_clear_t
+    {
+        WAKELOCK_CLEAR_INACTIVE,
+        WAKELOCK_CLEAR_DISPLAY,
+        WAKELOCK_CLEAR_SYSTEM,
+        WAKELOCK_CLEAR_INVALID
+    };
+
     Private(PlayerImplementation* parent,
             const dbus::types::ObjectPath& session_path,
             const std::shared_ptr<media::Service>& service,
@@ -51,7 +62,10 @@
                   session_path.as_string() + "/TrackList",
                   engine->meta_data_extractor())),
           sys_lock_name("media-hub-music-playback"),
-          active_display_on_request(false),
+          disp_cookie(-1),
+          system_wakelock_count(0),
+          display_wakelock_count(0),
+          previous_state(Engine::State::stopped),
           key(key)
     {
         auto bus = std::shared_ptr<dbus::Bus>(new dbus::Bus(core::dbus::WellKnownBus::system));
@@ -63,6 +77,14 @@
         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"));
 
+        /*
+         * Wakelock state logic:
+         *
+         * PLAYING->READY: delay 4 seconds and try to clear current wakelock type
+         * PLAYING->PAUSED or PLAYING->STOPPED: delay 4 seconds and try to clear current wakelock type
+         * READY->PAUSED: request a new wakelock (system or display)
+         * PLAYING->PAUSED: delay 4 seconds and try to clear current wakelock type
+         */
         engine->state().changed().connect(
                     [parent, this](const Engine::State& state)
         {
@@ -71,97 +93,178 @@
             case Engine::State::ready:
             {
                 parent->playback_status().set(media::Player::ready);
-                clear_power_state();
+                if (previous_state == Engine::State::playing)
+                {
+                    wakelock_timeout.reset(new timeout(4000, true, std::bind(&Private::clear_wakelock,
+                                    this, std::placeholders::_1), current_wakelock_type()));
+                }
                 break;
             }
             case Engine::State::playing:
             {
                 parent->playback_status().set(media::Player::playing);
-                request_power_state();
+                if (previous_state == Engine::State::stopped || previous_state == Engine::State::paused)
+                {
+                    request_power_state();
+                }
                 break;
             }
             case Engine::State::stopped:
             {
                 parent->playback_status().set(media::Player::stopped);
-                clear_power_state();
                 break;
             }
             case Engine::State::paused:
             {
                 parent->playback_status().set(media::Player::paused);
-                clear_power_state();
+                if (previous_state == Engine::State::ready)
+                {
+                    request_power_state();
+                }
+                else if (previous_state == Engine::State::playing)
+                {
+                    wakelock_timeout.reset(new timeout(4000, true, std::bind(&Private::clear_wakelock,
+                                    this, std::placeholders::_1), current_wakelock_type()));
+                }
                 break;
             }
             default:
                 break;
             };
+
+            // Keep track of the previous Engine playback state:
+            previous_state = state;
         });
 
     }
 
+    ~Private()
+    {
+        // Make sure that we don't hold on to the wakelocks if media-hub-server
+        // ever gets restarted manually or automatically
+        clear_wakelocks();
+    }
+
     void request_power_state()
     {
         try
         {
             if (parent->is_video_source())
             {
-                if (!active_display_on_request)
+                if (display_wakelock_count == 0)
                 {
                     auto result = uscreen_session->invoke_method_synchronously<core::UScreen::keepDisplayOn, int>();
                     if (result.is_error())
                         throw std::runtime_error(result.error().print());
-
                     disp_cookie = result.value();
-                    active_display_on_request = true;
+                    cout << "Requested new display wakelock" << endl;
+                }
+
+                {
+                    // Keep track of how many display wakelocks have been requested
+                    std::lock_guard<std::mutex> lock(wakelock_mutex);
+                    ++display_wakelock_count;
                 }
             }
             else
             {
-                if (sys_cookie.empty())
+                if (system_wakelock_count == 0)
                 {
                     auto result = powerd_session->invoke_method_synchronously<core::Powerd::requestSysState, std::string>(sys_lock_name, static_cast<int>(1));
                     if (result.is_error())
                         throw std::runtime_error(result.error().print());
-
                     sys_cookie = result.value();
+                    cout << "Requested new system wakelock" << endl;
+                }
+
+                {
+                    // Keep track of how many system wakelocks have been requested
+                    std::lock_guard<std::mutex> lock(wakelock_mutex);
+                    ++system_wakelock_count;
                 }
             }
         }
-        catch(std::exception& e)
+        catch(const std::exception& e)
         {
             std::cerr << "Warning: failed to request power state: ";
             std::cerr << e.what() << std::endl;
         }
     }
 
-    void clear_power_state()
+    void clear_wakelock(const wakelock_clear_t &wakelock)
     {
+        cout << __PRETTY_FUNCTION__ << endl;
         try
         {
-            if (parent->is_video_source())
-            {
-                if (active_display_on_request)
-                {
-                    uscreen_session->invoke_method_synchronously<core::UScreen::removeDisplayOnRequest, void>(disp_cookie);
-                    active_display_on_request = false;
-                }
-            }
-            else
-            {
-                if (!sys_cookie.empty())
-                {
-                    powerd_session->invoke_method_synchronously<core::Powerd::clearSysState, void>(sys_cookie);
-                    sys_cookie.clear();
-                }
+            switch (wakelock)
+            {
+                case wakelock_clear_t::WAKELOCK_CLEAR_INACTIVE:
+                    break;
+                case wakelock_clear_t::WAKELOCK_CLEAR_SYSTEM:
+                    {
+                        std::lock_guard<std::mutex> lock(wakelock_mutex);
+                        --system_wakelock_count;
+                    }
+                    // Only actually clear the system wakelock once the count reaches zero
+                    if (system_wakelock_count == 0)
+                    {
+                        cout << "Clearing system wakelock" << endl;
+                        powerd_session->invoke_method_synchronously<core::Powerd::clearSysState, void>(sys_cookie);
+                        sys_cookie.clear();
+                    }
+                    break;
+                case wakelock_clear_t::WAKELOCK_CLEAR_DISPLAY:
+                    {
+                        std::lock_guard<std::mutex> lock(wakelock_mutex);
+                        --display_wakelock_count;
+                    }
+                    // Only actually clear the display wakelock once the count reaches zero
+                    if (display_wakelock_count == 0)
+                    {
+                        cout << "Clearing display wakelock" << endl;
+                        uscreen_session->invoke_method_synchronously<core::UScreen::removeDisplayOnRequest, void>(disp_cookie);
+                        disp_cookie = -1;
+                    }
+                    break;
+                case wakelock_clear_t::WAKELOCK_CLEAR_INVALID:
+                default:
+                    cerr << "Can't clear invalid wakelock type" << endl;
             }
         }
-        catch(std::exception& e)
+        catch(const std::exception& e)
         {
             std::cerr << "Warning: failed to clear power state: ";
             std::cerr << e.what() << std::endl;
         }
     }
 
+    wakelock_clear_t current_wakelock_type() const
+    {
+        return (parent->is_video_source()) ?
+            wakelock_clear_t::WAKELOCK_CLEAR_DISPLAY : wakelock_clear_t::WAKELOCK_CLEAR_SYSTEM;
+    }
+
+    void clear_wakelocks()
+    {
+        // Clear both types of wakelocks (display and system)
+        if (system_wakelock_count > 0)
+        {
+            {
+                std::lock_guard<std::mutex> lock(wakelock_mutex);
+                system_wakelock_count = 1;
+            }
+            clear_wakelock(wakelock_clear_t::WAKELOCK_CLEAR_SYSTEM);
+        }
+        if (display_wakelock_count > 0)
+        {
+            {
+                std::lock_guard<std::mutex> lock(wakelock_mutex);
+                display_wakelock_count = 1;
+            }
+            clear_wakelock(wakelock_clear_t::WAKELOCK_CLEAR_DISPLAY);
+        }
+    }
+
     PlayerImplementation* parent;
     std::shared_ptr<Service> service;
     std::shared_ptr<Engine> engine;
@@ -171,8 +274,12 @@
     std::shared_ptr<dbus::Object> uscreen_session;
     std::string sys_lock_name;
     int disp_cookie;
-    bool active_display_on_request;
     std::string sys_cookie;
+    std::mutex wakelock_mutex;
+    uint8_t system_wakelock_count;
+    uint8_t display_wakelock_count;
+    std::unique_ptr<timeout> wakelock_timeout;
+    Engine::State previous_state;
     PlayerImplementation::PlayerKey key;
 };
 
@@ -240,6 +347,13 @@
         }
     });
 
+    d->engine->client_disconnected_signal().connect([this]()
+    {
+        // If the client disconnects, make sure both wakelock types
+        // are cleared
+        d->clear_wakelocks();
+    });
+
     d->engine->seeked_to_signal().connect([this](uint64_t value)
     {
         seeked_to()(value);

=== modified file 'src/core/media/powerd_service.h'
--- src/core/media/powerd_service.h	2014-06-16 22:28:53 +0000
+++ src/core/media/powerd_service.h	2014-08-13 01:41:13 +0000
@@ -41,7 +41,7 @@
         static std::string s = "com.canonical.powerd";
         return s;
     }
-    
+
     struct requestSysState
     {
         static std::string name()
@@ -62,7 +62,7 @@
             static std::string s = "clearSysState";
             return s;
         }
- 
+
         static const std::chrono::milliseconds default_timeout() { return std::chrono::seconds{1}; }
 
         typedef Powerd Interface;

=== added directory 'src/core/media/util'
=== added file 'src/core/media/util/timeout.h'
--- src/core/media/util/timeout.h	1970-01-01 00:00:00 +0000
+++ src/core/media/util/timeout.h	2014-08-13 01:41:13 +0000
@@ -0,0 +1,73 @@
+/*
+ *
+ * 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: Jim Hodapp <[e-mail]>
+ *
+ * Adapted from: http://stackoverflow.com/questions/14650885/how-to-create-timer-events-using-c-11
+ *
+ */
+
+#ifndef TIMEOUT_H_
+#define TIMEOUT_H_
+
+#include <functional>
+#include <chrono>
+#include <future>
+#include <cstdio>
+
+namespace core
+{
+namespace ubuntu
+{
+namespace media
+{
+
+class timeout
+{
+public:
+    /**
+     * @brief Start a timeout with millisecond resolution
+     * @param timeout_ms Timeout value in milliseconds
+     * @param async Whether to block while the timeout proceeds or not
+     * @param f The function to call when the timeout expires
+     * @param args Any arguments to pass to f when the timeout expires
+     */
+    template <class callable, class... arguments>
+    timeout(int timeout_ms, bool async, callable&& f, arguments&&... args)
+    {
+        std::function<typename std::result_of<callable(arguments...)>::type()>
+            task(std::bind(std::forward<callable>(f), std::forward<arguments>(args)...));
+
+        if (async)
+        {
+            // Timeout without blocking
+            std::thread([timeout_ms, task]() {
+                std::this_thread::sleep_for(std::chrono::milliseconds(timeout_ms));
+                task();
+            }).detach();
+        }
+        else
+        {
+            // Timeout with blocking
+            std::this_thread::sleep_for(std::chrono::milliseconds(timeout_ms));
+            task();
+        }
+    }
+};
+
+}
+}
+}
+
+#endif // TIMEOUT_H_