From 99d138d105d4ff61d2f324a6643335f3d0c9a5df Mon Sep 17 00:00:00 2001
From: philippegorley <philippe.gorley@savoirfairelinux.com>
Date: Wed, 19 Oct 2016 14:19:10 -0400
Subject: [PATCH] hardware acceleration configuration setting

Hardware acceleration is now correctly enabled/disabled using Ring's
config file. The setting was moved from DeviceParams/VideoSettings,
which are used for local video, to preferences under video.

The video preferences object calls serialize() on the video device
preferences, so as to not overwrite one another (they're under the
same section in the config file).

Change-Id: I8c11ea8813dee5cc514a4fa4c1bae962a14577eb
Tuleap: #1122
---
 src/manager.cpp                          | 21 +++++++++++++----
 src/manager.h                            |  9 ++++++++
 src/media/media_decoder.cpp              |  5 +++-
 src/media/media_decoder.h                |  1 +
 src/media/media_device.h                 |  1 -
 src/media/video/video_base.cpp           | 10 +-------
 src/media/video/video_base.h             |  1 -
 src/media/video/video_device.h           |  4 ----
 src/media/video/video_device_monitor.cpp |  2 --
 src/media/video/video_receive_thread.cpp |  8 +++++--
 src/media/video/video_receive_thread.h   |  3 ++-
 src/media/video/video_rtp_session.cpp    |  8 +++----
 src/preferences.cpp                      | 29 ++++++++++++++++++++++++
 src/preferences.h                        | 20 ++++++++++++++++
 14 files changed, 93 insertions(+), 29 deletions(-)

diff --git a/src/manager.cpp b/src/manager.cpp
index 12ef2550e3..1a89ba7ef9 100644
--- a/src/manager.cpp
+++ b/src/manager.cpp
@@ -229,8 +229,9 @@ Manager::setAutoAnswer(bool enable)
 Manager::Manager() :
     pluginManager_(new PluginManager)
     , preferences(), voipPreferences(),
-    hookPreference(),  audioPreference(), shortcutPreferences(),
-    hasTriedToRegister_(false)
+    hookPreference(),  audioPreference(), shortcutPreferences()
+    , videoPreferences()
+    , hasTriedToRegister_(false)
     , toneCtrl_(preferences)
     , currentCallMutex_(), dtmfKey_(), dtmfBuf_(0, AudioFormat::MONO())
     , audioLayerMutex_()
@@ -1537,7 +1538,7 @@ Manager::saveConfig()
         hookPreference.serialize(out);
         audioPreference.serialize(out);
 #ifdef RING_VIDEO
-        getVideoDeviceMonitor().serialize(out);
+        videoPreferences.serialize(out);
 #endif
         shortcutPreferences.serialize(out);
 
@@ -2639,7 +2640,7 @@ Manager::loadAccountMap(const YAML::Node &node)
     int errorCount = 0;
     try {
 #ifdef RING_VIDEO
-        getVideoDeviceMonitor().unserialize(node);
+        videoPreferences.unserialize(node);
 #endif
     } catch (const YAML::Exception &e) {
         RING_ERR("%s: No video node in config file", e.what());
@@ -2919,6 +2920,18 @@ Manager::getSinkClient(const std::string& id)
             return sink;
     return nullptr;
 }
+
+bool
+Manager::getDecodingAccelerated() const
+{
+    return videoPreferences.getDecodingAccelerated();
+}
+
+void
+Manager::setDecodingAccelerated(bool isAccelerated)
+{
+    videoPreferences.setDecodingAccelerated(isAccelerated);
+}
 #endif // RING_VIDEO
 
 } // namespace ring
