Commit graph

13 commits

Author SHA1 Message Date
Erik Språng
ae10029bff Prevents probing while paused.
The pacing controller allowed sending bitrate probes, despite it being
paused. This CL adresses that, and makes sure the task-queue based mode
also properly repsects pausing.

Bug: webrtc:10809
Change-Id: I79643c9a24666110d7583fce3ed1bfd6865e9e10
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/162520
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30109}
2019-12-17 17:57:38 +00:00
Erik Språng
b1ccae253e Reland "Fixes dynamic mode pacing issues."
This is a reland of 72e6cb0b3f
Was not the cause of perf alert, relanding.

TBR=ilnik@webrtc.org

Original change's description:
> Fixes dynamic mode pacing issues.
>
> This CL fixes a few issues in the (default-disabled) dynamic pacing
> mode:
> * Slight update to sleep timing to avoid short spin loops
> * Removed support for early execution as that lead to time-travel
>   contradictions that were difficult to solve.
> * Makes sure we schedule a process call when a packet is due to be
>   drained even if the queue is empty, so that padding will start at
>   the correct time.
> * While paused or empty, sleep relative last send time if we send
>   padding while silent - otherwise just relative to last process
>   time.
> * If target send time shifts so far back that packet should have
>   been sent prior to the last process, make sure we don't let the
>   buffer level remain.
> * Update the PacedSender test to _actually_ use dynamic processing
>   when the param says so.
>
> Bug: webrtc:10809
> Change-Id: Iebfde9769647d2390fd192a40bbe2d5bf1f6cc62
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/160407
> Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
> Commit-Queue: Erik Språng <sprang@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#29911}

Bug: webrtc:10809
Change-Id: Ie7b307e574c2057bb05af87b6718a132d639a416
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/160786
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29928}
2019-11-27 10:22:05 +00:00
Erik Språng
3967389d34 Revert "Fixes dynamic mode pacing issues."
This reverts commit 72e6cb0b3f.

Reason for revert: Speculative revert due to perf change

Original change's description:
> Fixes dynamic mode pacing issues.
> 
> This CL fixes a few issues in the (default-disabled) dynamic pacing
> mode:
> * Slight update to sleep timing to avoid short spin loops
> * Removed support for early execution as that lead to time-travel
>   contradictions that were difficult to solve.
> * Makes sure we schedule a process call when a packet is due to be
>   drained even if the queue is empty, so that padding will start at
>   the correct time.
> * While paused or empty, sleep relative last send time if we send
>   padding while silent - otherwise just relative to last process
>   time.
> * If target send time shifts so far back that packet should have
>   been sent prior to the last process, make sure we don't let the
>   buffer level remain.
> * Update the PacedSender test to _actually_ use dynamic processing
>   when the param says so.
> 
> Bug: webrtc:10809
> Change-Id: Iebfde9769647d2390fd192a40bbe2d5bf1f6cc62
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/160407
> Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
> Commit-Queue: Erik Språng <sprang@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#29911}

TBR=ilnik@webrtc.org,sprang@webrtc.org

Change-Id: I5d1532d2e041e60a7f1bfeb8185f7760c9789711
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:10809
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/160701
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29920}
2019-11-26 17:02:39 +00:00
Erik Språng
72e6cb0b3f Fixes dynamic mode pacing issues.
This CL fixes a few issues in the (default-disabled) dynamic pacing
mode:
* Slight update to sleep timing to avoid short spin loops
* Removed support for early execution as that lead to time-travel
  contradictions that were difficult to solve.
* Makes sure we schedule a process call when a packet is due to be
  drained even if the queue is empty, so that padding will start at
  the correct time.
* While paused or empty, sleep relative last send time if we send
  padding while silent - otherwise just relative to last process
  time.
* If target send time shifts so far back that packet should have
  been sent prior to the last process, make sure we don't let the
  buffer level remain.
* Update the PacedSender test to _actually_ use dynamic processing
  when the param says so.

Bug: webrtc:10809
Change-Id: Iebfde9769647d2390fd192a40bbe2d5bf1f6cc62
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/160407
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29911}
2019-11-25 23:11:02 +00:00
Erik Språng
eb48799ec5 Prepares PacingController for scheduled send tasks.
This CL is in preparation for a dynamic (possible TaskQueue-driven)
pacer that instead of processing blindly every 5ms, posts delayed
tasks to be executed when it is actually time to send packs.

This means we need the pacing controller to be able to figure out when
those execution times shall be, and be able to correctly update budget
levels as IntervalBudget only works correctly with periodic processing.

Bug: webrtc:10809
Change-Id: Idd12acaabfb24cc2e6bcc589aac206cd04beb6e4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/158790
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29800}
2019-11-14 13:53:56 +00:00
Erik Språng
22fd5d7455 Fixes incorrect probe timing check.
In a recent CL, a line that puts a lower bound of 0 on time to next
probe was omitted:
https://webrtc-review.googlesource.com/c/src/+/158841/7/modules/pacing/bitrate_prober.cc#b143
That cause a misinterpretation in
https://webrtc-review.googlesource.com/c/src/+/158841/7/modules/pacing/pacing_controller.cc#290
which may lead to probes aborting if the module processing thread
sleeps a little too long.

Bug: webrtc:10809
Change-Id: I672375fb213782e4e1f2215252f50894d7655f97
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/159023
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29728}
2019-11-07 14:00:33 +00:00
Erik Språng
b210eeb812 Reland "Refactors BitrateProber with unit types and absolute probe time."
This is a reland of 739a5b3692

Patchset 1 is the original CL, patchset 3 includes a fix

