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

Add PacingHandler #1153

Merged
merged 4 commits into from
Apr 26, 2024
Merged

Conversation

Sean-Der
Copy link
Contributor

@Sean-Der Sean-Der commented Apr 7, 2024

MediaHandler that can be used to pace packet delivery

@Sean-Der Sean-Der force-pushed the pacing-handler branch 5 times, most recently from cc8fc4f to c0be1f1 Compare April 9, 2024 18:36
@Sean-Der
Copy link
Contributor Author

Sean-Der commented Apr 9, 2024

Screenshot_2024-04-09_at_14 22 38

Screenshot_2024-04-09_at_14 21 55

These are two screeshots showing the difference that pacing causes. Without it I see much higher peaks on outbound bandwidth

src/pacinghandler.cpp Outdated Show resolved Hide resolved
src/pacinghandler.cpp Outdated Show resolved Hide resolved
src/pacinghandler.cpp Outdated Show resolved Hide resolved
include/rtc/pacinghandler.hpp Outdated Show resolved Hide resolved
include/rtc/pacinghandler.hpp Outdated Show resolved Hide resolved
src/pacinghandler.cpp Outdated Show resolved Hide resolved
src/pacinghandler.cpp Outdated Show resolved Hide resolved
include/rtc/pacinghandler.hpp Outdated Show resolved Hide resolved
include/rtc/pacinghandler.hpp Outdated Show resolved Hide resolved
@Sean-Der Sean-Der force-pushed the pacing-handler branch 3 times, most recently from ffc534c to 1d553f6 Compare April 17, 2024 18:48
@Sean-Der
Copy link
Contributor Author

@paullouisageneau I have addressed everything in the review! Mind giving it a look, thank you

include/rtc/pacinghandler.hpp Outdated Show resolved Hide resolved
src/pacinghandler.cpp Outdated Show resolved Hide resolved
src/pacinghandler.cpp Outdated Show resolved Hide resolved
@Sean-Der
Copy link
Contributor Author

@paullouisageneau can I get another review please, thank you!

Copy link
Owner

@paullouisageneau paullouisageneau left a comment

Choose a reason for hiding this comment

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

Last pass hopefully, it looks almost good!

src/impl/track.cpp Outdated Show resolved Hide resolved
src/pacinghandler.cpp Outdated Show resolved Hide resolved
src/pacinghandler.cpp Outdated Show resolved Hide resolved
src/pacinghandler.cpp Outdated Show resolved Hide resolved
src/pacinghandler.cpp Outdated Show resolved Hide resolved
include/rtc/pacinghandler.hpp Outdated Show resolved Hide resolved
src/pacinghandler.cpp Outdated Show resolved Hide resolved
MediaHandler that can be used to pace packet delivery

Resolves paullouisageneau#1017

Co-authored-by: Paul-Louis Ageneau <[email protected]>
@Sean-Der
Copy link
Contributor Author

@paullouisageneau Address everything, mind giving this a look! Fingers crossed :)

@Sean-Der
Copy link
Contributor Author

@paullouisageneau Build failures I believe unrelated to the PR

Copy link
Owner

@paullouisageneau paullouisageneau left a comment

Choose a reason for hiding this comment

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

Looking good now, thanks! The build failures are indeed unrelated, the macOS build hits a deprecation warning in libjuice.

@paullouisageneau paullouisageneau merged commit f61fe88 into paullouisageneau:master Apr 26, 2024
12 checks passed
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