diff --git a/src/manager.h b/src/manager.h
index 2734334847..f472115883 100644
--- a/src/manager.h
+++ b/src/manager.h
@@ -117,6 +117,11 @@ class Manager {
          */
         ShortcutPreferences shortcutPreferences;
 
+        /**
+         * Video preferences
+         */
+        VideoPreferences videoPreferences;
+
         // Manager should not be accessed until initialized.
         // FIXME this is an evil hack!
         static std::atomic_bool initialized;
@@ -1013,6 +1018,10 @@ class Manager {
         std::shared_ptr<video::SinkClient> getSinkClient(const std::string& id);
 
         VideoManager& getVideoManager() const { return *videoManager_; }
+
+        bool getDecodingAccelerated() const;
+
+        void setDecodingAccelerated(bool isAccelerated);
 #endif // RING_VIDEO
 
         std::atomic<unsigned> dhtLogLevel {0}; // default = disable
diff --git a/src/media/media_decoder.cpp b/src/media/media_decoder.cpp
index 959e41aca7..378fcdfec4 100644
--- a/src/media/media_decoder.cpp
+++ b/src/media/media_decoder.cpp
@@ -26,6 +26,7 @@
 #include "audio/audiobuffer.h"
 #include "audio/ringbuffer.h"
 #include "audio/resampler.h"
+#include "manager.h"
 
 #if defined(RING_VIDEO) && defined(RING_ACCEL)
 #include "video/accel.h"
@@ -96,7 +97,9 @@ int MediaDecoder::openInput(const DeviceParams& params)
     RING_DBG("Trying to open device %s with format %s, pixel format %s, size %dx%d, rate %lf", params.input.c_str(),
                                                         params.format.c_str(), params.pixel_format.c_str(), params.width, params.height, params.framerate.real());
 
-    enableAccel_ = (params.enableAccel != "0");
+    // if there was a fallback to software decoding, do not enable accel
+    // it has been disabled already by the video_receive_thread
+    enableAccel_ &= Manager::instance().getDecodingAccelerated();
 
     int ret = avformat_open_input(
         &inputCtx_,
diff --git a/src/media/media_decoder.h b/src/media/media_decoder.h
index 6340f3a916..83551ea3c7 100644
--- a/src/media/media_decoder.h
+++ b/src/media/media_decoder.h
@@ -94,6 +94,7 @@ class MediaDecoder {
         int getPixelFormat() const;
 
         void setOptions(const std::map<std::string, std::string>& options);
+        void enableAccel(const bool enableAccel) { enableAccel_ = enableAccel; }
 
     private:
         NON_COPYABLE(MediaDecoder);
diff --git a/src/media/media_device.h b/src/media/media_device.h
index abf4826e03..9fb6859434 100644
--- a/src/media/media_device.h
+++ b/src/media/media_device.h
@@ -45,7 +45,6 @@ struct DeviceParams {
     std::string sdp_flags {};
     unsigned offset_x {};
     unsigned offset_y {};
-    std::string enableAccel {};
 };
 
 }
diff --git a/src/media/video/video_base.cpp b/src/media/video/video_base.cpp
index 366447a185..63d1e01457 100644
--- a/src/media/video/video_base.cpp
+++ b/src/media/video/video_base.cpp
@@ -81,7 +81,6 @@ VideoSettings::VideoSettings(const std::map<std::string, std::string>& settings)
     channel = extractString(settings, "channel");
     video_size = extractString(settings, "size");
     framerate = extractString(settings, "rate");
-    enableAccel = extractString(settings, "enableAccel");
 }
 
 std::map<std::string, std::string>
@@ -91,8 +90,7 @@ VideoSettings::to_map() const
         {"name", name},
         {"size", video_size},
         {"channel", channel},
-        {"rate", framerate},
-        {"enableAccel", enableAccel}
+        {"rate", framerate}
     };
 }
 
@@ -107,7 +105,6 @@ convert<ring::video::VideoSettings>::encode(const ring::video::VideoSettings& rh
     node["video_size"] = rhs.video_size;
     node["channel"] = rhs.channel;
     node["framerate"] = rhs.framerate;
-    node["enableAccel"] = rhs.enableAccel;
     return node;
 }
 
@@ -121,11 +118,6 @@ convert<ring::video::VideoSettings>::decode(const Node& node, ring::video::Video
     rhs.video_size = node["video_size"].as<std::string>();
     rhs.channel = node["channel"].as<std::string>();
     rhs.framerate = node["framerate"].as<std::string>();
-    // optional setting that may or may not be there
-    try {
-        rhs.enableAccel = node["enableAccel"].as<std::string>();
-    } catch (...) {}
-
     return true;
 }
 
diff --git a/src/media/video/video_base.h b/src/media/video/video_base.h
index 54e3d531f7..46d2870409 100644
--- a/src/media/video/video_base.h
+++ b/src/media/video/video_base.h
@@ -159,7 +159,6 @@ struct VideoSettings
     std::string channel {};
     std::string video_size {};
     std::string framerate {};
