Fix concurrency issues in video bitrate stats access (#1305)

lynckia · Sep 26, 2018 · 8b4692c · 8b4692c
1 parent 93ca6e6
commit 8b4692c
Show file tree

Hide file tree

Showing 6 changed files with 13 additions and 17 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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());