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

Add "max" option for strategy #1713

Merged
merged 8 commits into from
May 24, 2021
Merged

Add "max" option for strategy #1713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
196794f
Adjust bandwidth estimation stats for distributors
lodoyun May 18, 2021
86c2ccd
Fix lint
lodoyun May 18, 2021
6849e2c
Remove unused variable
lodoyun May 18, 2021
58fe684
Implement max level in strategy
lodoyun May 19, 2021
acbf53a
Merge branch 'master' into add/maxOptionForStrategy
lodoyun May 19, 2021
9d678f0
Implement max option
lodoyun May 21, 2021
bb7607d
remove uneeded log
lodoyun May 21, 2021
e6daca5
Add default value when there is no max
lodoyun May 24, 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
23 changes: 21 additions & 2 deletions erizo/src/erizo/MediaStream.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ MediaStream::MediaStream(std::shared_ptr<Worker> worker,
audio_muted_{false}, video_muted_{false},
pipeline_initialized_{false},
is_publisher_{is_publisher},
target_is_max_video_bw_{false},
simulcast_{false},
bitrate_from_max_quality_layer_{0},
video_bitrate_{0},
Expand Down Expand Up @@ -131,6 +132,11 @@ void MediaStream::setMaxVideoBW(uint32_t max_video_bw) {
}
});
}
void MediaStream::cleanPriorityState() {
enableSlideShowBelowSpatialLayer(false, 0);
enableFallbackBelowMinLayer(false);
setTargetIsMaxVideoBW(false);
}

