From 8b4692c88f1fc24dedad66b4f40b1f3d804b50ca Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javier=20Cervi=C3=B1o?= <jcague@gmail.com>
Date: Wed, 26 Sep 2018 10:51:24 +0200
Subject: [PATCH] Fix concurrency issues in video bitrate stats access (#1305)

---
 erizo/src/erizo/MediaStream.cpp                     | 13 ++-----------
 erizo/src/erizo/MediaStream.h                       |  4 +++-
 .../erizo/bandwidth/TargetVideoBWDistributor.cpp    |  2 +-
 erizo/src/erizo/rtp/StatsHandler.cpp                |  7 +++++--
 .../src/test/bandwidth/TargetVideoBWDistributor.cpp |  2 +-
 erizo/src/test/utils/Mocks.h                        |  2 +-
 6 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/erizo/src/erizo/MediaStream.cpp b/erizo/src/erizo/MediaStream.cpp
index 50932f08f4..6023e478b6 100644
--- a/erizo/src/erizo/MediaStream.cpp
+++ b/erizo/src/erizo/MediaStream.cpp
@@ -62,7 +62,8 @@ MediaStream::MediaStream(std::shared_ptr<Worker> worker,
     pipeline_initialized_{false},
     is_publisher_{is_publisher},
     simulcast_{false},
-    bitrate_from_max_quality_layer_{0} {
+    bitrate_from_max_quality_layer_{0},
+    video_bitrate_{0} {
   setVideoSinkSSRC(kDefaultVideoSinkSSRC);
   setAudioSinkSSRC(kDefaultAudioSinkSSRC);
   ELOG_INFO("%s message: constructor, id: %s",
@@ -98,16 +99,6 @@ uint32_t MediaStream::getMaxVideoBW() {
   return bitrate;
 }
 
-uint32_t MediaStream::getBitrateSent() {
-  uint32_t bitrate = 0;
-  std::string video_ssrc = std::to_string(is_publisher_ ? getVideoSourceSSRC() : getVideoSinkSSRC());
-  if (stats_->getNode().hasChild(video_ssrc) &&
-      stats_->getNode()[video_ssrc].hasChild("bitrateCalculated")) {
-    bitrate = stats_->getNode()[video_ssrc]["bitrateCalculated"].value();
-  }
-  return bitrate;
-}
-
 void MediaStream::setMaxVideoBW(uint32_t max_video_bw) {
   asyncTask([max_video_bw] (std::shared_ptr<MediaStream> stream) {
     if (stream->rtcp_processor_) {
diff --git a/erizo/src/erizo/MediaStream.h b/erizo/src/erizo/MediaStream.h
index 692d13006e..f67fc85c0b 100644
--- a/erizo/src/erizo/MediaStream.h
+++ b/erizo/src/erizo/MediaStream.h
@@ -71,7 +71,8 @@ class MediaStream: public MediaSink, public MediaSource, public FeedbackSink,
   void close() override;
   virtual uint32_t getMaxVideoBW();
   virtual uint32_t getBitrateFromMaxQualityLayer() { return bitrate_from_max_quality_layer_; }
-  virtual uint32_t getBitrateSent();
+  virtual uint32_t getVideoBitrate() { return video_bitrate_; }
+  void setVideoBitrate(uint32_t bitrate) { video_bitrate_ = bitrate; }
   void setMaxVideoBW(uint32_t max_video_bw);
   void syncClose();
   bool setRemoteSdp(std::shared_ptr<SdpInfo> sdp);
@@ -211,6 +212,7 @@ class MediaStream: public MediaSink, public MediaSource, public FeedbackSink,
 
   std::atomic_bool simulcast_;
   std::atomic<uint64_t> bitrate_from_max_quality_layer_;
+  std::atomic<uint32_t> video_bitrate_;
  protected:
   std::shared_ptr<SdpInfo> remote_sdp_;
 };
diff --git a/erizo/src/erizo/bandwidth/TargetVideoBWDistributor.cpp b/erizo/src/erizo/bandwidth/TargetVideoBWDistributor.cpp
index 5b8d0e1fdc..c2d61dd4a2 100644
--- a/erizo/src/erizo/bandwidth/TargetVideoBWDistributor.cpp
+++ b/erizo/src/erizo/bandwidth/TargetVideoBWDistributor.cpp
@@ -20,7 +20,7 @@ void TargetVideoBWDistributor::distribute(uint32_t remb, uint32_t ssrc,
       stream_infos.push_back({stream,
                               stream->isSimulcast(),
                               stream->isSlideShowModeEnabled(),
-                              stream->getBitrateSent(),
+                              stream->getVideoBitrate(),
                               stream->getMaxVideoBW(),
                               stream->getBitrateFromMaxQualityLayer()});
     });
diff --git a/erizo/src/erizo/rtp/StatsHandler.cpp b/erizo/src/erizo/rtp/StatsHandler.cpp
index b1694dcc4c..6660ab2959 100644
--- a/erizo/src/erizo/rtp/StatsHandler.cpp
+++ b/erizo/src/erizo/rtp/StatsHandler.cpp
@@ -53,8 +53,11 @@ void StatsCalculator::processRtpPacket(std::shared_ptr<DataPacket> packet) {
   }
   getStatsInfo()[ssrc]["bitrateCalculated"] += len;
   getStatsInfo()["total"]["bitrateCalculated"] += len;
-  if (packet->type == VIDEO_PACKET && packet->is_keyframe) {
-    incrStat(ssrc, "keyFrames");
+  if (packet->type == VIDEO_PACKET) {
+    stream_->setVideoBitrate(getStatsInfo()[ssrc]["bitrateCalculated"].value());
+    if (packet->is_keyframe) {
+      incrStat(ssrc, "keyFrames");
+    }
   }
 }
 
diff --git a/erizo/src/test/bandwidth/TargetVideoBWDistributor.cpp b/erizo/src/test/bandwidth/TargetVideoBWDistributor.cpp
index 5fa7ad3c57..07b45f96bf 100644
--- a/erizo/src/test/bandwidth/TargetVideoBWDistributor.cpp
+++ b/erizo/src/test/bandwidth/TargetVideoBWDistributor.cpp
@@ -64,7 +64,7 @@ class BasicTargetVideoBWDistributor {
     media_stream->setAudioSourceSSRC(audio_source_ssrc);
 
     EXPECT_CALL(*media_stream, getMaxVideoBW()).Times(AtLeast(0)).WillRepeatedly(Return(config.max_video_bw));
-    EXPECT_CALL(*media_stream, getBitrateSent()).Times(AtLeast(0)).WillRepeatedly(Return(config.bitrate_sent));
+    EXPECT_CALL(*media_stream, getVideoBitrate()).Times(AtLeast(0)).WillRepeatedly(Return(config.bitrate_sent));
     EXPECT_CALL(*media_stream, getBitrateFromMaxQualityLayer()).Times(AtLeast(0))
      .WillRepeatedly(Return(config.max_quality_bitrate));
     EXPECT_CALL(*media_stream, isSlideShowModeEnabled()).Times(AtLeast(0)).WillRepeatedly(Return(config.slideshow));
diff --git a/erizo/src/test/utils/Mocks.h b/erizo/src/test/utils/Mocks.h
index 0a313e1e6e..8210e9943e 100644
--- a/erizo/src/test/utils/Mocks.h
+++ b/erizo/src/test/utils/Mocks.h
@@ -107,7 +107,7 @@ class MockMediaStream: public MediaStream {
   }
 
   MOCK_METHOD0(getMaxVideoBW, uint32_t());
-  MOCK_METHOD0(getBitrateSent, uint32_t());
+  MOCK_METHOD0(getVideoBitrate, uint32_t());
   MOCK_METHOD0(getBitrateFromMaxQualityLayer, uint32_t());
   MOCK_METHOD0(isSlideShowModeEnabled, bool());
   MOCK_METHOD0(isSimulcast, bool());