From 7dd7945186c209c83ca07291f55b8bfd8d3816c7 Mon Sep 17 00:00:00 2001 From: Pedro Rodriguez Date: Tue, 1 Jun 2021 14:56:09 +0200 Subject: [PATCH 01/12] WIP Move BandwidthEstimationHandler to WebrtcConnection --- erizo/src/erizo/MediaStream.cpp | 2 - erizo/src/erizo/WebRtcConnection.cpp | 13 ++++ .../erizo/rtp/BandwidthEstimationHandler.cpp | 67 +++++++++++-------- .../erizo/rtp/BandwidthEstimationHandler.h | 6 +- 4 files changed, 57 insertions(+), 31 deletions(-) diff --git a/erizo/src/erizo/MediaStream.cpp b/erizo/src/erizo/MediaStream.cpp index 4b83422128..ff4fc08e5d 100644 --- a/erizo/src/erizo/MediaStream.cpp +++ b/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()); pipeline_->addFront(std::make_shared()); - pipeline_->addFront(std::make_shared()); pipeline_->addFront(std::make_shared()); pipeline_->addFront(std::make_shared()); pipeline_->addFront(std::make_shared()); diff --git a/erizo/src/erizo/WebRtcConnection.cpp b/erizo/src/erizo/WebRtcConnection.cpp index 1ad5cc3656..81fcce4a29 100644 --- a/erizo/src/erizo/WebRtcConnection.cpp +++ b/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(this)); + pipeline_->addFront(std::make_shared()); + pipeline_->addFront(std::make_shared()); pipeline_->addFront(std::make_shared()); @@ -401,6 +404,7 @@ boost::future WebRtcConnection::addMediaStream(std::shared_ptrtask([weak_this, task_promise, media_stream] { if (auto this_ptr = weak_this.lock()) { this_ptr->addMediaStreamSync(media_stream); + this_ptr->notifyUpdateToHandlers(); } task_promise->set_value(); }); @@ -445,6 +449,7 @@ boost::future WebRtcConnection::removeMediaStream(const std::string& strea } }); } + connection->notifyUpdateToHandlers(); }); } @@ -658,6 +663,7 @@ std::shared_ptr WebRtcConnection::getLocalSdpInfoSync() { local_sdp_->profile = remote_sdp_->profile; auto local_sdp_copy = std::make_shared(*local_sdp_.get()); + notifyUpdateToHandlers(); return local_sdp_copy; } @@ -1266,6 +1272,13 @@ void WebRtcConnection::write(std::shared_ptr packet) { return; } extension_processor_.processRtpExtensions(packet); + RtcpHeader *chead = reinterpret_cast(packet->data); + uint32_t recv_ssrc = chead->getSourceSSRC(); + if (chead->isREMB()) { + uint64_t bitrate = chead->getBrMantis() << chead->getBrExp(); + ELOG_WARN("%s sending REMB %lu, numSSRC %u, SSRCFeed %u, SSRC %u, SourceSSRC %u", toLog(), bitrate, + chead->getREMBNumSSRC(), chead->getREMBFeedSSRC(0), chead->getSSRC(), chead->getSourceSSRC()); + } transport->write(packet->data, packet->length); } diff --git a/erizo/src/erizo/rtp/BandwidthEstimationHandler.cpp b/erizo/src/erizo/rtp/BandwidthEstimationHandler.cpp index 43f826c023..cbbfc26ace 100644 --- a/erizo/src/erizo/rtp/BandwidthEstimationHandler.cpp +++ b/erizo/src/erizo/rtp/BandwidthEstimationHandler.cpp @@ -2,6 +2,7 @@ #include +#include "./WebRtcConnection.h" #include "./MediaStream.h" #include "lib/Clock.h" #include "lib/ClockUtils.h" @@ -39,12 +40,12 @@ std::unique_ptr RemoteBitrateEstimatorPicker::pickEstima } BandwidthEstimationHandler::BandwidthEstimationHandler(std::shared_ptr 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}, max_video_bw_{3000000000}, last_remb_time_{0}, + sink_ssrc_{0}, running_{false}, active_{true}, initialized_{false} { rtc::LogMessage::SetLogToStderr(false); } @@ -57,28 +58,33 @@ void BandwidthEstimationHandler::disable() { } void BandwidthEstimationHandler::notifyUpdate() { + ELOG_ERROR("NOTIFY UPDATE"); auto pipeline = getContext()->getPipelineShared(); - - if (pipeline) { - auto rtcp_processor = pipeline->getService(); - if (rtcp_processor) { - max_video_bw_ = rtcp_processor->getMaxVideoBW(); - } + if (pipeline && !connection_) { + connection_ = pipeline->getService().get(); } - if (initialized_) { + if (!connection_) { + ELOG_ERROR("Returning because there is no connection"); return; } - if (pipeline && !stream_) { - stream_ = pipeline->getService().get(); - } - if (!stream_) { - return; - } - worker_ = stream_->getWorker(); + sink_ssrc_ = 0; + source_ssrcs_.clear(); + ELOG_DEBUG("Update MediaStream SSRCs"); + connection_->forEachMediaStream([this] (const std::shared_ptr &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()); + }); + + + worker_ = connection_->getWorker(); stats_ = pipeline->getService(); - RtpExtensionProcessor& ext_processor = stream_->getRtpExtensionProcessor(); + RtpExtensionProcessor& ext_processor = connection_->getRtpExtensionProcessor(); if (ext_processor.getVideoExtensionMap().size() == 0) { return; } @@ -216,24 +222,32 @@ void BandwidthEstimationHandler::pickEstimatorFromHeader() { } void BandwidthEstimationHandler::pickEstimator() { - rbe_ = picker_->pickEstimator(using_absolute_send_time_, clock_, this); + rbe_ = picker_->pickEstimator(true, clock_, this); rbe_->SetMinBitrate(min_bitrate_bps_); } void BandwidthEstimationHandler::sendREMBPacket() { + 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_.setSSRC(sink_ssrc_); + remb_packet_.setSourceSSRC(0); remb_packet_.setLength(5); - uint32_t capped_bitrate = max_video_bw_ > 0 ? std::min(max_video_bw_, bitrate_) : bitrate_; + uint32_t capped_bitrate = bitrate_; ELOG_DEBUG("Bitrates min(%u,%u) = %u", bitrate_, max_video_bw_, 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_); @@ -266,8 +280,7 @@ void BandwidthEstimationHandler::OnReceiveBitrateChanged(const std::vectorgetNode() - [stream_->getVideoSourceSSRC()].insertStat("erizoBandwidth", CumulativeStat{last_send_bitrate_}); + stats_->getNode().insertStat("erizoBandwidth", CumulativeStat{last_send_bitrate_}); sendREMBPacket(); } diff --git a/erizo/src/erizo/rtp/BandwidthEstimationHandler.h b/erizo/src/erizo/rtp/BandwidthEstimationHandler.h index da3aae4268..65f1c4a074 100644 --- a/erizo/src/erizo/rtp/BandwidthEstimationHandler.h +++ b/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 map); - MediaStream *stream_; + WebRtcConnection *connection_; std::shared_ptr worker_; std::shared_ptr stats_; webrtc::Clock* const clock_; @@ -84,6 +84,8 @@ class BandwidthEstimationHandler: public Handler, public RemoteBitrateObserver, uint32_t last_send_bitrate_; uint32_t max_video_bw_; uint64_t last_remb_time_; + uint32_t sink_ssrc_; + std::vector source_ssrcs_; bool running_; bool active_; bool initialized_; From ba1dc0b5608b0b22a5b2302de25a2f17b0f8c885 Mon Sep 17 00:00:00 2001 From: Pedro Rodriguez Date: Thu, 3 Jun 2021 11:52:16 +0200 Subject: [PATCH 02/12] [WIP] Fix estimator and REMB packets --- .../erizo/rtp/BandwidthEstimationHandler.cpp | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/erizo/src/erizo/rtp/BandwidthEstimationHandler.cpp b/erizo/src/erizo/rtp/BandwidthEstimationHandler.cpp index cbbfc26ace..0cd878e6ba 100644 --- a/erizo/src/erizo/rtp/BandwidthEstimationHandler.cpp +++ b/erizo/src/erizo/rtp/BandwidthEstimationHandler.cpp @@ -69,17 +69,6 @@ void BandwidthEstimationHandler::notifyUpdate() { return; } - sink_ssrc_ = 0; - source_ssrcs_.clear(); - ELOG_DEBUG("Update MediaStream SSRCs"); - connection_->forEachMediaStream([this] (const std::shared_ptr &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()); - }); worker_ = connection_->getWorker(); @@ -222,11 +211,24 @@ void BandwidthEstimationHandler::pickEstimatorFromHeader() { } void BandwidthEstimationHandler::pickEstimator() { + // TODO(pedro): HARDCODED ESTIMATOR rbe_ = picker_->pickEstimator(true, clock_, this); rbe_->SetMinBitrate(min_bitrate_bps_); } void BandwidthEstimationHandler::sendREMBPacket() { + sink_ssrc_ = 0; + source_ssrcs_.clear(); + ELOG_DEBUG("Update MediaStream SSRCs"); + connection_->forEachMediaStream([this] (const std::shared_ptr &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; @@ -237,7 +239,7 @@ void BandwidthEstimationHandler::sendREMBPacket() { remb_packet_.setSSRC(sink_ssrc_); remb_packet_.setSourceSSRC(0); - remb_packet_.setLength(5); + remb_packet_.setLength(4 + source_ssrcs_.size()); uint32_t capped_bitrate = bitrate_; ELOG_DEBUG("Bitrates min(%u,%u) = %u", bitrate_, max_video_bw_, capped_bitrate); remb_packet_.setREMBBitRate(capped_bitrate); From 4c02f9374e93c08a43863522994b8b692a53925e Mon Sep 17 00:00:00 2001 From: Pedro Rodriguez Date: Thu, 3 Jun 2021 15:26:11 +0200 Subject: [PATCH 03/12] Include simulcast options in stream --- .../erizoClient/src/ErizoConnectionManager.js | 10 +- erizo_controller/erizoClient/src/Room.js | 2 +- erizo_controller/erizoClient/src/Stream.js | 15 ++- .../src/webrtc-stacks/BaseStack.js | 104 ++++++++++++------ .../src/webrtc-stacks/ChromeStableStack.js | 77 ++----------- .../src/webrtc-stacks/FirefoxStack.js | 27 +++-- 6 files changed, 113 insertions(+), 122 deletions(-) diff --git a/erizo_controller/erizoClient/src/ErizoConnectionManager.js b/erizo_controller/erizoClient/src/ErizoConnectionManager.js index fdd9da289e..f3191bff30 100644 --- a/erizo_controller/erizoClient/src/ErizoConnectionManager.js +++ b/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); + this.stack.addStream(stream); } } @@ -164,12 +164,12 @@ class ErizoConnection extends EventEmitterConst { this.stack.updateSpec(configInput, streamId, callback); } - updateSimulcastLayersBitrate(bitrates) { - this.stack.updateSimulcastLayersBitrate(bitrates); + updateSimulcastLayersBitrate(bitrates, licodeStream) { + this.stack.updateSimulcastLayersBitrate(bitrates, licodeStream); } - updateSimulcastActiveLayers(layersInfo) { - this.stack.updateSimulcastActiveLayers(layersInfo); + updateSimulcastActiveLayers(layersInfo, licodeStream) { + this.stack.updateSimulcastActiveLayers(layersInfo, licodeStream); } setQualityLevel(level) { diff --git a/erizo_controller/erizoClient/src/Room.js b/erizo_controller/erizoClient/src/Room.js index da3dff8f8b..a93fb34588 100644 --- a/erizo_controller/erizoClient/src/Room.js +++ b/erizo_controller/erizoClient/src/Room.js @@ -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; } @@ -283,6 +282,7 @@ const Room = (altIo, altConnectionHelpers, altConnectionManager, specInput) => { const createLocalStreamErizoConnection = (streamInput, connectionId, erizoId, options) => { const stream = streamInput; const connectionOpts = getErizoConnectionOptions(stream, connectionId, erizoId, options); + stream.setSimulcastConfig(options.simulcast); const connection = that.erizoConnectionManager .getOrBuildErizoConnection(connectionOpts, erizoId, spec.singlePC); stream.addPC(connection); diff --git a/erizo_controller/erizoClient/src/Stream.js b/erizo_controller/erizoClient/src/Stream.js index 43c708dca4..91046f0ce3 100644 --- a/erizo_controller/erizoClient/src/Stream.js +++ b/erizo_controller/erizoClient/src/Stream.js @@ -37,6 +37,7 @@ const Stream = (altConnectionHelpers, specInput) => { callbackReceived: false, pcEventReceived: false, }; + that.simulcast = {}; that.p2p = false; that.ConnectionHelpers = altConnectionHelpers === undefined ? ConnectionHelpers : altConnectionHelpers; @@ -129,6 +130,16 @@ const Stream = (altConnectionHelpers, specInput) => { that.hasMedia = () => spec.audio || spec.video || spec.screen; that.isExternal = () => that.url !== undefined || that.recording !== undefined; + that.hasSimulcast = () => Object.keys(that.simulcast).length !== 0; + that.setSimulcastConfig = (simulcast) => { + that.simulcast = Object.assign(that.simulcast, simulcast); + }; + that.getSimulcastConfig = () => that.simulcast; + that.setSpatialLayersConfigs = (config) => { + if (that.hasSimulcast()) { + that.simulcast.spatialLayerConfigs = config; + } + }; that.addPC = (pc, p2pKey = undefined) => { if (p2pKey) { @@ -494,13 +505,13 @@ const Stream = (altConnectionHelpers, specInput) => { that.updateSimulcastLayersBitrate = (bitrates) => { if (that.pc && that.local) { - that.pc.updateSimulcastLayersBitrate(bitrates); + that.pc.updateSimulcastLayersBitrate(bitrates, that); } }; that.updateSimulcastActiveLayers = (layersInfo) => { if (that.pc && that.local) { - that.pc.updateSimulcastActiveLayers(layersInfo); + that.pc.updateSimulcastActiveLayers(layersInfo, that); } }; diff --git a/erizo_controller/erizoClient/src/webrtc-stacks/BaseStack.js b/erizo_controller/erizoClient/src/webrtc-stacks/BaseStack.js index 68815d9d57..dccc3cec67 100644 --- a/erizo_controller/erizoClient/src/webrtc-stacks/BaseStack.js +++ b/erizo_controller/erizoClient/src/webrtc-stacks/BaseStack.js @@ -96,40 +96,30 @@ const BaseStack = (specInput) => { return sdpInput; }; - that.enableSimulcast = (sdpInput) => { - log.error('message: Simulcast not implemented'); - return sdpInput; - }; - - const setSpatialLayersConfig = (field, values, check = () => true) => { - if (that.simulcast) { + const setSpatialLayersConfig = (field, values, stream, check = () => true) => { + if (stream.hasSimulcast()) { Object.keys(values).forEach((layerId) => { const value = values[layerId]; - if (!that.simulcast.spatialLayerConfigs) { - that.simulcast.spatialLayerConfigs = {}; - } - if (!that.simulcast.spatialLayerConfigs[layerId]) { - that.simulcast.spatialLayerConfigs[layerId] = {}; + const spatialLayerConfigs = stream.getSimulcastConfig().spatialLayerConfigs || {}; + if (!spatialLayerConfigs[layerId]) { + spatialLayerConfigs[layerId] = {}; } if (check(value)) { - that.simulcast.spatialLayerConfigs[layerId][field] = value; + spatialLayerConfigs[layerId][field] = value; } + stream.setSpatialLayersConfigs(spatialLayerConfigs); }); - that.setSimulcastLayersConfig(); + that.setSimulcastLayersConfig(stream); } }; - that.updateSimulcastLayersBitrate = (bitrates) => { - setSpatialLayersConfig('maxBitrate', bitrates); + that.updateSimulcastLayersBitrate = (bitrates, licodeStream) => { + setSpatialLayersConfig('maxBitrate', bitrates, licodeStream); }; - that.updateSimulcastActiveLayers = (layersInfo) => { + that.updateSimulcastActiveLayers = (layersInfo, licodeStream) => { const ifIsBoolean = value => value === true || value === false; - setSpatialLayersConfig('active', layersInfo, ifIsBoolean); - }; - - that.setSimulcastLayersConfig = () => { - log.error('message: Simulcast not implemented'); + setSpatialLayersConfig('active', layersInfo, licodeStream, ifIsBoolean); }; that.setSimulcast = (enable) => { @@ -190,8 +180,9 @@ const BaseStack = (specInput) => { { rid: '1', scaleResolutionDownBy: 4 }, ]; - const getSimulcastParameters = () => { - let numSpatialLayers = that.simulcast.numSpatialLayers || defaultSimulcastSpatialLayers; + const getSimulcastParameters = (streamInput) => { + const config = streamInput.getSimulcastConfig(); + let numSpatialLayers = config.numSpatialLayers || defaultSimulcastSpatialLayers; const totalLayers = possibleLayers.length; numSpatialLayers = numSpatialLayers < totalLayers ? numSpatialLayers : totalLayers; @@ -203,24 +194,71 @@ const BaseStack = (specInput) => { return parameters; }; + const configureParameterForLayer = (parameters, config, layerId) => { + if (parameters.encodings[layerId] === undefined || + config[layerId] === undefined) { + return parameters; + } + const newParameters = parameters; + newParameters.encodings[layerId].maxBitrate = config[layerId].maxBitrate; + if (config[layerId].active !== undefined) { + newParameters.encodings[layerId].active = config[layerId].active; + } + return newParameters; + }; + + const setBitrateForVideoLayers = (sender, spatialLayerConfigs) => { + if (typeof sender.getParameters !== 'function' || typeof sender.setParameters !== 'function') { + log.warning('message: Cannot set simulcast layers bitrate, reason: get/setParameters not available'); + return; + } + let parameters = sender.getParameters(); + Object.keys(spatialLayerConfigs).forEach((layerId) => { + if (parameters.encodings[layerId] !== undefined) { + log.warning(`message: Configure parameters for layer, layer: ${layerId}, config: ${spatialLayerConfigs[layerId]}`); + parameters = configureParameterForLayer(parameters, spatialLayerConfigs, layerId); + } + }); + sender.setParameters(parameters) + .then((result) => { + log.debug(`message: Success setting simulcast layer configs, result: ${result}`); + }) + .catch((e) => { + log.warning(`message: Error setting simulcast layer configs, error: ${e}`); + }); + }; + + that.setSimulcastLayersConfig = (streamInput) => { + log.debug(`message: Maybe set simulcast Layers config, simulcast: ${JSON.stringify(that.simulcast)}`); + if (streamInput.hasSimulcast() && streamInput.getSimulcastConfig().spatialLayerConfigs) { + streamInput.stream.transceivers.forEach((transceiver) => { + if (transceiver.sender.track.kind === 'video') { + setBitrateForVideoLayers( + transceiver.sender, + streamInput.getSimulcastConfig().spatialLayerConfigs); + } + }); + } + }; + that.addStream = (streamInput) => { - const stream = streamInput; - stream.transceivers = []; - stream.getTracks().forEach(async (track) => { + const nativeStream = streamInput.stream; + nativeStream.transceivers = []; + nativeStream.getTracks().forEach(async (track) => { let options = {}; - if (track.kind === 'video' && that.simulcast) { + if (track.kind === 'video' && streamInput.getSimulcastConfig()) { options = { - sendEncodings: getSimulcastParameters(), + sendEncodings: getSimulcastParameters(streamInput), }; } - options.streams = [stream]; + options.streams = [nativeStream]; const transceiver = that.peerConnection.addTransceiver(track, options); - stream.transceivers.push(transceiver); + nativeStream.transceivers.push(transceiver); }); }; - that.removeStream = (stream) => { - stream.transceivers.forEach((transceiver) => { + that.removeStream = (nativeStream) => { + nativeStream.transceivers.forEach((transceiver) => { log.error('Stopping transceiver', transceiver); // Don't remove the tagged m section, which is the first one (mid=0). if (transceiver.mid === '0') { diff --git a/erizo_controller/erizoClient/src/webrtc-stacks/ChromeStableStack.js b/erizo_controller/erizoClient/src/webrtc-stacks/ChromeStableStack.js index c0a45348f0..ff1490be58 100644 --- a/erizo_controller/erizoClient/src/webrtc-stacks/ChromeStableStack.js +++ b/erizo_controller/erizoClient/src/webrtc-stacks/ChromeStableStack.js @@ -14,64 +14,6 @@ const ChromeStableStack = (specInput) => { offerToReceiveAudio: true, }; - that.enableSimulcast = (sdpInput) => { - let result; - let sdp = sdpInput; - if (!that.simulcast) { - return sdp; - } - const hasAlreadySetSimulcast = sdp.match(new RegExp('a=ssrc-group:SIM', 'g')) !== null; - if (hasAlreadySetSimulcast) { - return sdp; - } - // TODO(javier): Improve the way we check for current video ssrcs - const matchGroup = sdp.match(/a=ssrc-group:FID ([0-9]*) ([0-9]*)\r?\n/); - if (!matchGroup || (matchGroup.length <= 0)) { - return sdp; - } - // TODO (pedro): Consider adding these to SdpHelpers - const numSpatialLayers = that.simulcast.numSpatialLayers || defaultSimulcastSpatialLayers; - const baseSsrc = parseInt(matchGroup[1], 10); - const baseSsrcRtx = parseInt(matchGroup[2], 10); - const cname = sdp.match(new RegExp(`a=ssrc:${matchGroup[1]} cname:(.*)\r?\n`))[1]; - const msid = sdp.match(new RegExp(`a=ssrc:${matchGroup[1]} msid:(.*)\r?\n`))[1]; - const mslabel = sdp.match(new RegExp(`a=ssrc:${matchGroup[1]} mslabel:(.*)\r?\n`))[1]; - const label = sdp.match(new RegExp(`a=ssrc:${matchGroup[1]} label:(.*)\r?\n`))[1]; - - sdp.match(new RegExp(`a=ssrc:${matchGroup[1]}.*\r?\n`, 'g')).forEach((line) => { - sdp = sdp.replace(line, ''); - }); - sdp.match(new RegExp(`a=ssrc:${matchGroup[2]}.*\r?\n`, 'g')).forEach((line) => { - sdp = sdp.replace(line, ''); - }); - - const spatialLayers = [baseSsrc]; - const spatialLayersRtx = [baseSsrcRtx]; - - for (let i = 1; i < numSpatialLayers; i += 1) { - spatialLayers.push(baseSsrc + (i * 1000)); - spatialLayersRtx.push(baseSsrcRtx + (i * 1000)); - } - - result = SdpHelpers.addSim(spatialLayers); - let spatialLayerId; - let spatialLayerIdRtx; - for (let i = 0; i < spatialLayers.length; i += 1) { - spatialLayerId = spatialLayers[i]; - spatialLayerIdRtx = spatialLayersRtx[i]; - result += SdpHelpers.addGroup(spatialLayerId, spatialLayerIdRtx); - } - - for (let i = 0; i < spatialLayers.length; i += 1) { - spatialLayerId = spatialLayers[i]; - spatialLayerIdRtx = spatialLayersRtx[i]; - result += SdpHelpers.addSpatialLayer(cname, - msid, mslabel, label, spatialLayerId, spatialLayerIdRtx); - } - result += 'a=x-google-flag:conference\r\n'; - return sdp.replace(matchGroup[0], result); - }; - const configureParameter = (parameters, config, layerId) => { if (parameters.encodings[layerId] === undefined || config[layerId] === undefined) { @@ -85,16 +27,17 @@ const ChromeStableStack = (specInput) => { return newParameters; }; - const setBitrateForVideoLayers = (sender) => { + const setBitrateForVideoLayers = (sender, streamInput) => { if (typeof sender.getParameters !== 'function' || typeof sender.setParameters !== 'function') { log.warning('message: Cannot set simulcast layers bitrate, reason: get/setParameters not available'); return; } let parameters = sender.getParameters(); - Object.keys(that.simulcast.spatialLayerConfigs).forEach((layerId) => { + const spatialLayerConfigs = streamInput.getSimulcastConfig().spatialLayerConfigs; + Object.keys(spatialLayerConfigs).forEach((layerId) => { if (parameters.encodings[layerId] !== undefined) { - log.debug(`message: Configure parameters for layer, layer: ${layerId}, config: ${that.simulcast.spatialLayerConfigs[layerId]}`); - parameters = configureParameter(parameters, that.simulcast.spatialLayerConfigs, layerId); + log.warning(`message: Configure parameters for layer, layer: ${layerId}, config: ${spatialLayerConfigs[layerId]}`); + parameters = configureParameter(parameters, spatialLayerConfigs, layerId); } }); sender.setParameters(parameters) @@ -108,12 +51,12 @@ const ChromeStableStack = (specInput) => { that.prepareCreateOffer = () => Promise.resolve(); - that.setSimulcastLayersConfig = () => { + that.setSimulcastLayersConfig = (streamInput) => { log.debug(`message: Maybe set simulcast Layers config, simulcast: ${JSON.stringify(that.simulcast)}`); - if (that.simulcast && that.simulcast.spatialLayerConfigs) { - that.peerConnection.getSenders().forEach((sender) => { - if (sender.track.kind === 'video') { - setBitrateForVideoLayers(sender); + if (streamInput.getSimulcastConfig()) { + streamInput.stream.transceivers.forEach((transceiver) => { + if (transceiver.sender.track.kind === 'video') { + setBitrateForVideoLayers(transceiver.sender, streamInput); } }); } diff --git a/erizo_controller/erizoClient/src/webrtc-stacks/FirefoxStack.js b/erizo_controller/erizoClient/src/webrtc-stacks/FirefoxStack.js index b0147c44c0..eb1339b88c 100644 --- a/erizo_controller/erizoClient/src/webrtc-stacks/FirefoxStack.js +++ b/erizo_controller/erizoClient/src/webrtc-stacks/FirefoxStack.js @@ -15,10 +15,9 @@ const FirefoxStack = (specInput) => { log.debug('message: Starting Firefox stack'); const that = BaseStack(specInput); - that.enableSimulcast = sdp => sdp; - - const getSimulcastParameters = () => { - let numSpatialLayers = that.simulcast.numSpatialLayers || defaultSimulcastSpatialLayers; + const getSimulcastParameters = (streamInput) => { + const config = streamInput.getSimulcastConfig(); + let numSpatialLayers = config.numSpatialLayers || defaultSimulcastSpatialLayers; const totalLayers = possibleLayers.length; numSpatialLayers = numSpatialLayers < totalLayers ? numSpatialLayers : totalLayers; @@ -30,28 +29,28 @@ const FirefoxStack = (specInput) => { return parameters; }; - const getSimulcastParametersForFirefox = (sender) => { + const getSimulcastParametersForFirefox = (sender, streamInput) => { const parameters = sender.getParameters() || {}; - parameters.encodings = getSimulcastParameters(); + parameters.encodings = getSimulcastParameters(streamInput); return sender.setParameters(parameters); }; that.addStream = (streamInput) => { - const stream = streamInput; - stream.transceivers = []; - stream.getTracks().forEach(async (track) => { + const nativeStream = streamInput.stream; + nativeStream.transceivers = []; + nativeStream.getTracks().forEach(async (track) => { let options = {}; - if (track.kind === 'video' && that.simulcast) { + if (track.kind === 'video' && streamInput.simulcast) { options = { sendEncodings: [], }; } - options.streams = [stream]; + options.streams = [nativeStream]; const transceiver = that.peerConnection.addTransceiver(track, options); - stream.transceivers.push(transceiver); - if (track.kind === 'video' && that.simulcast) { - getSimulcastParametersForFirefox(transceiver.sender).catch(() => {}); + nativeStream.transceivers.push(transceiver); + if (track.kind === 'video' && streamInput.simulcast) { + getSimulcastParametersForFirefox(transceiver.sender, streamInput).catch(() => {}); } }); }; From 1fe25ae00653545196e52c07063670286044b887 Mon Sep 17 00:00:00 2001 From: Pedro Rodriguez Date: Thu, 3 Jun 2021 15:30:35 +0200 Subject: [PATCH 04/12] Add missing file --- .../src/webrtc-stacks/ChromeStableStack.js | 47 ------------------- 1 file changed, 47 deletions(-) diff --git a/erizo_controller/erizoClient/src/webrtc-stacks/ChromeStableStack.js b/erizo_controller/erizoClient/src/webrtc-stacks/ChromeStableStack.js index ff1490be58..1d10c2a66f 100644 --- a/erizo_controller/erizoClient/src/webrtc-stacks/ChromeStableStack.js +++ b/erizo_controller/erizoClient/src/webrtc-stacks/ChromeStableStack.js @@ -8,60 +8,13 @@ const ChromeStableStack = (specInput) => { log.debug(`message: Starting Chrome stable stack, spec: ${JSON.stringify(specInput)}`); const spec = specInput; const that = BaseStack(specInput); - const defaultSimulcastSpatialLayers = 2; that.mediaConstraints = { offerToReceiveVideo: true, offerToReceiveAudio: true, }; - const configureParameter = (parameters, config, layerId) => { - if (parameters.encodings[layerId] === undefined || - config[layerId] === undefined) { - return parameters; - } - const newParameters = parameters; - newParameters.encodings[layerId].maxBitrate = config[layerId].maxBitrate; - if (config[layerId].active !== undefined) { - newParameters.encodings[layerId].active = config[layerId].active; - } - return newParameters; - }; - - const setBitrateForVideoLayers = (sender, streamInput) => { - if (typeof sender.getParameters !== 'function' || typeof sender.setParameters !== 'function') { - log.warning('message: Cannot set simulcast layers bitrate, reason: get/setParameters not available'); - return; - } - let parameters = sender.getParameters(); - const spatialLayerConfigs = streamInput.getSimulcastConfig().spatialLayerConfigs; - Object.keys(spatialLayerConfigs).forEach((layerId) => { - if (parameters.encodings[layerId] !== undefined) { - log.warning(`message: Configure parameters for layer, layer: ${layerId}, config: ${spatialLayerConfigs[layerId]}`); - parameters = configureParameter(parameters, spatialLayerConfigs, layerId); - } - }); - sender.setParameters(parameters) - .then((result) => { - log.debug(`message: Success setting simulcast layer configs, result: ${result}`); - }) - .catch((e) => { - log.warning(`message: Error setting simulcast layer configs, error: ${e}`); - }); - }; - that.prepareCreateOffer = () => Promise.resolve(); - that.setSimulcastLayersConfig = (streamInput) => { - log.debug(`message: Maybe set simulcast Layers config, simulcast: ${JSON.stringify(that.simulcast)}`); - if (streamInput.getSimulcastConfig()) { - streamInput.stream.transceivers.forEach((transceiver) => { - if (transceiver.sender.track.kind === 'video') { - setBitrateForVideoLayers(transceiver.sender, streamInput); - } - }); - } - }; - that.setStartVideoBW = (sdpInfo) => { if (that.video && spec.startVideoBW) { log.debug(`message: startVideoBW, requested: ${spec.startVideoBW}`); From 28251cf019c3772b62034782d884a4602f1b83f0 Mon Sep 17 00:00:00 2001 From: Pedro Rodriguez Date: Fri, 4 Jun 2021 12:20:23 +0200 Subject: [PATCH 05/12] Remove hardcoded estimator --- erizo/src/erizo/rtp/BandwidthEstimationHandler.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/erizo/src/erizo/rtp/BandwidthEstimationHandler.cpp b/erizo/src/erizo/rtp/BandwidthEstimationHandler.cpp index 0cd878e6ba..007acb11cb 100644 --- a/erizo/src/erizo/rtp/BandwidthEstimationHandler.cpp +++ b/erizo/src/erizo/rtp/BandwidthEstimationHandler.cpp @@ -211,8 +211,7 @@ void BandwidthEstimationHandler::pickEstimatorFromHeader() { } void BandwidthEstimationHandler::pickEstimator() { - // TODO(pedro): HARDCODED ESTIMATOR - rbe_ = picker_->pickEstimator(true, clock_, this); + rbe_ = picker_->pickEstimator(using_absolute_send_time_, clock_, this); rbe_->SetMinBitrate(min_bitrate_bps_); } From dcb0e418a71848e74e184d7e18b3d72fcf9cabf5 Mon Sep 17 00:00:00 2001 From: Pedro Rodriguez Date: Fri, 4 Jun 2021 12:22:41 +0200 Subject: [PATCH 06/12] Fix simulcast config check --- erizo_controller/erizoClient/src/webrtc-stacks/BaseStack.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erizo_controller/erizoClient/src/webrtc-stacks/BaseStack.js b/erizo_controller/erizoClient/src/webrtc-stacks/BaseStack.js index dccc3cec67..a85b74bea1 100644 --- a/erizo_controller/erizoClient/src/webrtc-stacks/BaseStack.js +++ b/erizo_controller/erizoClient/src/webrtc-stacks/BaseStack.js @@ -246,7 +246,7 @@ const BaseStack = (specInput) => { nativeStream.transceivers = []; nativeStream.getTracks().forEach(async (track) => { let options = {}; - if (track.kind === 'video' && streamInput.getSimulcastConfig()) { + if (track.kind === 'video' && streamInput.hasSimulcast()) { options = { sendEncodings: getSimulcastParameters(streamInput), }; From 7afa19bf8fe59437bf4adc0026dd9209a0659780 Mon Sep 17 00:00:00 2001 From: Pedro Rodriguez Date: Fri, 11 Jun 2021 13:13:51 +0200 Subject: [PATCH 07/12] WIP refactor maxVideoBW --- erizo_controller/erizoClient/src/Room.js | 15 +- erizo_controller/erizoClient/src/Stream.js | 137 +++++++++++++-- .../src/webrtc-stacks/BaseStack.js | 163 +++++++----------- 3 files changed, 198 insertions(+), 117 deletions(-) diff --git a/erizo_controller/erizoClient/src/Room.js b/erizo_controller/erizoClient/src/Room.js index a93fb34588..3be509f9e8 100644 --- a/erizo_controller/erizoClient/src/Room.js +++ b/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,7 @@ 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()}`); + log.error(`message: Sending message, data: ${JSON.stringify(message)}, ${stream.toLog()}, ${toLog()}`); if (message && message.type && message.type === 'updatestream') { socket.sendSDP('streamMessage', { streamId, @@ -263,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, options); connection.on('connection-failed', that.dispatchEvent.bind(this)); stream.on('added', dispatchStreamSubscribed.bind(null, stream)); @@ -282,10 +282,9 @@ const Room = (altIo, altConnectionHelpers, altConnectionManager, specInput) => { const createLocalStreamErizoConnection = (streamInput, connectionId, erizoId, options) => { const stream = streamInput; const connectionOpts = getErizoConnectionOptions(stream, connectionId, erizoId, options); - stream.setSimulcastConfig(options.simulcast); 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()}`); @@ -809,6 +808,8 @@ const Room = (altIo, altConnectionHelpers, altConnectionManager, specInput) => { options.maxVideoBW = spec.maxVideoBW; } + stream.maxVideoBW = options.maxVideoBW; + if (options.minVideoBW === undefined) { options.minVideoBW = 0; } diff --git a/erizo_controller/erizoClient/src/Stream.js b/erizo_controller/erizoClient/src/Stream.js index 91046f0ce3..150424019b 100644 --- a/erizo_controller/erizoClient/src/Stream.js +++ b/erizo_controller/erizoClient/src/Stream.js @@ -33,11 +33,15 @@ const Stream = (altConnectionHelpers, specInput) => { that.desktopStreamId = spec.desktopStreamId; that.audioMuted = false; that.videoMuted = false; + that.maxVideoBW = undefined; + that.maxAudioBW = undefined; + that.limitMaxVideoBW = undefined; + that.limitMaxAudioBW = undefined; that.unsubscribing = { callbackReceived: false, pcEventReceived: false, }; - that.simulcast = {}; + const senderEncodingsParameters = {}; that.p2p = false; that.ConnectionHelpers = altConnectionHelpers === undefined ? ConnectionHelpers : altConnectionHelpers; @@ -69,6 +73,59 @@ const Stream = (altConnectionHelpers, specInput) => { that.local = true; } + const setBitrateForVideoLayers = (sender, spatialLayerConfigs) => { + if (typeof sender.getParameters !== 'function' || typeof sender.setParameters !== 'function') { + log.warning('message: Cannot set simulcast layers bitrate, reason: get/setParameters not available'); + return; + } + let parameters = sender.getParameters(); + Object.keys(spatialLayerConfigs).forEach((layerId) => { + if (parameters.encodings[layerId] !== undefined) { + log.warning(`message: Configure parameters for layer, layer: ${layerId}, config: ${spatialLayerConfigs[layerId]}`); + parameters = that.pc.stack.configureParameterForLayer( + parameters, spatialLayerConfigs, layerId); + } + }); + + sender.setParameters(parameters) + .then((result) => { + log.debug(`message: Success setting simulcast layer configs, result: ${result}`); + }) + .catch((e) => { + log.warning(`message: Error setting simulcast layer configs, error: ${e}`); + }); + }; + + const applySenderEncodingConfig = () => { + that.stream.transceivers.forEach((transceiver) => { + if (transceiver.sender && transceiver.sender.track.kind === 'video') { + setBitrateForVideoLayers( + transceiver.sender, + that.getSimulcastConfig()); + } + }); + }; + + const setVideoBitrateForMaxLayer = (videoBitrate) => { + const bitrateToApply = videoBitrate < that.maxVideoBW ? videoBitrate : that.maxVideoBW; + const highestLayer = Math.max(...Object.keys(senderEncodingsParameters)); + senderEncodingsParameters[highestLayer].maxBitrate = bitrateToApply * 1000; + applySenderEncodingConfig(); + }; + + const setEncodingConfig = (field, values, check = () => true) => { + Object.keys(values).forEach((layerId) => { + const value = values[layerId]; + + if (!senderEncodingsParameters[layerId]) { + senderEncodingsParameters[layerId] = {}; + } + if (check(value)) { + senderEncodingsParameters[layerId][field] = value; + } + }); + }; + // Public functions that.getID = () => { let id; @@ -130,18 +187,57 @@ const Stream = (altConnectionHelpers, specInput) => { that.hasMedia = () => spec.audio || spec.video || spec.screen; that.isExternal = () => that.url !== undefined || that.recording !== undefined; - that.hasSimulcast = () => Object.keys(that.simulcast).length !== 0; - that.setSimulcastConfig = (simulcast) => { - that.simulcast = Object.assign(that.simulcast, simulcast); + + const setMaxVideoBW = (maxVideoBW) => { + if (that.local) { + log.info(`message: Setting maxVideoBW, streamId: ${that.getID()}, maxVideoBW: ${maxVideoBW}`); + const translated = (maxVideoBW * 1000 * 0.90) - (50 * 40 * 8); + log.info(`translating maxVideoBW ${maxVideoBW} woudl be ${translated}`); + that.maxVideoBW = translated; + } else { + that.maxVideoBW = maxVideoBW; + } + }; - that.getSimulcastConfig = () => that.simulcast; - that.setSpatialLayersConfigs = (config) => { - if (that.hasSimulcast()) { - that.simulcast.spatialLayerConfigs = config; + + that.getMaxVideoBW = () => that.maxVideoBW; + that.hasSimulcast = () => Object.keys(senderEncodingsParameters).length > 1; + + const maybeSetSimulcastConfig = (config) => { + log.info('Setting simulcast config', config, 'MaxVideoBW is ', that.maxVideoBW); + if (!config) { + senderEncodingsParameters[0] = {}; // No simulcast + return; + } + const supportedLayerConfig = that.pc.stack.getSupportedLayerConfiguration(); + + const totalLayers = supportedLayerConfig.length; + const layersToConfigure = config.numSpatialLayers < totalLayers ? + config.numSpatialLayers : totalLayers; + + for (let index = 0; index < layersToConfigure; index += 1) { + senderEncodingsParameters[index] = {}; + } + if (that.maxVideoBW) { + log.warning('Setting maxVideoBW', that.maxVideoBW); + // senderEncodingsParameters[layersToConfigure - 1].maxBitrate = that.maxVideoBW; } }; - that.addPC = (pc, p2pKey = undefined) => { + const setLayersOptions = (options) => { + that.limitMaxAudioBW = options.limitMaxVideoBW; + that.limitMaxVideoBW = options.limitMaxVideoBW; + if (options.maxVideoBW) { + setMaxVideoBW(options.maxVideoBW); + } + if (that.local) { + maybeSetSimulcastConfig(options.simulcast); + } + }; + + that.getSimulcastConfig = () => Object.assign({}, senderEncodingsParameters); + + that.addPC = (pc, p2pKey = undefined, options) => { if (p2pKey) { that.p2p = true; if (that.pc === undefined) { @@ -160,6 +256,9 @@ const Stream = (altConnectionHelpers, specInput) => { that.pc.on('add-stream', onStreamAddedToPC); that.pc.on('remove-stream', onStreamRemovedFromPC); that.pc.on('ice-state-change', onICEConnectionStateChange); + if (options) { + setLayersOptions(options); + } }; // Sends data through this stream. @@ -383,6 +482,12 @@ const Stream = (altConnectionHelpers, specInput) => { that.checkOptions = (configInput, isUpdate) => { const config = configInput; // TODO: Check for any incompatible options + if (config.maxVideoBW && config.maxVideoBW > that.limitMaxVideoBW) { + config.maxVideoBW = that.limitMaxVideoBW; + } + if (config.maxAudioBW && config.maxAudioBW > that.limitMaxAudioBW) { + config.maxAudioBW = that.limitMaxAudioBW; + } if (isUpdate === true) { // We are updating the stream if (config.audio || config.screen) { log.warning(`message: Cannot update type of subscription, ${that.toLog()}`); @@ -505,13 +610,16 @@ const Stream = (altConnectionHelpers, specInput) => { that.updateSimulcastLayersBitrate = (bitrates) => { if (that.pc && that.local) { - that.pc.updateSimulcastLayersBitrate(bitrates, that); + setEncodingConfig('maxBitrate', bitrates); + applySenderEncodingConfig(); } }; that.updateSimulcastActiveLayers = (layersInfo) => { if (that.pc && that.local) { - that.pc.updateSimulcastActiveLayers(layersInfo, that); + const ifIsBoolean = value => value === true || value === false; + setEncodingConfig('active', layersInfo, ifIsBoolean); + applySenderEncodingConfig(); } }; @@ -520,12 +628,19 @@ const Stream = (altConnectionHelpers, specInput) => { if (that.pc) { that.checkOptions(config, true); if (that.local) { + if (config.maxVideoBW) { + setMaxVideoBW(config.maxVideoBW); + applySenderEncodingConfig(); + } if (that.room.p2p) { for (let index = 0; index < that.pc.length; index += 1) { that.pc[index].updateSpec(config, that.getID(), callback); } } else { that.pc.updateSpec(config, that.getID(), callback); + if (config.maxVideoBW) { + setVideoBitrateForMaxLayer(config.maxVideoBW); + } } } else { that.pc.updateSpec(config, that.getID(), callback); diff --git a/erizo_controller/erizoClient/src/webrtc-stacks/BaseStack.js b/erizo_controller/erizoClient/src/webrtc-stacks/BaseStack.js index a85b74bea1..cffec1b11b 100644 --- a/erizo_controller/erizoClient/src/webrtc-stacks/BaseStack.js +++ b/erizo_controller/erizoClient/src/webrtc-stacks/BaseStack.js @@ -50,7 +50,6 @@ const BaseStack = (specInput) => { specBase.callback({ type: that.peerConnection.localDescription.type, sdp: that.peerConnection.localDescription.sdp, - config: { maxVideoBW: specBase.maxVideoBW }, }); } catch (e) { logSDP('onnegotiationneeded - error', e.message); @@ -96,36 +95,6 @@ const BaseStack = (specInput) => { return sdpInput; }; - const setSpatialLayersConfig = (field, values, stream, check = () => true) => { - if (stream.hasSimulcast()) { - Object.keys(values).forEach((layerId) => { - const value = values[layerId]; - const spatialLayerConfigs = stream.getSimulcastConfig().spatialLayerConfigs || {}; - if (!spatialLayerConfigs[layerId]) { - spatialLayerConfigs[layerId] = {}; - } - if (check(value)) { - spatialLayerConfigs[layerId][field] = value; - } - stream.setSpatialLayersConfigs(spatialLayerConfigs); - }); - that.setSimulcastLayersConfig(stream); - } - }; - - that.updateSimulcastLayersBitrate = (bitrates, licodeStream) => { - setSpatialLayersConfig('maxBitrate', bitrates, licodeStream); - }; - - that.updateSimulcastActiveLayers = (layersInfo, licodeStream) => { - const ifIsBoolean = value => value === true || value === false; - setSpatialLayersConfig('active', layersInfo, licodeStream, ifIsBoolean); - }; - - that.setSimulcast = (enable) => { - that.simulcast = enable; - }; - that.setVideo = (video) => { that.video = video; }; @@ -134,27 +103,9 @@ const BaseStack = (specInput) => { that.audio = audio; }; - that.updateSpec = (configInput, streamId, callback = () => {}) => { + that.updateSpec = (configInput, streamId) => { const config = configInput; - const shouldApplyMaxVideoBWToSdp = specBase.p2p && config.maxVideoBW; const shouldSendMaxVideoBWInOptions = !specBase.p2p && config.maxVideoBW; - if (config.maxVideoBW) { - log.debug(`message: Maxvideo Requested, value: ${config.maxVideoBW}, limit: ${specBase.limitMaxVideoBW}`); - if (config.maxVideoBW > specBase.limitMaxVideoBW) { - config.maxVideoBW = specBase.limitMaxVideoBW; - } - specBase.maxVideoBW = config.maxVideoBW; - log.debug(`message: Maxvideo Result, value: ${config.maxVideoBW}, limit: ${specBase.limitMaxVideoBW}`); - } - if (config.maxAudioBW) { - if (config.maxAudioBW > specBase.limitMaxAudioBW) { - config.maxAudioBW = specBase.limitMaxAudioBW; - } - specBase.maxAudioBW = config.maxAudioBW; - } - if (shouldApplyMaxVideoBWToSdp || config.maxAudioBW) { - that.enqueuedCalls.negotiationQueue.negotiateMaxBW(config, callback); - } if (shouldSendMaxVideoBWInOptions || config.minVideoBW || (config.slideShowMode !== undefined) || @@ -172,29 +123,31 @@ const BaseStack = (specInput) => { } }; - const defaultSimulcastSpatialLayers = 3; - - const possibleLayers = [ + const supportedLayerConfiguration = [ { rid: '3' }, { rid: '2', scaleResolutionDownBy: 2 }, { rid: '1', scaleResolutionDownBy: 4 }, ]; - const getSimulcastParameters = (streamInput) => { - const config = streamInput.getSimulcastConfig(); - let numSpatialLayers = config.numSpatialLayers || defaultSimulcastSpatialLayers; - const totalLayers = possibleLayers.length; - numSpatialLayers = numSpatialLayers < totalLayers ? - numSpatialLayers : totalLayers; - const parameters = []; + that.getSupportedLayerConfiguration = () => supportedLayerConfiguration; - for (let layer = totalLayers - 1; layer >= totalLayers - numSpatialLayers; layer -= 1) { - parameters.push(possibleLayers[layer]); + const translateSimulcastParameters = (streamInput) => { + const parameters = []; + const totalLayers = supportedLayerConfiguration.length; + const requestedLayers = Object.keys(streamInput.getSimulcastConfig()).length; + const layersToUse = requestedLayers < totalLayers ? requestedLayers : totalLayers; + + for (let layer = totalLayers - 1; layer >= totalLayers - layersToUse; layer -= 1) { + const layerConfig = Object.assign({}, streamInput.getSimulcastConfig()[layer]); + Object.assign(layerConfig, supportedLayerConfiguration[layer]); + console.warn('LAyerConfig is', layerConfig); + parameters.push(layerConfig); } + console.warn('Parameters translated', parameters, 'simulcastConfig', streamInput.getSimulcastConfig()); return parameters; }; - const configureParameterForLayer = (parameters, config, layerId) => { + that.configureParameterForLayer = (parameters, config, layerId) => { if (parameters.encodings[layerId] === undefined || config[layerId] === undefined) { return parameters; @@ -207,39 +160,6 @@ const BaseStack = (specInput) => { return newParameters; }; - const setBitrateForVideoLayers = (sender, spatialLayerConfigs) => { - if (typeof sender.getParameters !== 'function' || typeof sender.setParameters !== 'function') { - log.warning('message: Cannot set simulcast layers bitrate, reason: get/setParameters not available'); - return; - } - let parameters = sender.getParameters(); - Object.keys(spatialLayerConfigs).forEach((layerId) => { - if (parameters.encodings[layerId] !== undefined) { - log.warning(`message: Configure parameters for layer, layer: ${layerId}, config: ${spatialLayerConfigs[layerId]}`); - parameters = configureParameterForLayer(parameters, spatialLayerConfigs, layerId); - } - }); - sender.setParameters(parameters) - .then((result) => { - log.debug(`message: Success setting simulcast layer configs, result: ${result}`); - }) - .catch((e) => { - log.warning(`message: Error setting simulcast layer configs, error: ${e}`); - }); - }; - - that.setSimulcastLayersConfig = (streamInput) => { - log.debug(`message: Maybe set simulcast Layers config, simulcast: ${JSON.stringify(that.simulcast)}`); - if (streamInput.hasSimulcast() && streamInput.getSimulcastConfig().spatialLayerConfigs) { - streamInput.stream.transceivers.forEach((transceiver) => { - if (transceiver.sender.track.kind === 'video') { - setBitrateForVideoLayers( - transceiver.sender, - streamInput.getSimulcastConfig().spatialLayerConfigs); - } - }); - } - }; that.addStream = (streamInput) => { const nativeStream = streamInput.stream; @@ -248,7 +168,7 @@ const BaseStack = (specInput) => { let options = {}; if (track.kind === 'video' && streamInput.hasSimulcast()) { options = { - sendEncodings: getSimulcastParameters(streamInput), + sendEncodings: translateSimulcastParameters(streamInput), }; } options.streams = [nativeStream]; @@ -278,9 +198,51 @@ const BaseStack = (specInput) => { that.getLocalDescription = async () => that.peerConnection.localDescription.sdp; that.setRemoteDescription = async (description) => { + console.warn('SetRemoteDescription', description); await that.peerConnection.setRemoteDescription(description); }; + // const setMediaBitrate = (sdp, media, bitrate) => { + // const lines = sdp.split('\n'); + // let line = -1; + // for (let i = 0; i < lines.length; i += 1) { + // if (lines[i].indexOf(`a=mid:${media}`) === 0) { + // line = i; + // break; + // } + // } + // if (line === -1) { + // console.warn('Could not find the m line for', media); + // return sdp; + // } + // console.warn('Found the m line for', media, 'at line', line); + + // // Pass the m line + // line += 1; + + // // Skip i and c lines + // while (lines[line].indexOf('i=') === 0 || lines[line].indexOf('c=') === 0) { + // line += 1; + // } + + // // If we're on a b line, replace it + // if (lines[line].indexOf('b') === 0) { + // console.warn('Replaced b line at line', line); + // lines[line] = `b=AS:${bitrate}`; + // return lines.join('\n'); + // } + + // // Add a new b line + // console.warn('Adding new b line before line', line); + // let newLines = lines.slice(0, line); + // newLines.push(`b=AS:${bitrate}`); + // console.warn('newLinesbefore', newLines); + // newLines = newLines.concat(lines.slice(line, lines.length)); + // console.warn('newLines', newLines); + // return newLines.join('\n'); + // }; + + that.processSignalingMessage = async (msgInput) => { const msg = msgInput; logSDP('processSignalingMessage, type: ', msgInput.type); @@ -303,13 +265,17 @@ const BaseStack = (specInput) => { log.error(`message: Received error signaling message, state: ${msgInput.previousType}`); } else if (['offer', 'answer'].indexOf(msgInput.type) !== -1) { try { + if (msg.type === 'answer') { + // msg.sdp = setMediaBitrate(msg.sdp, '1', 500); + // msg.sdp = setMediaBitrate(msg.sdp, '2', 1500); + // console.warn('Setting answer', msg.sdp); + } await that.peerConnection.setRemoteDescription(msg); if (msg.type === 'offer') { await that.peerConnection.setLocalDescription(); specBase.callback({ type: that.peerConnection.localDescription.type, sdp: that.peerConnection.localDescription.sdp, - config: { maxVideoBW: specBase.maxVideoBW }, }); } } catch (e) { @@ -318,7 +284,6 @@ const BaseStack = (specInput) => { specBase.callback({ type: 'offer-error', sdp: that.peerConnection.localDescription.sdp, - config: { maxVideoBW: specBase.maxVideoBW }, }); } } From 304608d38caa1235ce455049f7b23580309fe479 Mon Sep 17 00:00:00 2001 From: Pedro Rodriguez Date: Wed, 16 Jun 2021 17:27:29 +0200 Subject: [PATCH 08/12] fix firefox --- erizo_controller/erizoClient/src/Stream.js | 12 +++---- .../src/webrtc-stacks/FirefoxStack.js | 34 +++---------------- 2 files changed, 10 insertions(+), 36 deletions(-) diff --git a/erizo_controller/erizoClient/src/Stream.js b/erizo_controller/erizoClient/src/Stream.js index 2c173a402d..64ea1af503 100644 --- a/erizo_controller/erizoClient/src/Stream.js +++ b/erizo_controller/erizoClient/src/Stream.js @@ -96,7 +96,7 @@ const Stream = (altConnectionHelpers, specInput) => { return newParameters; }; - const applyLicodeSenderParameters = () => { + that.applySenderEncoderParameters = () => { that.stream.transceivers.forEach((transceiver) => { if (transceiver.sender && transceiver.sender.track.kind === 'video') { const parameters = transceiver.sender.getParameters(); @@ -120,7 +120,7 @@ const Stream = (altConnectionHelpers, specInput) => { }); }; - const initializeEncoderConfig = (simulcastConfig) => { + const initializeEncoderParameters = (simulcastConfig) => { log.info('Initializing encoder simulcastConfig', simulcastConfig, 'MaxVideoBW is ', that.maxVideoBW); if (!simulcastConfig) { videoSenderLicodeParameters[0] = { maxBitrate: that.maxVideoBW }; // No simulcast @@ -144,7 +144,7 @@ const Stream = (altConnectionHelpers, specInput) => { setMaxVideoBW(options.maxVideoBW); } if (that.local) { - initializeEncoderConfig(options.simulcast); + initializeEncoderParameters(options.simulcast); } }; @@ -626,7 +626,7 @@ const Stream = (altConnectionHelpers, specInput) => { limitedBitrates[key] > that.maxVideoBW ? that.maxVideoBW : limitedBitrates[key]; }); setEncodingConfig('maxBitrate', limitedBitrates); - applyLicodeSenderParameters(); + that.applySenderEncoderParameters(); } }; @@ -634,7 +634,7 @@ const Stream = (altConnectionHelpers, specInput) => { if (that.pc && that.local) { const ifIsBoolean = value => value === true || value === false; setEncodingConfig('active', layersInfo, ifIsBoolean); - applyLicodeSenderParameters(); + that.applySenderEncoderParameters(); } }; @@ -645,7 +645,7 @@ const Stream = (altConnectionHelpers, specInput) => { if (that.local) { if (config.maxVideoBW) { setMaxVideoBW(config.maxVideoBW); - applyLicodeSenderParameters(); + that.applySenderEncoderParameters(); } if (that.room.p2p) { for (let index = 0; index < that.pc.length; index += 1) { diff --git a/erizo_controller/erizoClient/src/webrtc-stacks/FirefoxStack.js b/erizo_controller/erizoClient/src/webrtc-stacks/FirefoxStack.js index 6022b7b10e..941c7d4d57 100644 --- a/erizo_controller/erizoClient/src/webrtc-stacks/FirefoxStack.js +++ b/erizo_controller/erizoClient/src/webrtc-stacks/FirefoxStack.js @@ -3,38 +3,11 @@ import BaseStack from './BaseStack'; const log = Logger.module('FirefoxStack'); -const defaultSimulcastSpatialLayers = 3; - -const scaleResolutionDownBase = 2; -const scaleResolutionDownBaseScreenshare = 1; const FirefoxStack = (specInput) => { log.debug('message: Starting Firefox stack'); const that = BaseStack(specInput); - const getSimulcastParameters = (streamInput) => { - const numSpatialLayers = Object.keys(streamInput.getSimulcastConfig()).length || - defaultSimulcastSpatialLayers; - const parameters = []; - const isScreenshare = streamInput.hasScreen(); - const base = isScreenshare ? scaleResolutionDownBaseScreenshare : scaleResolutionDownBase; - - for (let layer = 1; layer <= numSpatialLayers; layer += 1) { - parameters.push({ - rid: (layer).toString(), - scaleResolutionDownBy: base ** (numSpatialLayers - layer), - }); - } - return parameters; - }; - - const getSimulcastParametersForFirefox = (sender, streamInput) => { - const parameters = sender.getParameters() || {}; - parameters.encodings = getSimulcastParameters(streamInput); - - return sender.setParameters(parameters); - }; - that.addStream = (streamInput) => { const nativeStream = streamInput.stream; nativeStream.transceivers = []; @@ -48,9 +21,10 @@ const FirefoxStack = (specInput) => { options.streams = [nativeStream]; const transceiver = that.peerConnection.addTransceiver(track, options); nativeStream.transceivers.push(transceiver); - if (track.kind === 'video' && streamInput.simulcast) { - getSimulcastParametersForFirefox(transceiver.sender, streamInput).catch(() => {}); - } + const parameters = transceiver.sender.getParameters() || {}; + parameters.encodings = streamInput.generateEncoderParameters(); + console.warn('parameters to set', parameters); + return transceiver.sender.setParameters(parameters); }); }; From b98b2f790a51d1a4184f8d5230e4e4f83a285979 Mon Sep 17 00:00:00 2001 From: Pedro Rodriguez Date: Thu, 17 Jun 2021 11:11:36 +0200 Subject: [PATCH 09/12] Improve notifyToHandlers --- erizo/src/erizo/WebRtcConnection.cpp | 4 +--- erizo/src/erizo/rtp/BandwidthEstimationHandler.cpp | 12 +++++------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/erizo/src/erizo/WebRtcConnection.cpp b/erizo/src/erizo/WebRtcConnection.cpp index 0c7f5cd919..f972ba772e 100644 --- a/erizo/src/erizo/WebRtcConnection.cpp +++ b/erizo/src/erizo/WebRtcConnection.cpp @@ -404,7 +404,6 @@ boost::future WebRtcConnection::addMediaStream(std::shared_ptrtask([weak_this, task_promise, media_stream] { if (auto this_ptr = weak_this.lock()) { this_ptr->addMediaStreamSync(media_stream); - this_ptr->notifyUpdateToHandlers(); } task_promise->set_value(); }); @@ -449,7 +448,6 @@ boost::future WebRtcConnection::removeMediaStream(const std::string& strea } }); } - connection->notifyUpdateToHandlers(); }); } @@ -663,7 +661,6 @@ std::shared_ptr WebRtcConnection::getLocalSdpInfoSync() { local_sdp_->profile = remote_sdp_->profile; auto local_sdp_copy = std::make_shared(*local_sdp_.get()); - notifyUpdateToHandlers(); return local_sdp_copy; } @@ -775,6 +772,7 @@ boost::future WebRtcConnection::processRemoteSdp() { local_sdp_->setOfferSdp(remote_sdp_); extension_processor_.setSdpInfo(local_sdp_); + notifyUpdateToHandlers(); local_sdp_->updateSupportedExtensionMap(extension_processor_.getSupportedExtensionMap()); if (first_remote_sdp_processed_) { diff --git a/erizo/src/erizo/rtp/BandwidthEstimationHandler.cpp b/erizo/src/erizo/rtp/BandwidthEstimationHandler.cpp index aed1da0f8e..71a2687740 100644 --- a/erizo/src/erizo/rtp/BandwidthEstimationHandler.cpp +++ b/erizo/src/erizo/rtp/BandwidthEstimationHandler.cpp @@ -58,7 +58,6 @@ void BandwidthEstimationHandler::disable() { } void BandwidthEstimationHandler::notifyUpdate() { - ELOG_ERROR("NOTIFY UPDATE"); auto pipeline = getContext()->getPipelineShared(); if (pipeline && !connection_) { connection_ = pipeline->getService().get(); @@ -69,15 +68,14 @@ void BandwidthEstimationHandler::notifyUpdate() { return; } - - - worker_ = connection_->getWorker(); - stats_ = pipeline->getService(); RtpExtensionProcessor& ext_processor = connection_->getRtpExtensionProcessor(); - if (ext_processor.getVideoExtensionMap().size() == 0) { + updateExtensionMaps(ext_processor.getVideoExtensionMap(), ext_processor.getAudioExtensionMap()); + + if (initialized_) { return; } - updateExtensionMaps(ext_processor.getVideoExtensionMap(), ext_processor.getAudioExtensionMap()); + worker_ = connection_->getWorker(); + stats_ = pipeline->getService(); pickEstimator(); initialized_ = true; From 7d36acce7effc170bb01da9aab189140506f5fb9 Mon Sep 17 00:00:00 2001 From: Pedro Rodriguez Date: Thu, 17 Jun 2021 14:33:00 +0200 Subject: [PATCH 10/12] Fix tests --- .../erizo/rtp/BandwidthEstimationHandler.cpp | 6 +++-- .../erizo/rtp/BandwidthEstimationHandler.h | 1 - .../rtp/BandwidthEstimationHandlerTest.cpp | 24 ++++--------------- 3 files changed, 8 insertions(+), 23 deletions(-) diff --git a/erizo/src/erizo/rtp/BandwidthEstimationHandler.cpp b/erizo/src/erizo/rtp/BandwidthEstimationHandler.cpp index 71a2687740..9996a896a6 100644 --- a/erizo/src/erizo/rtp/BandwidthEstimationHandler.cpp +++ b/erizo/src/erizo/rtp/BandwidthEstimationHandler.cpp @@ -44,7 +44,7 @@ BandwidthEstimationHandler::BandwidthEstimationHandler(std::shared_ptrgetPipelineShared(); if (pipeline && !connection_) { connection_ = pipeline->getService().get(); @@ -246,7 +247,7 @@ void BandwidthEstimationHandler::sendREMBPacket() { remb_packet_.setSourceSSRC(0); remb_packet_.setLength(4 + source_ssrcs_.size()); uint32_t capped_bitrate = bitrate_; - ELOG_DEBUG("Bitrates min(%u,%u) = %u", bitrate_, max_video_bw_, capped_bitrate); + ELOG_DEBUG("Bitrates min(%u) = %u", bitrate_, capped_bitrate); remb_packet_.setREMBBitRate(capped_bitrate); remb_packet_.setREMBNumSSRC(source_ssrcs_.size()); @@ -265,6 +266,7 @@ void BandwidthEstimationHandler::sendREMBPacket() { void BandwidthEstimationHandler::OnReceiveBitrateChanged(const std::vector& 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) { diff --git a/erizo/src/erizo/rtp/BandwidthEstimationHandler.h b/erizo/src/erizo/rtp/BandwidthEstimationHandler.h index 65f1c4a074..300ecafeb1 100644 --- a/erizo/src/erizo/rtp/BandwidthEstimationHandler.h +++ b/erizo/src/erizo/rtp/BandwidthEstimationHandler.h @@ -82,7 +82,6 @@ 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 source_ssrcs_; diff --git a/erizo/src/test/rtp/BandwidthEstimationHandlerTest.cpp b/erizo/src/test/rtp/BandwidthEstimationHandlerTest.cpp index 1748316cb0..b390abde53 100644 --- a/erizo/src/test/rtp/BandwidthEstimationHandlerTest.cpp +++ b/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(picker); pipeline->addBack(bwe_handler); + pipeline->notifyUpdate(); } std::shared_ptr bwe_handler; @@ -91,23 +94,4 @@ TEST_F(BandwidthEstimationHandlerTest, shouldSendRembPacketWithEstimatedBitrate) EXPECT_CALL(*writer.get(), write(_, _)).With(Args<1>(erizo::RembHasBitrateValue(kArbitraryBitrate))).Times(1); picker->observer_->OnReceiveBitrateChanged(std::vector(), 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(), kArbitraryBitrate); -} +} \ No newline at end of file From eb1f0ac0219c83ae59eb38a0b1563bbf95c677e8 Mon Sep 17 00:00:00 2001 From: Pedro Rodriguez Date: Thu, 17 Jun 2021 15:12:23 +0200 Subject: [PATCH 11/12] Fix lint --- erizo/src/test/rtp/BandwidthEstimationHandlerTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erizo/src/test/rtp/BandwidthEstimationHandlerTest.cpp b/erizo/src/test/rtp/BandwidthEstimationHandlerTest.cpp index b390abde53..02de68bf92 100644 --- a/erizo/src/test/rtp/BandwidthEstimationHandlerTest.cpp +++ b/erizo/src/test/rtp/BandwidthEstimationHandlerTest.cpp @@ -94,4 +94,4 @@ TEST_F(BandwidthEstimationHandlerTest, shouldSendRembPacketWithEstimatedBitrate) EXPECT_CALL(*writer.get(), write(_, _)).With(Args<1>(erizo::RembHasBitrateValue(kArbitraryBitrate))).Times(1); picker->observer_->OnReceiveBitrateChanged(std::vector(), kArbitraryBitrate); -} \ No newline at end of file +} From b3929a165a8ade4c7f7651a32e96cb0162f7cba0 Mon Sep 17 00:00:00 2001 From: Pedro Rodriguez Date: Fri, 18 Jun 2021 14:37:28 +0200 Subject: [PATCH 12/12] Address code review comments --- erizo/src/erizo/MediaStream.h | 2 +- .../erizo/rtp/BandwidthEstimationHandler.cpp | 21 ++++---- .../erizo/rtp/BandwidthEstimationHandler.h | 1 - .../rtp/BandwidthEstimationHandlerTest.cpp | 52 +++++++++++++++++-- erizo/src/test/utils/Mocks.h | 1 + erizo_controller/erizoClient/src/Stream.js | 6 ++- .../src/webrtc-stacks/BaseStack.js | 2 +- .../src/webrtc-stacks/FirefoxStack.js | 1 - 8 files changed, 64 insertions(+), 22 deletions(-) diff --git a/erizo/src/erizo/MediaStream.h b/erizo/src/erizo/MediaStream.h index 3a6b6f9186..04b8c6e6d6 100644 --- a/erizo/src/erizo/MediaStream.h +++ b/erizo/src/erizo/MediaStream.h @@ -184,7 +184,7 @@ class MediaStream: public MediaSink, public MediaSource, public FeedbackSink, bool isPipelineInitialized() { return pipeline_initialized_; } bool isRunning() { return pipeline_initialized_ && sending_; } - bool isReady() { return ready_; } + virtual bool isReady() { return ready_; } Pipeline::Ptr getPipeline() { return pipeline_; } bool isPublisher() { return is_publisher_; } void setBitrateFromMaxQualityLayer(uint64_t bitrate) { bitrate_from_max_quality_layer_ = bitrate; } diff --git a/erizo/src/erizo/rtp/BandwidthEstimationHandler.cpp b/erizo/src/erizo/rtp/BandwidthEstimationHandler.cpp index 9996a896a6..03b08f074c 100644 --- a/erizo/src/erizo/rtp/BandwidthEstimationHandler.cpp +++ b/erizo/src/erizo/rtp/BandwidthEstimationHandler.cpp @@ -45,7 +45,7 @@ BandwidthEstimationHandler::BandwidthEstimationHandler(std::shared_ptrgetPipelineShared(); if (pipeline && !connection_) { connection_ = pipeline->getService().get(); @@ -223,27 +222,28 @@ void BandwidthEstimationHandler::pickEstimator() { } void BandwidthEstimationHandler::sendREMBPacket() { - sink_ssrc_ = 0; + uint32_t sink_ssrc = 0; source_ssrcs_.clear(); ELOG_DEBUG("Update MediaStream SSRCs"); - connection_->forEachMediaStream([this] (const std::shared_ptr &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()); + connection_->forEachMediaStream([this, &sink_ssrc] (const std::shared_ptr &media_stream) { + ELOG_DEBUG("MediaStream %s, publisher %u, sink %u, source %u, isReady %d", media_stream->getId().c_str(), + media_stream->isPublisher(), media_stream->getVideoSinkSSRC(), media_stream->getVideoSourceSSRC(), + media_stream->isReady()); if (media_stream->isReady() && media_stream->isPublisher()) { - sink_ssrc_ = media_stream->getVideoSinkSSRC(); + sink_ssrc = media_stream->getVideoSinkSSRC(); } source_ssrcs_.push_back(media_stream->getVideoSourceSSRC()); }); - if (sink_ssrc_ == 0) { - ELOG_WARN("No SSRC available to send REMB"); + if (sink_ssrc == 0) { + ELOG_DEBUG("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(sink_ssrc_); + remb_packet_.setSSRC(sink_ssrc); remb_packet_.setSourceSSRC(0); remb_packet_.setLength(4 + source_ssrcs_.size()); uint32_t capped_bitrate = bitrate_; @@ -266,7 +266,6 @@ void BandwidthEstimationHandler::sendREMBPacket() { void BandwidthEstimationHandler::OnReceiveBitrateChanged(const std::vector& 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) { diff --git a/erizo/src/erizo/rtp/BandwidthEstimationHandler.h b/erizo/src/erizo/rtp/BandwidthEstimationHandler.h index 300ecafeb1..ca616b2304 100644 --- a/erizo/src/erizo/rtp/BandwidthEstimationHandler.h +++ b/erizo/src/erizo/rtp/BandwidthEstimationHandler.h @@ -83,7 +83,6 @@ class BandwidthEstimationHandler: public Handler, public RemoteBitrateObserver, uint32_t bitrate_; uint32_t last_send_bitrate_; uint64_t last_remb_time_; - uint32_t sink_ssrc_; std::vector source_ssrcs_; bool running_; bool active_; diff --git a/erizo/src/test/rtp/BandwidthEstimationHandlerTest.cpp b/erizo/src/test/rtp/BandwidthEstimationHandlerTest.cpp index 02de68bf92..6cb989190b 100644 --- a/erizo/src/test/rtp/BandwidthEstimationHandlerTest.cpp +++ b/erizo/src/test/rtp/BandwidthEstimationHandlerTest.cpp @@ -20,6 +20,7 @@ using ::testing::_; using ::testing::IsNull; using ::testing::Args; using ::testing::Return; +using ::testing::AtLeast; using erizo::DataPacket; using erizo::packetType; using erizo::AUDIO_PACKET; @@ -46,17 +47,37 @@ class BandwidthEstimationHandlerTest : public erizo::HandlerTest { picker = std::make_shared(); EXPECT_CALL(*picker.get(), pickEstimatorProxy(_, _, _)) .WillRepeatedly(Return(new erizo::RemoteBitrateEstimatorProxy(&estimator))); - - connection->addMediaStream(media_stream); - bwe_handler = std::make_shared(picker); pipeline->addBack(bwe_handler); - pipeline->notifyUpdate(); + } + + void afterPipelineSetup() { + } + + std::shared_ptr addMediaStreamToConnection(std::string id, + bool is_publisher, bool ready) { + auto media_stream = + std::make_shared(simulated_worker, connection, id, id, rtp_maps, is_publisher); + std::shared_ptr stream_ptr = std::dynamic_pointer_cast(media_stream); + connection->addMediaStream(stream_ptr); + simulated_worker->executeTasks(); + EXPECT_CALL(*media_stream.get(), isReady()).WillRepeatedly(Return(ready)); + streams.push_back(media_stream); + return media_stream; + } + + void internalTearDown() { + std::for_each(streams.begin(), streams.end(), + [this](const std::shared_ptr &stream) { + connection->removeMediaStream(stream->getId()); + }); + simulated_worker->executeTasks(); } std::shared_ptr bwe_handler; std::shared_ptr picker; erizo::MockRemoteBitrateEstimator estimator; + std::vector> streams; }; TEST_F(BandwidthEstimationHandlerTest, basicBehaviourShouldWritePackets) { @@ -81,8 +102,11 @@ TEST_F(BandwidthEstimationHandlerTest, basicBehaviourShouldReadPackets) { pipeline->read(packet2); } -TEST_F(BandwidthEstimationHandlerTest, shouldSendRembPacketWithEstimatedBitrate) { +TEST_F(BandwidthEstimationHandlerTest, shouldSendRembPacketWithEstimatedBitrateIfThereIsAPublisherReady) { uint32_t kArbitraryBitrate = 100000; + + addMediaStreamToConnection("test1", true, true); + auto packet = erizo::PacketTools::createDataPacket(erizo::kArbitrarySeqNumber, VIDEO_PACKET); EXPECT_CALL(estimator, Process()); @@ -95,3 +119,21 @@ TEST_F(BandwidthEstimationHandlerTest, shouldSendRembPacketWithEstimatedBitrate) EXPECT_CALL(*writer.get(), write(_, _)).With(Args<1>(erizo::RembHasBitrateValue(kArbitraryBitrate))).Times(1); picker->observer_->OnReceiveBitrateChanged(std::vector(), kArbitraryBitrate); } + +TEST_F(BandwidthEstimationHandlerTest, shouldNotSendRembPacketWithEstimatedBitrateIfThePublisherIsNotReady) { + uint32_t kArbitraryBitrate = 100000; + + addMediaStreamToConnection("test1", true, false); + + 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); + pipeline->read(packet); + + EXPECT_CALL(*writer.get(), write(_, _)).With(Args<1>(erizo::RembHasBitrateValue(kArbitraryBitrate))).Times(0); + picker->observer_->OnReceiveBitrateChanged(std::vector(), kArbitraryBitrate); +} diff --git a/erizo/src/test/utils/Mocks.h b/erizo/src/test/utils/Mocks.h index a16f8afebc..8dcfb7c0e8 100644 --- a/erizo/src/test/utils/Mocks.h +++ b/erizo/src/test/utils/Mocks.h @@ -117,6 +117,7 @@ class MockMediaStream: public MediaStream { MOCK_METHOD0(getBitrateFromMaxQualityLayer, uint32_t()); MOCK_METHOD0(isSlideShowModeEnabled, bool()); MOCK_METHOD0(isSimulcast, bool()); + MOCK_METHOD0(isReady, bool()); MOCK_METHOD2(onTransportData, void(std::shared_ptr, Transport*)); MOCK_METHOD1(deliverEventInternal, void(MediaEventPtr)); MOCK_METHOD0(getTargetPaddingBitrate, uint64_t()); diff --git a/erizo_controller/erizoClient/src/Stream.js b/erizo_controller/erizoClient/src/Stream.js index 64ea1af503..7dde97c23c 100644 --- a/erizo_controller/erizoClient/src/Stream.js +++ b/erizo_controller/erizoClient/src/Stream.js @@ -79,6 +79,8 @@ const Stream = (altConnectionHelpers, specInput) => { const setMaxVideoBW = (maxVideoBW) => { if (that.local) { + // Estimate codec bitrate from connection (with overhead) bitrate - source https://datatracker.ietf.org/doc/html/rfc8829 + // using 0.90 instead of 0.95 to allow more margin to our quality selection algorithms const translated = (maxVideoBW * 1000 * 0.90) - (50 * 40 * 8); log.info(`message: Setting maxVideoBW, streamId: ${that.getID()}, maxVideoBW: ${maxVideoBW}, translated: ${translated}`); that.maxVideoBW = translated; @@ -131,13 +133,13 @@ const Stream = (altConnectionHelpers, specInput) => { videoSenderLicodeParameters[index] = {}; } if (that.maxVideoBW) { - log.warning('Setting maxVideoBW', that.maxVideoBW); + log.debug('Setting maxVideoBW', that.maxVideoBW); videoSenderLicodeParameters[layersToConfigure - 1].maxBitrate = that.maxVideoBW; } }; const configureVideoStream = (options) => { - log.warning('configureVideoStream', options); + log.debug('configureVideoStream', options); limitMaxAudioBW = options.limitMaxAudioBW; limitMaxVideoBW = options.limitMaxVideoBW; if (options.maxVideoBW) { diff --git a/erizo_controller/erizoClient/src/webrtc-stacks/BaseStack.js b/erizo_controller/erizoClient/src/webrtc-stacks/BaseStack.js index 09749e8673..f16dae11d9 100644 --- a/erizo_controller/erizoClient/src/webrtc-stacks/BaseStack.js +++ b/erizo_controller/erizoClient/src/webrtc-stacks/BaseStack.js @@ -14,7 +14,7 @@ const BaseStack = (specInput) => { }; that.getNegotiationLogs = () => logs.reduce((a, b) => `${a}'\n'${b}`); - log.warning(`message: Starting Base stack, spec: ${JSON.stringify(specBase)}`); + log.debug(`message: Starting Base stack, spec: ${JSON.stringify(specBase)}`); that.pcConfig = { iceServers: [], diff --git a/erizo_controller/erizoClient/src/webrtc-stacks/FirefoxStack.js b/erizo_controller/erizoClient/src/webrtc-stacks/FirefoxStack.js index 941c7d4d57..14f2e60336 100644 --- a/erizo_controller/erizoClient/src/webrtc-stacks/FirefoxStack.js +++ b/erizo_controller/erizoClient/src/webrtc-stacks/FirefoxStack.js @@ -23,7 +23,6 @@ const FirefoxStack = (specInput) => { nativeStream.transceivers.push(transceiver); const parameters = transceiver.sender.getParameters() || {}; parameters.encodings = streamInput.generateEncoderParameters(); - console.warn('parameters to set', parameters); return transceiver.sender.setParameters(parameters); }); };