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.

Sign up for GitHub

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

Make connection quality level depend on strategy #1767

Merged
merged 4 commits into from
Nov 22, 2021
Merged

Make connection quality level depend on strategy #1767

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ccdd7c7
Make connection quality level depend on strategy
jcague Nov 15, 2021
1e1eb6b
Fix wrong level
jcague Nov 15, 2021
938eeaa
Implement changes proposed in the PR
jcague Nov 18, 2021
6de6912
Fix lint
jcague Nov 22, 2021
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
12 changes: 4 additions & 8 deletions erizo/src/erizo/WebRtcConnection.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,11 +312,10 @@ bool WebRtcConnection::createOfferSync(bool bundle) {
}

ConnectionQualityLevel WebRtcConnection::getConnectionQualityLevel() {
return connection_quality_check_.getLevel();
}

bool WebRtcConnection::werePacketLossesRecently() {
return connection_quality_check_.werePacketLossesRecently();
if (distributor_->tooLowBandwidthEstimation()) {
return ConnectionQualityLevel::VERY_LOW;
}
return ConnectionQualityLevel::GOOD;
}

void WebRtcConnection::associateMediaStreamToTransceiver(std::shared_ptr<MediaStream> media_stream,
Expand Down Expand Up @@ -996,9 +995,6 @@ void WebRtcConnection::onREMBFromTransport(RtcpHeader *chead, Transport *transpo
}

void WebRtcConnection::onRtcpFromTransport(std::shared_ptr<DataPacket> packet, Transport *transport) {
if (enable_connection_quality_check_) {
connection_quality_check_.onFeedback(packet, streams_);
}
RtpUtils::forEachRtcpBlock(packet, [this, packet, transport](RtcpHeader *chead) {
uint32_t ssrc = chead->isFeedback() ? chead->getSourceSSRC() : chead->getSSRC();
if (chead->isREMB()) {
Expand Down
9 changes: 6 additions & 3 deletions erizo/src/erizo/WebRtcConnection.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#include "./Transport.h"
#include "./Stats.h"
#include "bandwidth/BandwidthDistributionAlgorithm.h"
#include "bandwidth/ConnectionQualityCheck.h"
#include "bandwidth/BwDistributionConfig.h"
#include "pipeline/Pipeline.h"
#include "thread/Worker.h"
Expand All @@ -40,6 +39,12 @@ class IceConfig;
class MediaStream;
class Transceiver;

enum ConnectionQualityLevel {
VERY_LOW = 0,
LOW = 1,
GOOD = 2
};

/**
* WebRTC Events
*/
Expand Down Expand Up @@ -179,7 +184,6 @@ class WebRtcConnection: public TransportListener, public LogContext, public Hand
void write(std::shared_ptr<DataPacket> packet);
void notifyUpdateToHandlers() override;
ConnectionQualityLevel getConnectionQualityLevel();
bool werePacketLossesRecently();
void getJSONStats(std::function<void(std::string)> callback);

private:
Expand Down Expand Up @@ -238,7 +242,6 @@ class WebRtcConnection: public TransportListener, public LogContext, public Hand

std::unique_ptr<BandwidthDistributionAlgorithm> distributor_;
BwDistributionConfig bw_distribution_config_;
ConnectionQualityCheck connection_quality_check_;
bool enable_connection_quality_check_;
bool encrypt_transport_;
std::atomic <uint32_t> connection_target_bw_;
Expand Down
1 change: 1 addition & 0 deletions erizo/src/erizo/bandwidth/BandwidthDistributionAlgorithm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class BandwidthDistributionAlgorithm {
virtual ~BandwidthDistributionAlgorithm() {}
virtual void distribute(uint32_t remb, uint32_t ssrc, std::vector<std::shared_ptr<MediaStream>> streams,
Transport *transport) = 0;
virtual bool tooLowBandwidthEstimation() = 0;
};

} // namespace erizo
Expand Down
127 changes: 0 additions & 127 deletions erizo/src/erizo/bandwidth/ConnectionQualityCheck.cpp
View file
Open in desktop

This file was deleted.

63 changes: 0 additions & 63 deletions erizo/src/erizo/bandwidth/ConnectionQualityCheck.h
View file
Open in desktop

This file was deleted.

1 change: 1 addition & 0 deletions erizo/src/erizo/bandwidth/MaxVideoBWDistributor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ class MaxVideoBWDistributor : public BandwidthDistributionAlgorithm {
virtual ~MaxVideoBWDistributor() {}
void distribute(uint32_t remb, uint32_t ssrc, std::vector<std::shared_ptr<MediaStream>> streams,
Transport *transport) override;
bool tooLowBandwidthEstimation() override { return false; }
};

} // namespace erizo
Expand Down
14 changes: 13 additions & 1 deletion erizo/src/erizo/bandwidth/StreamPriorityBWDistributor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
namespace erizo {
DEFINE_LOGGER(StreamPriorityBWDistributor, "bandwidth.StreamPriorityBWDistributor");
StreamPriorityBWDistributor::StreamPriorityBWDistributor(StreamPriorityStrategy strategy,
std::shared_ptr<Stats>stats): strategy_{strategy}, stats_{stats} {
std::shared_ptr<Stats>stats): strategy_{strategy}, stats_{stats}, only_using_default_levels_{false} {
}

std::string StreamPriorityBWDistributor::getStrategyId() {
Expand All @@ -32,6 +32,10 @@ void StreamPriorityBWDistributor::distribute(uint32_t remb, uint32_t ssrc,
}
ELOG_DEBUG("Starting distribution with bitrate %lu for strategy %s", remaining_bitrate, getStrategyId().c_str());
strategy_.reset();

bool has_no_default_levels = false;
Copy link
Contributor

@lodoyun lodoyun Nov 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

These variable names are really confusing to me. I think it's because I think of fallback and slideshow as "default levels". An alternative would be has_no_spatial_levels and distribute_bitrate_to_spatial_levels since we always expect temporal layers there anyway.
Other options are probably fine as long as we avoid default as it's not clear what default is in the context.

bool distribute_bitrate_to_no_default_levels = false;

while (strategy_.hasNextStep()) {
StreamPriorityStep step = strategy_.getNextStep();
std::string priority = step.priority;
Expand All @@ -54,11 +58,16 @@ void StreamPriorityBWDistributor::distribute(uint32_t remb, uint32_t ssrc,
}
continue;
}

has_no_default_levels = true;

if (remaining_bitrate == 0) {
ELOG_DEBUG("No more bitrate to distribute");
break;
}

distribute_bitrate_to_no_default_levels = true;

bool is_max = step.isLevelMax();
int layer = step.getSpatialLayer();
ELOG_DEBUG("Step with priority %s, layer %d, is_max %u remaining %lu, bitrate assigned to priority %lu",
Expand Down Expand Up @@ -106,6 +115,9 @@ void StreamPriorityBWDistributor::distribute(uint32_t remb, uint32_t ssrc,
remb, stream_info.stream->getId().c_str(), remaining_bitrate);
}
}

only_using_default_levels_ = has_no_default_levels && !distribute_bitrate_to_no_default_levels;

stats_->getNode()["total"].insertStat("unnasignedBitrate", CumulativeStat{remaining_bitrate});
for (const auto& kv_pair : stream_infos) {
for (MediaStreamPriorityInfo& stream_info : stream_infos[kv_pair.first]) {
Expand Down
3 changes: 3 additions & 0 deletions erizo/src/erizo/bandwidth/StreamPriorityBWDistributor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,12 @@ DECLARE_LOGGER();
Transport *transport) override;
std::string getStrategyId();

bool tooLowBandwidthEstimation() override { return only_using_default_levels_; }

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

} // namespace erizo
Expand Down
1 change: 1 addition & 0 deletions erizo/src/erizo/bandwidth/TargetVideoBWDistributor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class TargetVideoBWDistributor : public BandwidthDistributionAlgorithm {
virtual ~TargetVideoBWDistributor() {}
void distribute(uint32_t remb, uint32_t ssrc, std::vector<std::shared_ptr<MediaStream>> streams,
Transport *transport) override;
bool tooLowBandwidthEstimation() override { return false; }
};

} // namespace erizo
Expand Down
Loading