-    std::string enableAccel {};
 };
 
 }} // namespace ring::video
diff --git a/src/media/video/video_device.h b/src/media/video/video_device.h
index b4e9f69119..504501f810 100644
--- a/src/media/video/video_device.h
+++ b/src/media/video/video_device.h
@@ -128,8 +128,6 @@ public:
                       settings.framerate.c_str());
         }
 
-        settings.enableAccel = "1";
-
         return settings;
     }
 
@@ -143,7 +141,6 @@ public:
         settings.channel = params.channel_name;
         settings.video_size = sizeToString(params.width, params.height);
         settings.framerate = ring::to_string(params.framerate.real());
-        settings.enableAccel = params.enableAccel;
         return settings;
     }
 
@@ -162,7 +159,6 @@ public:
         params.width = size.first;
         params.height = size.second;
         params.framerate = rateFromString(settings.channel, size, settings.framerate);
-        params.enableAccel = settings.enableAccel;
         setDeviceParams(params);
     }
 
diff --git a/src/media/video/video_device_monitor.cpp b/src/media/video/video_device_monitor.cpp
index ba1cff48e7..204f33a53b 100644
--- a/src/media/video/video_device_monitor.cpp
+++ b/src/media/video/video_device_monitor.cpp
@@ -303,9 +303,7 @@ VideoDeviceMonitor::overwritePreferences(VideoSettings settings)
 void
 VideoDeviceMonitor::serialize(YAML::Emitter &out)
 {
-    out << YAML::Key << CONFIG_LABEL << YAML::Value << YAML::BeginMap;
     out << YAML::Key << "devices" << YAML::Value << preferences_;
-    out << YAML::EndMap;
 }
 
 void
diff --git a/src/media/video/video_receive_thread.cpp b/src/media/video/video_receive_thread.cpp
index e3ab0bd559..0b0d0a70e5 100644
--- a/src/media/video/video_receive_thread.cpp
+++ b/src/media/video/video_receive_thread.cpp
@@ -38,14 +38,15 @@ using std::string;
 
 VideoReceiveThread::VideoReceiveThread(const std::string& id,
                                        const std::string &sdp,
-                                       const DeviceParams& args) :
+                                       const bool isReset) :
     VideoGenerator::VideoGenerator()
-    , args_(args)
+    , args_()
     , dstWidth_(0)
     , dstHeight_(0)
     , id_(id)
     , stream_(sdp)
     , restartDecoder_(false)
+    , isReset_(isReset)
     , sdpContext_(stream_.str().size(), false, &readFunction, 0, 0, this)
     , sink_ {Manager::instance().createSinkClient(id)}
     , requestKeyFrameCallback_(0)