Original change's description:
> Refactors BitrateProber with unit types and absolute probe time.
>
> Using unit types improves readability and some conversion in PacedSender
> can be removed.
>
> TimeUntilNextProbe() is replaced by NextProbeTime(), so returning an
> absolute time rather than a delta. This fits better with the upcoming
> TaskQueue based pacer, and is also what is already stored internally
> in BitrateProber.
>
> Bug: webrtc:10809
> Change-Id: I5a4e289d2b53e99d3c0a2f4b36a966dba759d5cf
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/158743
> Commit-Queue: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Sebastian Jansson <srte@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#29670}

Bug: webrtc:10809
Change-Id: I033193c78474fdd82c109fdab0a8f09a05f7b30e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/158841
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29688}
2019-11-05 11:02:22 +00:00
Erik Språng
bb56d4b0e2 Revert "Refactors BitrateProber with unit types and absolute probe time."
This reverts commit 739a5b3692.

Reason for revert: Speculate revert due to perf alerts.

Original change's description:
> Refactors BitrateProber with unit types and absolute probe time.
> 
> Using unit types improves readability and some conversion in PacedSender
> can be removed.
> 
> TimeUntilNextProbe() is replaced by NextProbeTime(), so returning an
> absolute time rather than a delta. This fits better with the upcoming
> TaskQueue based pacer, and is also what is already stored internally
> in BitrateProber.
> 
> Bug: webrtc:10809
> Change-Id: I5a4e289d2b53e99d3c0a2f4b36a966dba759d5cf
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/158743
> Commit-Queue: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Sebastian Jansson <srte@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#29670}

TBR=sprang@webrtc.org,srte@webrtc.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: webrtc:10809
Change-Id: Ic0ad7d45031bf33c24583dfde308bdd8087a62aa
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/158799
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29682}
2019-11-04 14:37:03 +00:00
Erik Språng
739a5b3692 Refactors BitrateProber with unit types and absolute probe time.
Using unit types improves readability and some conversion in PacedSender
can be removed.

TimeUntilNextProbe() is replaced by NextProbeTime(), so returning an
absolute time rather than a delta. This fits better with the upcoming
TaskQueue based pacer, and is also what is already stored internally
in BitrateProber.

Bug: webrtc:10809
Change-Id: I5a4e289d2b53e99d3c0a2f4b36a966dba759d5cf
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/158743
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29670}
2019-10-31 15:34:39 +00:00
Erik Språng
78c82a4040 Adds trial to always start probes with a small padding packet.
This will reduce bias caused by uncertainty in averaging window.

Bug: None
Change-Id: I5c4fe39ffe69fb4af87d86995196a54115d3e0b2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/144720
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29374}
2019-10-03 17:19:22 +00:00
Mirko Bonadei
317a1f09ed Use std::make_unique instead of absl::make_unique.
WebRTC is now using C++14 so there is no need to use the Abseil version
of std::make_unique.

This CL has been created with the following steps:

git grep -l absl::make_unique | sort | uniq > /tmp/make_unique.txt
git grep -l absl::WrapUnique | sort | uniq > /tmp/wrap_unique.txt
git grep -l "#include <memory>" | sort | uniq > /tmp/memory.txt

diff --new-line-format="" --unchanged-line-format="" \
  /tmp/make_unique.txt /tmp/wrap_unique.txt | sort | \
  uniq > /tmp/only_make_unique.txt
diff --new-line-format="" --unchanged-line-format="" \
  /tmp/only_make_unique.txt /tmp/memory.txt | \
  xargs grep -l "absl/memory" > /tmp/add-memory.txt

git grep -l "\babsl::make_unique\b" | \
  xargs sed -i "s/\babsl::make_unique\b/std::make_unique/g"

git checkout PRESUBMIT.py abseil-in-webrtc.md

cat /tmp/add-memory.txt | \
  xargs sed -i \
  's/#include "absl\/memory\/memory.h"/#include <memory>/g'
git cl format
# Manual fix order of the new inserted #include <memory>

cat /tmp/only_make_unique | xargs grep -l "#include <memory>" | \
  xargs sed -i '/#include "absl\/memory\/memory.h"/d'

git ls-files | grep BUILD.gn | \
  xargs sed -i '/\/\/third_party\/abseil-cpp\/absl\/memory/d'

python tools_webrtc/gn_check_autofix.py \
  -m tryserver.webrtc -b linux_rel

# Repead the gn_check_autofix step for other platforms

git ls-files | grep BUILD.gn | \
  xargs sed -i 's/absl\/memory:memory/absl\/memory/g'
git cl format

Bug: webrtc:10945
Change-Id: I3fe28ea80f4dd3ba3cf28effd151d5e1f19aff89
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/153221
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29209}
2019-09-17 15:47:29 +00:00
Erik Språng
f5815fa6bb Remove WebRTC-Pacer-LegacyPacketReferencing flag and most usage
This flag has been default-off since Jul 24th (m77 branch) and apart
from a bug fixed on Aug 5th, there have been no reports of issues, so
let's remove it and start cleaning away the old code path.

Most of the usage within RtpSender/PacingController and their
respective unit tests are removed with this CL, but there will be
several more to follow.

Bug: webrtc:10633
Change-Id: I1986ccf093434ac8fbd8d6db82a0bb44f50b514e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/149838
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28930}
2019-08-21 16:40:55 +00:00
Erik Språng
d05edecf4c Extract most of PacedSender into PacedSendingController.
The Pacer now just handles interaction with Module/ProcessThread and
forwarding packets to PacketRouter.
All other logic is moved to PacedSendingController, including tests.
PacedSender unittest are now just some basic sanity tests.

Bug: webrtc:10809
Change-Id: I69223cd9d8300997375b03706d2e99c88e46241c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/149041
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28886}
2019-08-19 09:07:28 +00:00