Estimate bandwidth in connection and don't limit by REMBs

erizo/src/erizo/MediaStream.cpp

@@ -21,7 +21,6 @@
 #include "rtp/RtcpForwarder.h"
 #include "rtp/RtpSlideShowHandler.h"
 #include "rtp/RtpTrackMuteHandler.h"
-#include "rtp/BandwidthEstimationHandler.h"
 #include "rtp/FecReceiverHandler.h"
 #include "rtp/RtcpProcessorHandler.h"
 #include "rtp/RtpRetransmissionHandler.h"
@@ -470,7 +469,6 @@ void MediaStream::initializePipeline() {
   addHandlerInPosition(MIDDLE, handler_pointer_dic, handler_order);
   pipeline_->addFront(std::make_shared<PliPacerHandler>());
   pipeline_->addFront(std::make_shared<RtpPaddingRemovalHandler>());
-  pipeline_->addFront(std::make_shared<BandwidthEstimationHandler>());
   pipeline_->addFront(std::make_shared<RtcpFeedbackGenerationHandler>());
   pipeline_->addFront(std::make_shared<RtpRetransmissionHandler>());
   pipeline_->addFront(std::make_shared<SRPacketHandler>());

erizo/src/erizo/WebRtcConnection.cpp

@@ -19,6 +19,7 @@
 #include "bandwidth/TargetVideoBWDistributor.h"
 #include "rtp/RtpHeaders.h"
 #include "rtp/SenderBandwidthEstimationHandler.h"
+#include "rtp/BandwidthEstimationHandler.h"
 #include "rtp/RtpPaddingManagerHandler.h"
 #include "rtp/RtpUtils.h"
 
@@ -113,6 +114,8 @@ void WebRtcConnection::initializePipeline() {
 
   pipeline_->addFront(std::make_shared<ConnectionPacketReader>(this));
 
+  pipeline_->addFront(std::make_shared<BandwidthEstimationHandler>());
+
   pipeline_->addFront(std::make_shared<SenderBandwidthEstimationHandler>());
   pipeline_->addFront(std::make_shared<RtpPaddingManagerHandler>());
 
@@ -769,6 +772,7 @@ boost::future<void> WebRtcConnection::processRemoteSdp() {
 
   local_sdp_->setOfferSdp(remote_sdp_);
   extension_processor_.setSdpInfo(local_sdp_);
+  notifyUpdateToHandlers();
   local_sdp_->updateSupportedExtensionMap(extension_processor_.getSupportedExtensionMap());
 
   if (first_remote_sdp_processed_) {

erizo/src/erizo/rtp/BandwidthEstimationHandler.cpp

@@ -2,6 +2,7 @@
 
 #include <vector>
 
+#include "./WebRtcConnection.h"
 #include "./MediaStream.h"
 #include "lib/Clock.h"
 #include "lib/ClockUtils.h"
@@ -39,12 +40,12 @@ std::unique_ptr<RemoteBitrateEstimator> RemoteBitrateEstimatorPicker::pickEstima
 }
 
 BandwidthEstimationHandler::BandwidthEstimationHandler(std::shared_ptr<RemoteBitrateEstimatorPicker> picker) :
-  stream_{nullptr}, clock_{webrtc::Clock::GetRealTimeClock()},
+  connection_{nullptr}, clock_{webrtc::Clock::GetRealTimeClock()},
   picker_{picker},
   using_absolute_send_time_{false}, packets_since_absolute_send_time_{0},
   min_bitrate_bps_{kMinBitRateAllowed},
-  bitrate_{0}, last_send_bitrate_{0}, max_video_bw_{kDefaultMaxVideoBWInKbps}, last_remb_time_{0},
-  running_{false}, active_{true}, initialized_{false} {
+  bitrate_{0}, last_send_bitrate_{0}, last_remb_time_{0},
+  sink_ssrc_{0}, running_{false}, active_{true}, initialized_{false} {
     rtc::LogMessage::SetLogToStderr(false);
 }
 
@@ -57,32 +58,25 @@ void BandwidthEstimationHandler::disable() {
 }
 
 void BandwidthEstimationHandler::notifyUpdate() {
+  ELOG_DEBUG("NotifyUPDATE");
   auto pipeline = getContext()->getPipelineShared();
-
-  if (pipeline) {
-    auto rtcp_processor = pipeline->getService<RtcpProcessor>();
-    if (rtcp_processor) {
-      max_video_bw_ = rtcp_processor->getMaxVideoBW();
-    }
+  if (pipeline && !connection_) {
+    connection_ = pipeline->getService<WebRtcConnection>().get();
   }
 
-  if (initialized_) {
+  if (!connection_) {
+    ELOG_ERROR("Returning because there is no connection");
     return;
   }
 
-  if (pipeline && !stream_) {
-    stream_ = pipeline->getService<MediaStream>().get();
-  }
-  if (!stream_) {
+  RtpExtensionProcessor& ext_processor = connection_->getRtpExtensionProcessor();
+  updateExtensionMaps(ext_processor.getVideoExtensionMap(), ext_processor.getAudioExtensionMap());
+
+  if (initialized_) {
     return;
   }
-  worker_ = stream_->getWorker();
+  worker_ = connection_->getWorker();
   stats_ = pipeline->getService<Stats>();
-  RtpExtensionProcessor& ext_processor = stream_->getRtpExtensionProcessor();
-  if (ext_processor.getVideoExtensionMap().size() == 0) {
-    return;
-  }
-  updateExtensionMaps(ext_processor.getVideoExtensionMap(), ext_processor.getAudioExtensionMap());
 
   pickEstimator();
   initialized_ = true;
@@ -229,19 +223,39 @@ void BandwidthEstimationHandler::pickEstimator() {
 }
 
 void BandwidthEstimationHandler::sendREMBPacket() {
+  sink_ssrc_ = 0;
+  source_ssrcs_.clear();
+  ELOG_DEBUG("Update MediaStream SSRCs");
+  connection_->forEachMediaStream([this] (const std::shared_ptr<MediaStream> &media_stream) {
+    ELOG_DEBUG("MediaStream %s, publisher %u, sink %u, source %u", media_stream->getId().c_str(),
+    media_stream->isPublisher(), media_stream->getVideoSinkSSRC(), media_stream->getVideoSourceSSRC());
+    if (media_stream->isReady() && media_stream->isPublisher()) {
+      sink_ssrc_ = media_stream->getVideoSinkSSRC();
+    }
+    source_ssrcs_.push_back(media_stream->getVideoSourceSSRC());
+  });
+
+  if (sink_ssrc_ == 0) {
+    ELOG_WARN("No SSRC available to send REMB");
+    return;
+  }
   remb_packet_.setPacketType(RTCP_PS_Feedback_PT);
   remb_packet_.setBlockCount(RTCP_AFB);
   memcpy(&remb_packet_.report.rembPacket.uniqueid, "REMB", 4);
 
-  remb_packet_.setSSRC(stream_->getVideoSinkSSRC());
-  //  todo(pedro) figure out which sourceSSRC to use here
-  remb_packet_.setSourceSSRC(stream_->getVideoSourceSSRC());
-  remb_packet_.setLength(5);
-  uint32_t capped_bitrate = max_video_bw_ > 0 ? std::min(max_video_bw_, bitrate_) : bitrate_;
-  ELOG_DEBUG("Bitrates min(%u,%u) = %u", bitrate_, max_video_bw_, capped_bitrate);
+  remb_packet_.setSSRC(sink_ssrc_);
+  remb_packet_.setSourceSSRC(0);
+  remb_packet_.setLength(4 + source_ssrcs_.size());
+  uint32_t capped_bitrate = bitrate_;
+  ELOG_DEBUG("Bitrates min(%u) = %u", bitrate_, capped_bitrate);
   remb_packet_.setREMBBitRate(capped_bitrate);
-  remb_packet_.setREMBNumSSRC(1);
-  remb_packet_.setREMBFeedSSRC(0, stream_->getVideoSourceSSRC());
+  remb_packet_.setREMBNumSSRC(source_ssrcs_.size());
+
+  for (std::size_t i = 0; i < source_ssrcs_.size(); i++) {
+    ELOG_DEBUG("Setting REMBFeedSSRC %u to ssrc %u, size %u", i, source_ssrcs_[i], source_ssrcs_.size());
+    remb_packet_.setREMBFeedSSRC(i, source_ssrcs_[i]);
+  }
+
   int remb_length = (remb_packet_.getLength() + 1) * 4;
   if (active_) {
     ELOG_DEBUG("BWE Estimation is %d", last_send_bitrate_);
@@ -252,6 +266,7 @@ void BandwidthEstimationHandler::sendREMBPacket() {
 
 void BandwidthEstimationHandler::OnReceiveBitrateChanged(const std::vector<uint32_t>& ssrcs,
                                      uint32_t bitrate) {
+  ELOG_WARN("Onreceive bitrate %lu", bitrate);
   if (last_send_bitrate_ > 0) {
     unsigned int new_remb_bitrate = last_send_bitrate_ - bitrate_ + bitrate;
     if (new_remb_bitrate < kSendThresholdPercent * last_send_bitrate_ / 100) {
@@ -274,8 +289,7 @@ void BandwidthEstimationHandler::OnReceiveBitrateChanged(const std::vector<uint3
   }
   last_remb_time_ = now;
   last_send_bitrate_ = bitrate_;
-  stats_->getNode()
-  [stream_->getVideoSourceSSRC()].insertStat("erizoBandwidth", CumulativeStat{last_send_bitrate_});
+  stats_->getNode().insertStat("erizoBandwidth", CumulativeStat{last_send_bitrate_});
   sendREMBPacket();
 }
 

erizo/src/erizo/rtp/BandwidthEstimationHandler.h

@@ -20,7 +20,7 @@
 
 namespace erizo {
 
-class MediaStream;
+class WebRtcConnection;
 using webrtc::RemoteBitrateEstimator;
 using webrtc::RemoteBitrateObserver;
 using webrtc::RtpHeaderExtensionMap;
@@ -68,7 +68,7 @@ class BandwidthEstimationHandler: public Handler, public RemoteBitrateObserver,
 
   void updateExtensionMap(bool video, std::array<RTPExtensions, 15> map);
 
-  MediaStream *stream_;
+  WebRtcConnection *connection_;
   std::shared_ptr<Worker> worker_;
   std::shared_ptr<Stats> stats_;
   webrtc::Clock* const clock_;
@@ -82,8 +82,9 @@ class BandwidthEstimationHandler: public Handler, public RemoteBitrateObserver,
   RtpHeaderExtensionMap ext_map_audio_, ext_map_video_;
   uint32_t bitrate_;
   uint32_t last_send_bitrate_;
-  uint32_t max_video_bw_;
   uint64_t last_remb_time_;
+  uint32_t sink_ssrc_;
+  std::vector<uint32_t> source_ssrcs_;
   bool running_;
   bool active_;
   bool initialized_;

erizo/src/test/rtp/BandwidthEstimationHandlerTest.cpp

@@ -47,8 +47,11 @@ class BandwidthEstimationHandlerTest : public erizo::HandlerTest {
     EXPECT_CALL(*picker.get(), pickEstimatorProxy(_, _, _))
       .WillRepeatedly(Return(new erizo::RemoteBitrateEstimatorProxy(&estimator)));
 
+    connection->addMediaStream(media_stream);
+
     bwe_handler = std::make_shared<BandwidthEstimationHandler>(picker);
     pipeline->addBack(bwe_handler);
+    pipeline->notifyUpdate();
   }
 
   std::shared_ptr<BandwidthEstimationHandler> bwe_handler;
@@ -92,22 +95,3 @@ TEST_F(BandwidthEstimationHandlerTest, shouldSendRembPacketWithEstimatedBitrate)
   EXPECT_CALL(*writer.get(), write(_, _)).With(Args<1>(erizo::RembHasBitrateValue(kArbitraryBitrate))).Times(1);
   picker->observer_->OnReceiveBitrateChanged(std::vector<uint32_t>(), kArbitraryBitrate);
 }
-
-TEST_F(BandwidthEstimationHandlerTest, shouldSendRembPacketWithCappedBitrate) {
-  uint32_t kArbitraryBitrate = 100000;
-  uint32_t kArbitraryCappedBitrate = kArbitraryBitrate - 100;
-  auto packet = erizo::PacketTools::createDataPacket(erizo::kArbitrarySeqNumber, VIDEO_PACKET);
-
-  EXPECT_CALL(estimator, Process());
-  EXPECT_CALL(estimator, TimeUntilNextProcess()).WillRepeatedly(Return(1000));
-  EXPECT_CALL(estimator, IncomingPacket(_, _, _));
-  EXPECT_CALL(*reader.get(), read(_, _)).
-    With(Args<1>(erizo::RtpHasSequenceNumber(erizo::kArbitrarySeqNumber))).Times(1);
-  EXPECT_CALL(*processor.get(), getMaxVideoBW()).WillRepeatedly(Return(kArbitraryCappedBitrate));
-  pipeline->notifyUpdate();
-  pipeline->read(packet);
-
-  EXPECT_CALL(*writer.get(), write(_, _)).With(Args<1>(erizo::RembHasBitrateValue(kArbitraryCappedBitrate))).Times(1);
-
-  picker->observer_->OnReceiveBitrateChanged(std::vector<uint32_t>(), kArbitraryBitrate);
-}

erizo_controller/erizoClient/src/ErizoConnectionManager.js

@@ -122,7 +122,7 @@ class ErizoConnection extends EventEmitterConst {
     log.debug(`message: Adding stream to Connection, ${this.toLog()}, ${stream.toLog()}`);
     this.streamsMap.add(stream.getID(), stream);
     if (stream.local) {
-      this.stack.addStream(stream.stream, stream.hasScreen());
+      this.stack.addStream(stream);
     }
   }
 
@@ -148,30 +148,10 @@ class ErizoConnection extends EventEmitterConst {
     this.stack.sendSignalingMessage(msg);
   }
 
-  setSimulcast(enable) {
-    this.stack.setSimulcast(enable);
-  }
-
-  setVideo(video) {
-    this.stack.setVideo(video);
-  }
-
-  setAudio(audio) {
-    this.stack.setAudio(audio);
-  }
-
   updateSpec(configInput, streamId, callback) {
     this.stack.updateSpec(configInput, streamId, callback);
   }
 
-  updateSimulcastLayersBitrate(bitrates) {
-    this.stack.updateSimulcastLayersBitrate(bitrates);
-  }
-
-  updateSimulcastActiveLayers(layersInfo) {
-    this.stack.updateSimulcastActiveLayers(layersInfo);
-  }
-
   setQualityLevel(level) {
     this.qualityLevel = QUALITY_LEVELS[level];
   }
@@ -229,16 +209,6 @@ class ErizoConnectionManager {
         this.ErizoConnectionsMap.set(erizoId, connectionEntry);
       }
     }
-    if (specInput.simulcast) {
-      connection.setSimulcast(specInput.simulcast);
-    }
-    if (specInput.video) {
-      connection.setVideo(specInput.video);
-    }
-    if (specInput.audio) {
-      connection.setVideo(specInput.audio);
-    }
-
     return connection;
   }
 

erizo_controller/erizoClient/src/Room.js

@@ -165,9 +165,9 @@ const Room = (altIo, altConnectionHelpers, altConnectionManager, specInput) => {
 
   const createRemoteStreamP2PConnection = (streamInput, peerSocket) => {
     const stream = streamInput;
-    const connection = that.erizoConnectionManager.getOrBuildErizoConnection(
-      getP2PConnectionOptions(stream, peerSocket));
-    stream.addPC(connection);
+    const connectionOptions = getP2PConnectionOptions(stream, peerSocket);
+    const connection = that.erizoConnectionManager.getOrBuildErizoConnection(connectionOptions);
+    stream.addPC(connection, false, connectionOptions);
     connection.on('connection-failed', that.dispatchEvent.bind(this));
     stream.on('added', dispatchStreamSubscribed.bind(null, stream));
     stream.on('icestatechanged', (evt) => {
@@ -220,7 +220,6 @@ const Room = (altIo, altConnectionHelpers, altConnectionManager, specInput) => {
   const getErizoConnectionOptions = (stream, connectionId, erizoId, options, isRemote) => {
     const connectionOpts = {
       callback(message, streamId = stream.getID()) {
-        log.debug(`message: Sending message, data: ${JSON.stringify(message)}, ${stream.toLog()}, ${toLog()}`);
         if (message && message.type && message.type === 'updatestream') {
           socket.sendSDP('streamMessage', {
             streamId,
@@ -241,6 +240,7 @@ const Room = (altIo, altConnectionHelpers, altConnectionManager, specInput) => {
       video: options.video && stream.hasVideo(),
       maxAudioBW: options.maxAudioBW,
       maxVideoBW: options.maxVideoBW,
+      simulcast: options.simulcast,
       limitMaxAudioBW: spec.maxAudioBW,
       limitMaxVideoBW: spec.maxVideoBW,
       label: stream.getLabel(),
@@ -252,7 +252,6 @@ const Room = (altIo, altConnectionHelpers, altConnectionManager, specInput) => {
       isRemote,
     };
     if (!isRemote) {
-      connectionOpts.simulcast = options.simulcast;
       connectionOpts.startVideoBW = options.startVideoBW;
       connectionOpts.hardMinVideoBW = options.hardMinVideoBW;
     }
@@ -264,7 +263,7 @@ const Room = (altIo, altConnectionHelpers, altConnectionManager, specInput) => {
     const connectionOpts = getErizoConnectionOptions(stream, connectionId, erizoId, options, true);
     const connection = that.erizoConnectionManager
       .getOrBuildErizoConnection(connectionOpts, erizoId, spec.singlePC);
-    stream.addPC(connection);
+    stream.addPC(connection, false, connectionOpts);
     connection.on('connection-failed', that.dispatchEvent.bind(this));
 
     stream.on('added', dispatchStreamSubscribed.bind(null, stream));
@@ -285,7 +284,7 @@ const Room = (altIo, altConnectionHelpers, altConnectionManager, specInput) => {
     const connectionOpts = getErizoConnectionOptions(stream, connectionId, erizoId, options);
     const connection = that.erizoConnectionManager
       .getOrBuildErizoConnection(connectionOpts, erizoId, spec.singlePC);
-    stream.addPC(connection);
+    stream.addPC(connection, false, options);
     connection.on('connection-failed', that.dispatchEvent.bind(this));
     stream.on('icestatechanged', (evt) => {
       log.debug(`message: icestatechanged, ${stream.toLog()}, iceConnectionState: ${evt.msg.state}, ${toLog()}`);
@@ -805,6 +804,8 @@ const Room = (altIo, altConnectionHelpers, altConnectionManager, specInput) => {
     log.info(`message: Publishing stream, ${stream.toLog()}, ${toLog()}`);
 
     options.maxVideoBW = options.maxVideoBW || spec.defaultVideoBW;
+    options.limitMaxVideoBW = spec.maxVideoBW;
+    options.limitMaxAudioBW = spec.maxAudioBW;
     if (options.maxVideoBW > spec.maxVideoBW) {
       options.maxVideoBW = spec.maxVideoBW;
     }