From ecf3e9012774f298ef1fe566f05cedb4b16927f0 Mon Sep 17 00:00:00 2001
From: philippegorley <philippe.gorley@savoirfairelinux.com>
Date: Wed, 29 Nov 2017 15:52:36 -0500
Subject: [PATCH] accel: refactor and speed up fallback

Falling back from hardware to software decoding requires only a
reinitialization of the AVCodecContext. Hardware decoding failure is no
longer bubbled up to the RTP session, which makes it much faster.

Change-Id: I79fdfcfa41f822b3299d74ac654146789fcfd97b
---
 src/call.h                               |  2 --
 src/media/audio/audio_rtp_session.cpp    | 11 ------
 src/media/audio/audio_rtp_session.h      |  1 -
 src/media/media_decoder.cpp              |  9 +++--
 src/media/media_decoder.h                |  2 ++
 src/media/rtp_session.h                  |  1 -
 src/media/video/video_receive_thread.cpp | 19 +++-------
 src/media/video/video_receive_thread.h   |  5 +--
 src/media/video/video_rtp_session.cpp    | 45 ++----------------------
 src/media/video/video_rtp_session.h      |  6 ----
 src/sip/sipcall.cpp                      | 10 ------
 src/sip/sipcall.h                        |  1 -
 12 files changed, 16 insertions(+), 96 deletions(-)