void MediaStream::setPriority(const std::string& priority) {
boost::mutex::scoped_lock lock(priority_mutex_);
Expand All @@ -139,8 +145,7 @@ void MediaStream::setPriority(const std::string& priority) {
}
ELOG_INFO("%s message: setting Priority to %s", toLog(), priority.c_str());
priority_ = priority;
enableSlideShowBelowSpatialLayer(false, 0);
enableFallbackBelowMinLayer(false);
cleanPriorityState();
asyncTask([priority] (std::shared_ptr<MediaStream> media_stream) {
media_stream->stats_->getNode()[media_stream->getVideoSinkSSRC()].insertStat(
"streamPriority",
Expand Down Expand Up @@ -719,6 +724,14 @@ void MediaStream::setSlideShowMode(bool state) {
notifyUpdateToHandlers();
}

void MediaStream::setTargetIsMaxVideoBW(bool state) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider adding a TODO here to remove this new state once we have a connection maximum bandwidth limit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think we would necessarily have to remove this, I think it adds a different option that is helpful when you have connection maximum bandwidth limit

ELOG_DEBUG("%s targetIsMaxVideoBw: %u", toLog(), state);
if (target_is_max_video_bw_ == state) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need this check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, we are going to be calling this every time the bw estimation algorithm runs. It's true that, in this particular case, it's an atomic and we're not doing anything else like updating stats. As a result, I'm not sure if this is actually an optimization vs updating the value every time. I somewhat like the pattern but I can be convinced to remove it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I doubt that this optimization worths making the function not threadsafe.

return;
}
target_is_max_video_bw_ = state;
}

void MediaStream::setTargetPaddingBitrate(uint64_t target_padding_bitrate) {
target_padding_bitrate_ = target_padding_bitrate;
notifyUpdateToHandlers();
Expand All @@ -732,6 +745,12 @@ uint32_t MediaStream::getTargetVideoBitrate() {
uint32_t bitrate_from_max_quality_layer = getBitrateFromMaxQualityLayer();

uint32_t target_bitrate = max_bitrate;

if (target_is_max_video_bw_) {
Copy link
Contributor

Choose a reason for hiding this comment

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

could max_bitrate be 0 because it has not been initialized? Maybe we should return kInitialBitrate in that case, as we do below.

target_bitrate = target_bitrate == 0? kInitialBitrate: target_bitrate;
return target_bitrate;
}

if (is_simulcast) {
target_bitrate = std::min(bitrate_from_max_quality_layer, max_bitrate);
}
Expand Down
4 changes: 4 additions & 0 deletions erizo/src/erizo/MediaStream.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ class MediaStream: public MediaSink, public MediaSource, public FeedbackSink,
virtual uint32_t getBitrateFromMaxQualityLayer() { return bitrate_from_max_quality_layer_; }
virtual uint32_t getVideoBitrate() { return video_bitrate_; }
void setPriority(const std::string& priority);
void cleanPriorityState();
std::string getPriority();
void setVideoBitrate(uint32_t bitrate) { video_bitrate_ = bitrate; }
void setMaxVideoBW(uint32_t max_video_bw);
Expand Down Expand Up @@ -133,6 +134,8 @@ class MediaStream: public MediaSink, public MediaSource, public FeedbackSink,
void muteStream(bool mute_video, bool mute_audio);
void setVideoConstraints(int max_video_width, int max_video_height, int max_video_frame_rate);

void setTargetIsMaxVideoBW(bool state);

void setMetadata(std::map<std::string, std::string> metadata);

void read(std::shared_ptr<DataPacket> packet);
Expand Down Expand Up @@ -249,6 +252,7 @@ class MediaStream: public MediaSink, public MediaSource, public FeedbackSink,

bool is_publisher_;

std::atomic_bool target_is_max_video_bw_;
std::atomic_bool simulcast_;
std::atomic<uint64_t> bitrate_from_max_quality_layer_;
std::atomic<uint32_t> video_bitrate_;
Expand Down
3 changes: 1 addition & 2 deletions erizo/src/erizo/WebRtcConnection.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -877,8 +877,7 @@ void WebRtcConnection::setBwDistributionConfigSync(BwDistributionConfig distribu
ELOG_INFO("%s message: setting distribution config type %u", toLog(), distribution_config.selected_distributor);
bw_distribution_config_ = distribution_config;
forEachMediaStream([] (const std::shared_ptr<MediaStream> &media_stream) {
media_stream->enableSlideShowBelowSpatialLayer(false, 0);
media_stream->enableFallbackBelowMinLayer(false);
media_stream->cleanPriorityState();
});

switch (distribution_config.selected_distributor) {
Expand Down
5 changes: 4 additions & 1 deletion erizo/src/erizo/bandwidth/BwDistributionConfig.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,15 @@ bool StreamPriorityStep::isLevelFallback() {
bool StreamPriorityStep::isLevelSlideshow() {
return level == "slideshow";
}
bool StreamPriorityStep::isLevelMax() {
return level == "max";
}
bool StreamPriorityStep::isValid() {
return level != "invalid";
}

int StreamPriorityStep::getSpatialLayer() {
if (isLevelSlideshow() || isLevelFallback() || !isValid()) {
if (isLevelSlideshow() || isLevelFallback() || isLevelMax() || !isValid()) {
return -1;
} else {
return std::stoi(level);
Expand Down
1 change: 1 addition & 0 deletions erizo/src/erizo/bandwidth/BwDistributionConfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class StreamPriorityStep {

bool isLevelFallback();
bool isLevelSlideshow();
bool isLevelMax();
bool isValid();
int getSpatialLayer();
};
Expand Down
11 changes: 8 additions & 3 deletions erizo/src/erizo/bandwidth/StreamPriorityBWDistributor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,11 @@ void StreamPriorityBWDistributor::distribute(uint32_t remb, uint32_t ssrc,
ELOG_DEBUG("No more bitrate to distribute");
break;
}

bool is_max = step.isLevelMax();
int layer = step.getSpatialLayer();
ELOG_DEBUG("Step with priority %s, layer %u, remaining %lu, bitrate assigned to priority %lu",
priority.c_str(), layer, remaining_bitrate, bitrate_for_priority[priority]);
ELOG_DEBUG("Step with priority %s, layer %d, is_max %u remaining %lu, bitrate assigned to priority %lu",
priority.c_str(), layer, is_max, remaining_bitrate, bitrate_for_priority[priority]);
// bitrate_for_priority is automatically initialized to 0 with the first [] call to the map
remaining_bitrate += bitrate_for_priority[priority];
bitrate_for_priority[priority] = 0;
Expand All @@ -71,7 +73,10 @@ void StreamPriorityBWDistributor::distribute(uint32_t remb, uint32_t ssrc,
}
for (MediaStreamPriorityInfo& stream_info : stream_infos[priority]) {
uint64_t needed_bitrate_for_stream = 0;
if (!stream_info.stream->isSimulcast()) {
if (is_max) {
stream_info.stream->setTargetIsMaxVideoBW(true);
needed_bitrate_for_stream = stream_info.stream->getMaxVideoBW();
} else if (!stream_info.stream->isSimulcast()) {
ELOG_DEBUG("Stream %s is not simulcast", stream_info.stream->getId());
int number_of_layers = strategy_.getHighestLayerForPriority(priority) + 1;
if (number_of_layers > 0) {
Expand Down
57 changes: 56 additions & 1 deletion erizo/src/test/bandwidth/StreamPriorityBWDistributor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,4 +392,59 @@ INSTANTIATE_TEST_CASE_P(
StreamPriorityStep("20", "1"),
StreamPriorityStep("20", "2")
},
100, EnabledList{1, 1, 1, 1}, ExpectedList{50, 50, 0, 0})));
100, EnabledList{1, 1, 1, 1}, ExpectedList{50, 50, 0, 0}),
make_tuple(StreamConfigList{
{1000, 0, 450, "0", std::vector<std::vector<uint64_t>>({ { 100, 150, 200 }, { 250, 300, 450} }), false, true},
{1000, 0, 450, "20", std::vector<std::vector<uint64_t>>({ { 100, 150, 200 }, { 250, 300, 450} }), false, true}
},
StrategyVector{
StreamPriorityStep("20", "0"),
StreamPriorityStep("10", "0"),
StreamPriorityStep("0", "max"),
StreamPriorityStep("20", "1"),
StreamPriorityStep("20", "2")
},
1500, EnabledList{1, 1}, ExpectedList{
1000,
static_cast<uint32_t>(450 * (1 + QualityManager::kIncreaseLayerBitrateThreshold))}),
make_tuple(StreamConfigList{
{1000, 0, 450, "20", std::vector<std::vector<uint64_t>>({ { 100, 150, 200 }, { 250, 300, 450} }), false, true},
{1000, 0, 450, "0", std::vector<std::vector<uint64_t>>({ { 100, 150, 200 }, { 250, 300, 450} }), false, true}
},
StrategyVector{
StreamPriorityStep("20", "max"),
StreamPriorityStep("10", "0"),
StreamPriorityStep("0", "0"),
},
1500, EnabledList{1, 1}, ExpectedList{
1000,
static_cast<uint32_t>(200 * (1 + QualityManager::kIncreaseLayerBitrateThreshold))}),
make_tuple(StreamConfigList{
{1000, 0, 600, "20", std::vector<std::vector<uint64_t>>({ { 100, 150, 200 }, { 250, 300, 450} }), false, true},
{1000, 0, 600, "20", std::vector<std::vector<uint64_t>>({ { 100, 150, 200 }, { 250, 300, 450} }), false, true}
},
StrategyVector{
StreamPriorityStep("20", "max"),
StreamPriorityStep("10", "0"),
StreamPriorityStep("0", "0"),
},
1500, EnabledList{1, 1}, ExpectedList{
750,
750}),
make_tuple(StreamConfigList{
{1000, 0, 600, "20", std::vector<std::vector<uint64_t>>({ { 100, 150, 200 }, { 250, 300, 450} }), false, true},
{1000, 0, 600, "20", std::vector<std::vector<uint64_t>>({ { 100, 150, 200 }, { 250, 300, 450} }), false, true},
{1000, 0, 450, "10", std::vector<std::vector<uint64_t>>({ { 100, 150, 200 }, { 250, 300, 450} }), false, true},
{1000, 0, 450, "10", std::vector<std::vector<uint64_t>>({ { 100, 150, 200 }, { 250, 300, 450} }), false, true}
},
StrategyVector{
StreamPriorityStep("20", "max"),
StreamPriorityStep("10", "0"),
StreamPriorityStep("10", "1"),
},
2500, EnabledList{1, 1}, ExpectedList{
1000,
1000,
static_cast<uint32_t>(200 * (1 + QualityManager::kIncreaseLayerBitrateThreshold)),
static_cast<uint32_t>(200 * (1 + QualityManager::kIncreaseLayerBitrateThreshold))})
));
17 changes: 15 additions & 2 deletions erizo_controller/erizoJS/models/Publisher.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class Source extends NodeClass {
this.muteAudio = false;
this.muteVideo = false;
this.muxer = new addon.OneToManyProcessor();
this.maxVideoBW = false;
}

get numSubscribers() {
Expand Down Expand Up @@ -172,7 +173,7 @@ class Source extends NodeClass {
this.muteStream(msg.config.muteStream);
}
if (msg.config.maxVideoBW) {
this.mediaStream.setMaxVideoBW(msg.config.maxVideoBW);
this.setMaxVideoBW(msg.config.maxVideoBW);
}
}
} else if (msg.type === 'control') {
Expand Down Expand Up @@ -404,7 +405,7 @@ class Publisher extends Source {
this.scheme = options.scheme;

if (options.maxVideoBW) {
this.mediaStream.setMaxVideoBW(options.maxVideoBW);
this.setMaxVideoBW(options.maxVideoBW);
}

this.mediaStream.setAudioReceiver(this.muxer);
Expand All @@ -415,6 +416,18 @@ class Publisher extends Source {
this.muteStream({ video: muteVideo, audio: muteAudio });
}

getMaxVideoBW() {
return this.maxVideoBW;
}

setMaxVideoBW(maxVideoBW) {
this.maxVideoBW = maxVideoBW;
this.mediaStream.setMaxVideoBW(maxVideoBW);
this.forEachSubscriber((id, subscriber) => {
subscriber.updatePublisherMaxVideoBW();
});
}

getDurationDistribution() {
if (!this.mediaStream) {
return [];
Expand Down
23 changes: 22 additions & 1 deletion erizo_controller/erizoJS/models/Subscriber.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class Subscriber extends NodeClass {
});
this.mediaStream = connection.getMediaStream(this.erizoStreamId);
this.publisher = publisher;
this.setMaxVideoBW();
}

copySdpInfoFromPublisher() {
Expand All @@ -37,6 +38,26 @@ class Subscriber extends NodeClass {
}
}

updatePublisherMaxVideoBW() {
this.setMaxVideoBW();
}

setMaxVideoBW(maxVideoBW) {
let updatedMaxVideoBW;
if (maxVideoBW) {
this.maxVideoBW = maxVideoBW;
}
if (this.maxVideoBW) {
updatedMaxVideoBW = this.publisher.getMaxVideoBW() ?
Math.min(this.publisher.getMaxVideoBW(), this.maxVideoBW) : this.maxVideoBW;
this.maxVideoBW = updatedMaxVideoBW;
} else {
updatedMaxVideoBW = this.publisher.getMaxVideoBW();
}
log.debug(`Setting maxVideoBW in subscriber, requested: ${maxVideoBW}, publisher ${this.publisher.getMaxVideoBW()}, result:${updatedMaxVideoBW}`);
this.mediaStream.setMaxVideoBW(updatedMaxVideoBW);
}

_onMediaStreamEvent(mediaStreamEvent) {
if (mediaStreamEvent.mediaStreamId !== this.erizoStreamId) {
return;
Expand Down Expand Up @@ -79,7 +100,7 @@ class Subscriber extends NodeClass {
this.publisher.setVideoConstraints(msg.config.video, this.clientId);
}
if (msg.config.maxVideoBW) {
this.mediaStream.setMaxVideoBW(msg.config.maxVideoBW);
this.setMaxVideoBW(msg.config.maxVideoBW);
}
if (msg.config.priorityLevel !== undefined) {
this.mediaStream.setPriority(msg.config.priorityLevel);
Expand Down