Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop padding if the distributor does not assign all the available #1696

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion erizo/src/erizo/WebRtcConnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -890,7 +890,7 @@ void WebRtcConnection::setBwDistributionConfigSync(BwDistributionConfig distribu
break;
case STREAM_PRIORITY:
distributor_ = std::unique_ptr<BandwidthDistributionAlgorithm>(
new StreamPriorityBWDistributor(distribution_config.priority_strategy));
new StreamPriorityBWDistributor(distribution_config.priority_strategy, stats_));
break;
}
}
Expand Down
4 changes: 3 additions & 1 deletion erizo/src/erizo/bandwidth/StreamPriorityBWDistributor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
namespace erizo {
DEFINE_LOGGER(StreamPriorityBWDistributor, "bandwidth.StreamPriorityBWDistributor");

StreamPriorityBWDistributor::StreamPriorityBWDistributor(StreamPriorityStrategy strategy): strategy_{strategy} {
StreamPriorityBWDistributor::StreamPriorityBWDistributor(StreamPriorityStrategy strategy,
std::shared_ptr<Stats>stats): strategy_{strategy}, stats_{stats} {
}

std::string StreamPriorityBWDistributor::getStrategyId() {
Expand Down Expand Up @@ -99,6 +100,7 @@ void StreamPriorityBWDistributor::distribute(uint32_t remb, uint32_t ssrc,
remb, stream_info.stream->getId().c_str(), remaining_bitrate);
}
}
stats_->getNode()["total"].insertStat("unnasignedBitrate", CumulativeStat{remaining_bitrate});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

for (const auto& kv_pair : stream_infos) {
for (MediaStreamPriorityInfo& stream_info : stream_infos[kv_pair.first]) {
auto generated_remb = RtpUtils::createREMB(ssrc,
Expand Down
4 changes: 3 additions & 1 deletion erizo/src/erizo/bandwidth/StreamPriorityBWDistributor.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#define ERIZO_SRC_ERIZO_BANDWIDTH_STREAMPRIORITYBWDISTRIBUTOR_H_

#include "./logger.h"
#include "./Stats.h"
#include "bandwidth/BwDistributionConfig.h"
#include "bandwidth/BandwidthDistributionAlgorithm.h"

Expand All @@ -18,14 +19,15 @@ class MediaStreamPriorityInfo {
class StreamPriorityBWDistributor : public BandwidthDistributionAlgorithm {
DECLARE_LOGGER();
public:
explicit StreamPriorityBWDistributor(StreamPriorityStrategy strategy);
explicit StreamPriorityBWDistributor(StreamPriorityStrategy strategy, std::shared_ptr<Stats> stats);
virtual ~StreamPriorityBWDistributor() {}
void distribute(uint32_t remb, uint32_t ssrc, std::vector<std::shared_ptr<MediaStream>> streams,
Transport *transport) override;
std::string getStrategyId();

private:
StreamPriorityStrategy strategy_;
std::shared_ptr<Stats> stats_;
};

} // namespace erizo
Expand Down
2 changes: 1 addition & 1 deletion erizo/src/erizo/rtp/QualityManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ void QualityManager::forceLayers(int spatial_layer, int temporal_layer) {
}

void QualityManager::enableSlideShowBelowSpatialLayer(bool enable, int spatial_layer) {
if (enable_fallback_below_min_layer_ == enable && slideshow_below_spatial_layer_ == spatial_layer_) {
if (enable_fallback_below_min_layer_ == enable && slideshow_below_spatial_layer_ == spatial_layer) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch! ;)

return;
}
ELOG_DEBUG("message: enableSlideShowBelowSpatialLayer, enable %d, spatial_layer: %d", enable, spatial_layer);
Expand Down
7 changes: 6 additions & 1 deletion erizo/src/erizo/rtp/RtpPaddingManagerHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ static constexpr duration kStatsPeriod = std::chrono::milliseconds(100);
static constexpr duration kMinDurationToSendPaddingAfterPacketLosses = std::chrono::seconds(180);
static constexpr double kBitrateComparisonMargin = 1.3;
static constexpr uint64_t kInitialBitrate = 300000;
static constexpr uint64_t kUnnasignedBitrateMargin = 50000;

RtpPaddingManagerHandler::RtpPaddingManagerHandler(std::shared_ptr<erizo::Clock> the_clock) :
initialized_{false},
Expand Down Expand Up @@ -112,7 +113,11 @@ void RtpPaddingManagerHandler::recalculatePaddingRate() {

bool can_send_more_bitrate = (kBitrateComparisonMargin * media_bitrate) < estimated_bandwidth;
bool estimated_is_high_enough = estimated_bandwidth > (target_bitrate * kBitrateComparisonMargin);
if (estimated_is_high_enough) {
bool has_unnasigned_bitrate = false;
if (stats_->getNode()["total"].hasChild("unnasignedBitrate")) {
has_unnasigned_bitrate = stats_->getNode()["total"]["unnasignedBitrate"].value() > kUnnasignedBitrateMargin;
}
if (estimated_is_high_enough || has_unnasigned_bitrate) {
target_padding_bitrate = 0;
}

Expand Down
8 changes: 6 additions & 2 deletions erizo/src/test/bandwidth/StreamPriorityBWDistributor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
#include <MediaDefinitions.h>
#include <bandwidth/StreamPriorityBWDistributor.h>
#include <bandwidth/BwDistributionConfig.h>
#include <stats/StatNode.h>
#include <Stats.h>

#include <string>
#include <tuple>
Expand All @@ -26,6 +28,7 @@ using erizo::IceConfig;
using erizo::RtpMap;
using erizo::RtpUtils;
using erizo::MediaStream;
using erizo::Stats;
using erizo::StreamPriorityBWDistributor;
using erizo::StreamPriorityStep;
using erizo::StreamPriorityStrategy;
Expand Down Expand Up @@ -146,6 +149,7 @@ class BasicStreamPriorityBWDistributorTest {
uint32_t index;
std::shared_ptr<StreamPriorityBWDistributor> distributor;
std::queue<std::shared_ptr<DataPacket>> packet_queue;
std::shared_ptr<Stats> stats;
};

class StreamPriorityBWDistributorTest : public BasicStreamPriorityBWDistributorTest,
Expand All @@ -162,10 +166,10 @@ class StreamPriorityBWDistributorTest : public BasicStreamPriorityBWDistributorT
bitrate_value = std::tr1::get<2>(GetParam());
add_to_remb_list = std::tr1::get<3>(GetParam());
expected_bitrates = std::tr1::get<4>(GetParam());

stats = std::make_shared<erizo::Stats>();
setUpStrategy();

distributor = std::make_shared<erizo::StreamPriorityBWDistributor>(strategy);
distributor = std::make_shared<erizo::StreamPriorityBWDistributor>(strategy, stats);

setUpStreams();
}
Expand Down