@@ -71,6 +72,9 @@ bool VideoReceiveThread::setup()
 {
     videoDecoder_.reset(new MediaDecoder());
 
+    // disable accel if there was a fallback to software decoding
+    videoDecoder_->enableAccel(!isReset_);
+
     dstWidth_ = args_.width;
     dstHeight_ = args_.height;
 
diff --git a/src/media/video/video_receive_thread.h b/src/media/video/video_receive_thread.h
index 57d1032de8..773acd1516 100644
--- a/src/media/video/video_receive_thread.h
+++ b/src/media/video/video_receive_thread.h
@@ -47,7 +47,7 @@ class SinkClient;
 
 class VideoReceiveThread : public VideoGenerator {
 public:
-    VideoReceiveThread(const std::string &id, const std::string &sdp, const DeviceParams& args);
+    VideoReceiveThread(const std::string &id, const std::string &sdp, const bool isReset);
     ~VideoReceiveThread();
     void startLoop();
 
@@ -79,6 +79,7 @@ private:
     std::unique_ptr<MediaIOHandle> demuxContext_;
     std::shared_ptr<SinkClient> sink_;
     std::atomic_bool restartDecoder_;
+    bool isReset_;
     void (*requestKeyFrameCallback_)(const std::string &);
     void openDecoder();
     bool decodeFrame();
diff --git a/src/media/video/video_rtp_session.cpp b/src/media/video/video_rtp_session.cpp
index 077dff5363..72b7f97e1c 100644
--- a/src/media/video/video_rtp_session.cpp
+++ b/src/media/video/video_rtp_session.cpp
@@ -129,13 +129,13 @@ VideoRtpSession::restartSender()
 void VideoRtpSession::startReceiver()
 {
     if (receive_.enabled and not receive_.holding) {
-        DeviceParams receiverArgs = {};
+        bool isReset = false;
         if (receiveThread_) {
             RING_WARN("Restarting video receiver");
-            receiverArgs.enableAccel = "0"; // most likely cause of this restart
+            isReset = true;
         }
         receiveThread_.reset(
-            new VideoReceiveThread(callID_, receive_.receiving_sdp, receiverArgs)
+            new VideoReceiveThread(callID_, receive_.receiving_sdp, isReset)
         );
         /* ebail: keyframe requests can lead to timeout if they are not answered.
          * we decided so to disable them for the moment
@@ -160,7 +160,7 @@ VideoRtpSession::restartReceiver()
     if (not socketPair_)
         return;
 
-    startReceiver(); // disable accel
+    startReceiver();
 }
 
 void VideoRtpSession::start(std::unique_ptr<IceSocket> rtp_sock,
diff --git a/src/preferences.cpp b/src/preferences.cpp
index 2a612c0a56..6b9c37ac96 100644
--- a/src/preferences.cpp
+++ b/src/preferences.cpp
@@ -45,6 +45,8 @@
 #endif
 #endif /* HAVE_OPENSL */
 
+#include "client/videomanager.h"
+
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
 #include <yaml-cpp/yaml.h>
@@ -122,6 +124,10 @@ static const char * const POPUP_SHORT_KEY = "popupWindow";
 static const char * const TOGGLE_HOLD_SHORT_KEY = "toggleHold";
 static const char * const TOGGLE_PICKUP_HANGUP_SHORT_KEY = "togglePickupHangup";
 
+// video preferences
+constexpr const char * const VideoPreferences::CONFIG_LABEL;
+static const char * const DECODING_ACCELERATED_KEY = "decodingAccelerated";
+
 static const char * const DFT_PULSE_LENGTH_STR = "250"; /** Default DTMF lenght */
 static const char * const ALSA_DFT_CARD    = "0";          /** Default sound card index */
 
@@ -533,4 +539,27 @@ void ShortcutPreferences::unserialize(const YAML::Node &in)
     parseValue(node, TOGGLE_PICKUP_HANGUP_SHORT_KEY, togglePickupHangup_);
 }
 
+VideoPreferences::VideoPreferences()
+    : decodingAccelerated_(false)
+{
+}
+
+void VideoPreferences::serialize(YAML::Emitter &out)
+{
+    out << YAML::Key << CONFIG_LABEL << YAML::Value << YAML::BeginMap;
+    out << YAML::Key << DECODING_ACCELERATED_KEY << YAML::Value << decodingAccelerated_;
+    getVideoDeviceMonitor().serialize(out);
+    out << YAML::EndMap;
+}
+
+void VideoPreferences::unserialize(const YAML::Node &in)
+{
+    const auto &node = in[CONFIG_LABEL];
+    // value may or may not be present
+    try {
+        parseValue(node, DECODING_ACCELERATED_KEY, decodingAccelerated_);
+    } catch (...) { decodingAccelerated_ = false; } // experimental, so disabled by default
+    getVideoDeviceMonitor().unserialize(node);
+}
+
 } // namespace ring
diff --git a/src/preferences.h b/src/preferences.h
index 1a8edbb86b..27716a4fd1 100644
--- a/src/preferences.h
+++ b/src/preferences.h
@@ -439,6 +439,26 @@ class ShortcutPreferences : public Serializable {
         constexpr static const char * const CONFIG_LABEL = "shortcuts";
 };
 
+class VideoPreferences : public Serializable {
+    public:
+        VideoPreferences();
+
+        void serialize(YAML::Emitter &out);
+        void unserialize(const YAML::Node &in);
+
+        bool getDecodingAccelerated() const {
+            return decodingAccelerated_;
+        }
+
+        void setDecodingAccelerated(bool decodingAccelerated) {
+            decodingAccelerated_ = decodingAccelerated;
+        }
+
+    private:
+        bool decodingAccelerated_;
+        constexpr static const char* const CONFIG_LABEL = "video";
+};
+
 } // namespace ring
 
 #endif
-- 
GitLab