diff --git a/src/call.h b/src/call.h
index d4c9c4148a..dd8dfb43cd 100644
--- a/src/call.h
+++ b/src/call.h
@@ -304,8 +304,6 @@ class Call : public Recordable, public std::enable_shared_from_this<Call> {
 
         virtual void restartMediaSender() = 0;
 
-        virtual void restartMediaReceiver() = 0;
-
         /**
          * Update call details after creation.
          * @param details to update
diff --git a/src/media/audio/audio_rtp_session.cpp b/src/media/audio/audio_rtp_session.cpp
index c89b5044b6..e656493c0b 100644
--- a/src/media/audio/audio_rtp_session.cpp
+++ b/src/media/audio/audio_rtp_session.cpp
@@ -438,17 +438,6 @@ AudioRtpSession::startReceiver()
     receiveThread_->startLoop();
 }
 
-void
-AudioRtpSession::restartReceiver()
-{
-    std::lock_guard<std::recursive_mutex> lock(mutex_);
-    // ensure that start has been called before restart
-    if (not socketPair_)
-        return;
-
-    startReceiver();
-}
-
 void
 AudioRtpSession::start(std::unique_ptr<IceSocket> rtp_sock, std::unique_ptr<IceSocket> rtcp_sock)
 {
diff --git a/src/media/audio/audio_rtp_session.h b/src/media/audio/audio_rtp_session.h
index c00fb8d395..c9385e2beb 100644
--- a/src/media/audio/audio_rtp_session.h
+++ b/src/media/audio/audio_rtp_session.h
@@ -44,7 +44,6 @@ class AudioRtpSession : public RtpSession {
         void start(std::unique_ptr<IceSocket> rtp_sock,
                    std::unique_ptr<IceSocket> rtcp_sock) override;
         void restartSender() override;
-        void restartReceiver() override;
         void stop() override;
         void setMuted(bool isMuted);
 
diff --git a/src/media/media_decoder.cpp b/src/media/media_decoder.cpp
index 7813353f12..cc4b7ee066 100644
--- a/src/media/media_decoder.cpp
+++ b/src/media/media_decoder.cpp
@@ -244,9 +244,11 @@ int MediaDecoder::setupFromVideoData()
     static const unsigned MAX_ANALYZE_DURATION = 30; // time in seconds
 
     inputCtx_->max_analyze_duration = MAX_ANALYZE_DURATION * AV_TIME_BASE;
-
-    RING_DBG("Finding stream info");
-    ret = avformat_find_stream_info(inputCtx_, NULL);
+    // if fallback from accel, don't check for stream info, it's already done
+    if (!fallback_) {
+        RING_DBG("Finding stream info");
+        ret = avformat_find_stream_info(inputCtx_, NULL);
+    }
     if (ret < 0) {
         // workaround for this bug:
         // http://patches.libav.org/patch/22541/
@@ -378,6 +380,7 @@ MediaDecoder::decode(VideoFrame& result)
                 if (accelFailures_ >= MAX_ACCEL_FAILURES) {
                     RING_ERR("Hardware decoding failure");
                     accelFailures_ = 0; // reset error count for next time
+                    fallback_ = true;
                     return Status::RestartRequired;
                 }
             }
diff --git a/src/media/media_decoder.h b/src/media/media_decoder.h
index 7835d67ee6..4d2b466687 100644
--- a/src/media/media_decoder.h
+++ b/src/media/media_decoder.h
@@ -116,6 +116,8 @@ class MediaDecoder {
         void extract(const std::map<std::string, std::string>& map, const std::string& key);
         int correctPixFmt(int input_pix_fmt);
 
+        bool fallback_ = false;
+
 #ifdef RING_ACCEL
         bool enableAccel_ = true;
         video::HardwareAccel accel_;
diff --git a/src/media/rtp_session.h b/src/media/rtp_session.h
index 2a80154ccd..a46dcb2b8f 100644
--- a/src/media/rtp_session.h
+++ b/src/media/rtp_session.h
@@ -41,7 +41,6 @@ public:
     virtual void start(std::unique_ptr<IceSocket> rtp_sock,
                        std::unique_ptr<IceSocket> rtcp_sock) = 0;
     virtual void restartSender() = 0;
-    virtual void restartReceiver() = 0;
     virtual void stop() = 0;
 
     virtual void updateMedia(const MediaDescription& send,
diff --git a/src/media/video/video_receive_thread.cpp b/src/media/video/video_receive_thread.cpp
index 1042429771..4442dd34ce 100644
--- a/src/media/video/video_receive_thread.cpp
+++ b/src/media/video/video_receive_thread.cpp
@@ -38,7 +38,6 @@ using std::string;
 
 VideoReceiveThread::VideoReceiveThread(const std::string& id,
                                        const std::string &sdp,
-                                       const bool isReset,
                                        uint16_t mtu) :
     VideoGenerator::VideoGenerator()
     , args_()
@@ -48,8 +47,6 @@ VideoReceiveThread::VideoReceiveThread(const std::string& id,
     , stream_(sdp)
     , sdpContext_(stream_.str().size(), false, &readFunction, 0, 0, this)
     , sink_ {Manager::instance().createSinkClient(id)}
-    , restartDecoder_(false)
-    , isReset_(isReset)
     , mtu_(mtu)
     , requestKeyFrameCallback_(0)
     , loop_(std::bind(&VideoReceiveThread::setup, this),
@@ -74,11 +71,6 @@ bool VideoReceiveThread::setup()
 {
     videoDecoder_.reset(new MediaDecoder());
 
-#ifdef RING_ACCEL
-    // disable accel if there was a fallback to software decoding
-    videoDecoder_->enableAccel(!isReset_);
-#endif
-
     dstWidth_ = args_.width;
     dstHeight_ = args_.height;
 
@@ -188,9 +180,12 @@ bool VideoReceiveThread::decodeFrame()
             break;
 
         case MediaDecoder::Status::RestartRequired:
-            restartDecoder_ = true;
+            // disable accel, reset decoder's AVCodecContext
+#ifdef RING_ACCEL
+            videoDecoder_->enableAccel(false);
+#endif
+            videoDecoder_->setupFromVideoData();
             break;
-
         case MediaDecoder::Status::Success:
         case MediaDecoder::Status::EOFError:
             break;
@@ -231,10 +226,6 @@ int VideoReceiveThread::getHeight() const
 int VideoReceiveThread::getPixelFormat() const
 { return videoDecoder_->getPixelFormat(); }
 
-bool
-VideoReceiveThread::restartDecoder() const
-{ return restartDecoder_.load(); }
-
 void
 VideoReceiveThread::triggerKeyFrameRequest()
 {
diff --git a/src/media/video/video_receive_thread.h b/src/media/video/video_receive_thread.h
index 20ff0bb5fc..164ded49d8 100644
--- a/src/media/video/video_receive_thread.h
+++ b/src/media/video/video_receive_thread.h
@@ -34,7 +34,6 @@
 #include <climits>
 #include <sstream>
 #include <memory>
-#include <atomic>
 
 namespace ring {
 class SocketPair;
@@ -47,7 +46,7 @@ class SinkClient;
 
 class VideoReceiveThread : public VideoGenerator {
 public:
-    VideoReceiveThread(const std::string &id, const std::string &sdp, const bool isReset, uint16_t mtu);
+    VideoReceiveThread(const std::string &id, const std::string &sdp, uint16_t mtu);
     ~VideoReceiveThread();
     void startLoop();
 
@@ -60,7 +59,6 @@ public:
     int getWidth() const;
     int getHeight() const;
     int getPixelFormat() const;
-    bool restartDecoder() const;
     void triggerKeyFrameRequest();
 
 private:
@@ -79,7 +77,6 @@ private:
     MediaIOHandle sdpContext_;
     std::unique_ptr<MediaIOHandle> demuxContext_;
     std::shared_ptr<SinkClient> sink_;
-    std::atomic_bool restartDecoder_;
     bool isReset_;
     uint16_t mtu_;
 
diff --git a/src/media/video/video_rtp_session.cpp b/src/media/video/video_rtp_session.cpp
index 5a17db9fc5..f4ac96d4df 100644
--- a/src/media/video/video_rtp_session.cpp
+++ b/src/media/video/video_rtp_session.cpp
@@ -60,9 +60,6 @@ VideoRtpSession::VideoRtpSession(const string &callID,
     , rtcpCheckerThread_([] { return true; },
             [this]{ processRtcpChecker(); },
             []{})
-    , receiverRestartThread_([]{ return true; },
-            [this]{ processReceiverRestart(); },
-            []{})
     , packetLossThread_([] { return true; },
             [this]{ processPacketLoss(); },
             []{})
@@ -151,24 +148,19 @@ VideoRtpSession::restartSender()
 void VideoRtpSession::startReceiver()
 {
     if (receive_.enabled and not receive_.holding) {
-        bool isReset = false;
-        if (receiveThread_) {
+        if (receiveThread_)
             RING_WARN("Restarting video receiver");
-            isReset = true;
-        }
         receiveThread_.reset(
-                             new VideoReceiveThread(callID_, receive_.receiving_sdp, isReset, mtu_)
+            new VideoReceiveThread(callID_, receive_.receiving_sdp, mtu_)
         );
 
         // XXX keyframe requests can timeout if unanswered
         receiveThread_->setRequestKeyFrameCallback(&SIPVoIPLink::enqueueKeyframeRequest);
-        receiverRestartThread_.start();
         receiveThread_->addIOContext(*socketPair_);
         receiveThread_->startLoop();
         packetLossThread_.start();
     } else {
         RING_DBG("Video receiving disabled");
-        receiverRestartThread_.join();
         if (receiveThread_)
             receiveThread_->detach(videoMixer_.get());
         receiveThread_.reset();
@@ -176,18 +168,6 @@ void VideoRtpSession::startReceiver()
     }
 }
 
-void
-VideoRtpSession::restartReceiver()
-{
-    std::lock_guard<std::recursive_mutex> lock(mutex_);
-
-    // ensure that start has been called before restart
-    if (not socketPair_)
-        return;
-
-    startReceiver();
-}
-
 void VideoRtpSession::start(std::unique_ptr<IceSocket> rtp_sock,
                             std::unique_ptr<IceSocket> rtcp_sock)
 {
@@ -225,7 +205,6 @@ void VideoRtpSession::stop()
 {
     std::lock_guard<std::recursive_mutex> lock(mutex_);
     rtcpCheckerThread_.join();
-    receiverRestartThread_.join();
     packetLossThread_.join();
     if (videoLocal_)
         videoLocal_->detach(sender_.get());
@@ -565,19 +544,6 @@ VideoRtpSession::storeVideoBitrateInfo() {
     }
 }
 
-void
-VideoRtpSession::checkReceiver()
-{
-    if (receiveThread_ && receiveThread_->restartDecoder()) {
-        const auto& cid = callID_;
-        runOnMainThread([cid]{
-            if (auto call = Manager::instance().callFactory.getCall(cid)) {
-                call->restartMediaReceiver();
-            }
-        });
-    }
-}
-
 void
 VideoRtpSession::processRtcpChecker()
 {
@@ -585,13 +551,6 @@ VideoRtpSession::processRtcpChecker()
     rtcpCheckerThread_.wait_for(std::chrono::seconds(RTCP_CHECKING_INTERVAL));
 }
 
-void
-VideoRtpSession::processReceiverRestart()
-{
-    checkReceiver();
-    receiverRestartThread_.wait_for(std::chrono::seconds(RECEIVER_RESTART_INTERVAL));
-}
-
 void
 VideoRtpSession::processPacketLoss()
 {
diff --git a/src/media/video/video_rtp_session.h b/src/media/video/video_rtp_session.h
index 9f96ee86d9..e6ed4c0f29 100644
--- a/src/media/video/video_rtp_session.h
+++ b/src/media/video/video_rtp_session.h
@@ -66,7 +66,6 @@ public:
     void start(std::unique_ptr<IceSocket> rtp_sock,
                std::unique_ptr<IceSocket> rtcp_sock) override;
     void restartSender() override;
-    void restartReceiver() override;
     void stop() override;
 
     void forceKeyFrame();
@@ -121,8 +120,6 @@ private:
     // max size of quality and bitrate historic
     static constexpr unsigned MAX_SIZE_HISTO_QUALITY_ {30};
     static constexpr unsigned MAX_SIZE_HISTO_BITRATE_ {100};
-    // how long (in seconds) to wait before rechecking if the receiver needs to restart
-    const unsigned RECEIVER_RESTART_INTERVAL {4};
 
     // 5 tries in a row
     static constexpr unsigned  MAX_ADAPTATIVE_BITRATE_ITERATION {5};
@@ -133,9 +130,6 @@ private:
     InterruptedThreadLoop rtcpCheckerThread_;
     void processRtcpChecker();
 
-    InterruptedThreadLoop receiverRestartThread_;
-    void processReceiverRestart();
-
     InterruptedThreadLoop packetLossThread_;
     void processPacketLoss();
 };
diff --git a/src/sip/sipcall.cpp b/src/sip/sipcall.cpp
index d1a2e2eb17..e8b75b707a 100644
--- a/src/sip/sipcall.cpp
+++ b/src/sip/sipcall.cpp
@@ -946,16 +946,6 @@ SIPCall::restartMediaSender()
 #endif
 }
 
-void
-SIPCall::restartMediaReceiver()
-{
-    RING_DBG("[call:%s] restarting RX media streams", getCallId().c_str());
-    avformatrtp_->restartReceiver();
-#ifdef RING_VIDEO
-    videortp_->restartReceiver();
-#endif
-}
-
 void
 SIPCall::stopAllMedia()
 {
diff --git a/src/sip/sipcall.h b/src/sip/sipcall.h
index 2f57e01d72..56677be9b1 100644
--- a/src/sip/sipcall.h
+++ b/src/sip/sipcall.h
@@ -102,7 +102,6 @@ public: // overridden
     void removeCall() override;
     void muteMedia(const std::string& mediaType, bool isMuted) override;
     void restartMediaSender() override;
-    void restartMediaReceiver() override;
     bool useVideoCodec(const AccountVideoCodecInfo* codec) const override;
     std::map<std::string, std::string> getDetails() const override;
 
-- 
GitLab