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

Implement Pli Priority Handler #1480

Merged
merged 3 commits into from
Oct 18, 2019
Merged

Conversation

lodoyun
Copy link
Contributor

@lodoyun lodoyun commented Oct 17, 2019

Description

This PR implements a handler that throttles PLI requests based on priority.
All PLI packets are marked with high or low priority. High priority PLIs go directly through the handler but low priority ones are throttled so only 1 is sent every 3 seconds.
Currently the only PLI packets that are marked as low priority are those requested when switching to a higher layer.

[] It needs and includes Unit Tests

Changes in Client or Server public APIs

[] It includes documentation for these changes in /doc.

Copy link
Contributor

@jcague jcague left a comment

Choose a reason for hiding this comment

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

LGTM mod comments. I'm wondering if we should optimize it a little bit with the two things I comment in the PR.

erizo/src/erizo/rtp/PliPriorityHandler.cpp Outdated Show resolved Hide resolved
if (this_ptr->plis_received_in_interval_ > 0) {
ELOG_DEBUG("%s, message: Sending Low priority PLI, plis_received_in_interval_: %u",
this_ptr->stream_->toLog(), this_ptr->plis_received_in_interval_);
this_ptr->plis_received_in_interval_ = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

does it make sense to also set first_received_ to false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thats a possible optimization, I would then return false to stop this scheduled Task and have it start again next time a Low priority pli is received... However, I prefer adding a comment for it as a possible improvement in the near future and start testing the functionality.

erizo_controller/erizoAgent/log4cxx.properties Outdated Show resolved Hide resolved
erizo/src/erizo/rtp/PliPriorityHandler.cpp Show resolved Hide resolved
erizo/src/erizo/rtp/QualityFilterHandler.cpp Show resolved Hide resolved
@lodoyun lodoyun merged commit 59c19c9 into lynckia:master Oct 18, 2019
Arri98 pushed a commit to Arri98/licode that referenced this pull request Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants