This CL is one in a series. To finish the work, the following CLs will be added:
1. CL for connecting RPLR as well
2. CL for RPLR-based FecController
3. CL for allowing experiment-driven configuration of the above (through both field-trials and protobuf)
BUG=webrtc:7058
Review-Url: https://codereview.webrtc.org/2638083002
Cr-Commit-Position: refs/heads/master@{#17365}
New class ReceiveSideCongestionController, extracted from CongestionController, and responsible for the
OnReceivedPacket processing.
Rest of the CongestionController moved to a new class
SendSideCongestionController.
To avoid breaking applications, CongestionController is redefined
as a union of these two classes, with no intended change in behavior.
With one exception: CongestionController::SetBweBitrates used to call
remote_bitrate_estimator_.SetMinBitrate, but after remote_bitrate_estimator_ was moved to ReceiveSideCongestionController,
it no longer does this.
BUG=webrtc:6847
Review-Url: https://codereview.webrtc.org/2752233002
Cr-Commit-Position: refs/heads/master@{#17321}
Lateness is determined by the length of the send-side history, currently
set to 60 seconds.
BUG=webrtc:5079
Review-Url: https://codereview.webrtc.org/2684353004
Cr-Commit-Position: refs/heads/master@{#16588}
This avoids issues where the bitrate produced by the codec is far lower than the target bitrate in the beginning, which causes the delay-based BWE to be initialized accordingly.
BUG=webrtc:5079
Review-Url: https://codereview.webrtc.org/2653883002
Cr-Commit-Position: refs/heads/master@{#16327}
It was only assigned at construction, and this improves consistency
with remote_estimator_proxy_.
The declaration of the private WrappingBitrateEstimator had to be
moved to the header file, and it was also converted from
system_wrappers' CriticalSectionWrapper to rtc::CriticalSection.
BUG=webrtc:6847
Review-Url: https://codereview.webrtc.org/2642363003
Cr-Commit-Position: refs/heads/master@{#16236}
Reason for revert:
Speculative revert for perf regression related to ramp-up on android. See https://bugs.chromium.org/p/chromium/issues/detail?id=682611
Original issue's description:
> Move congestion controller processing to the pacer thread.
>
> Also rename it from pacer_thread_ to congestion_controller_thread_.
>
> BUG=webrtc:6847
>
> Review-Url: https://codereview.webrtc.org/2637783003
> Cr-Commit-Position: refs/heads/master@{#16134}
> Committed: b3dc2b7b1eTBR=danilchap@webrtc.org,stefan@webrtc.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=webrtc:6847
Review-Url: https://codereview.webrtc.org/2644603003
Cr-Commit-Position: refs/heads/master@{#16163}
Also rename it from pacer_thread_ to congestion_controller_thread_.
BUG=webrtc:6847
Review-Url: https://codereview.webrtc.org/2637783003
Cr-Commit-Position: refs/heads/master@{#16134}
This lets the RTP code be unaware of lower layers, and the
SetTransportOverhead method is deleted from RTPSender and RtpRtcp.
Instead, that method is added to CongestionController and
TransportFeedbackAdapter, where it is more appropriate.
BUG=wertc:6847
Review-Url: https://codereview.webrtc.org/2589743002
Cr-Commit-Position: refs/heads/master@{#15995}
Problem fixed: RTP header extensions were not properly set in tests.
BUG=webrtc:5654
Review-Url: https://codereview.webrtc.org/2593963003
Cr-Commit-Position: refs/heads/master@{#15741}
This also avoids an issue where -1 is interpreted as 0xFFFF by the feedback adapter, which in practice likely isn't a problem, but still wrong.
BUG=None
Review-Url: https://codereview.webrtc.org/2579263002
Cr-Commit-Position: refs/heads/master@{#15647}
And delete the method CongestionController::packet_router.
BUG=None
Review-Url: https://codereview.webrtc.org/2516983004
Cr-Commit-Position: refs/heads/master@{#15323}
Now ProbeController can send periodic bandwidth probes when in
application-limited region. This will allow to maintain correct
bottleneck bandwidth estimate, even not all bandwidth is being used.
The feature is not enabled by default, but can be enabled with a flag.
Interval between probes is currently set to 5 seconds.
BUG=webrtc:6332
Review-Url: https://codereview.webrtc.org/2504023002
Cr-Commit-Position: refs/heads/master@{#15279}
Reason for revert:
Unfortunately, this change breaks internal projects. Specifically the change to the CongestionController interface means anything implementing it will be forced to change in lock-step.
Original issue's description:
> Pass time constanct to bwe smoothing filter.
>
> BUG=webrtc:6443, webrtc:6303
>
> Committed: https://crrev.com/9abbf5ae4ec7d688a9b4aa03a405f3faadb74b90
> Cr-Commit-Position: refs/heads/master@{#15266}
TBR=minyue@webrtc.org,stefan@webrtc.org,solenberg@webrtc.org,michaelt@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:6443, webrtc:6303
Review-Url: https://codereview.webrtc.org/2532993002
Cr-Commit-Position: refs/heads/master@{#15272}
Previously ProbeController was starting probing as soon as SetBitrates()
is called. As result these probes would often timeout while connection
is being established. Now ProbeController receives notifications about
network route changes. This allows to start probing only when transport
is connected. This also makes it possible to restart probing whenever
transport route changes (will be done in a separate change).
BUG=webrtc:6332
R=honghaiz@webrtc.org, philipel@webrtc.org, stefan@webrtc.org
Review URL: https://codereview.webrtc.org/2458863002 .
Committed: https://crrev.com/5c99c76255ee7bface3c742c25fb5617748ac86e
Cr-Original-Commit-Position: refs/heads/master@{#15094}
Cr-Commit-Position: refs/heads/master@{#15204}
Reason for revert:
It broke downstream test.
Original issue's description:
> Start probes only after network is connected.
>
> Previously ProbeController was starting probing as soon as SetBitrates()
> is called. As result these probes would often timeout while connection
> is being established. Now ProbeController receives notifications about
> network route changes. This allows to start probing only when transport
> is connected. This also makes it possible to restart probing whenever
> transport route changes (will be done in a separate change).
>
> BUG=webrtc:6332
>
> Committed: https://crrev.com/5c99c76255ee7bface3c742c25fb5617748ac86e
> Cr-Commit-Position: refs/heads/master@{#15094}
TBR=philipel@webrtc.org,stefan@webrtc.org,sergeyu@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:6332
Review-Url: https://codereview.webrtc.org/2504783002
Cr-Commit-Position: refs/heads/master@{#15098}
Previously ProbeController was starting probing as soon as SetBitrates()
is called. As result these probes would often timeout while connection
is being established. Now ProbeController receives notifications about
network route changes. This allows to start probing only when transport
is connected. This also makes it possible to restart probing whenever
transport route changes (will be done in a separate change).
BUG=webrtc:6332
Review-Url: https://codereview.webrtc.org/2458863002
Cr-Commit-Position: refs/heads/master@{#15094}
To achieve this some refactoring was done to make it possible to synchronize
access to the DelayBasedBwe in TransportFeedbackAdapter:
- The callback was removed from DelayBasedBwe, it now instead returns its
result.
- TransportFeedbackAdapter was moved to modules/congestion_controller to avoid
unnecessary dependencies.
Reenables previously disabled flaky test. Can no longer reproduce flakiness with gtest-parallel and asan/tsan builds.
BUG=webrtc:6427, webrtc:6422
R=terelius@webrtc.org
Review URL: https://codereview.webrtc.org/2378103005 .
Cr-Commit-Position: refs/heads/master@{#14452}
Reason for revert:
Caused issues with webrtc_perf_tests on build bots.
Original issue's description:
> Fix race / crash in OnNetworkRouteChanged().
>
> To achieve this some refactoring was done to make it possible to synchronize
> access to the DelayBasedBwe in TransportFeedbackAdapter:
> - The callback was removed from DelayBasedBwe, it now instead returns its
> result.
> - TransportFeedbackAdapter was moved to modules/congestion_controller to avoid
> unnecessary dependencies.
>
> Reenables previously disabled flaky test. Can no longer reproduce flakiness with gtest-parallel and asan/tsan builds.
>
> BUG=webrtc:6427, webrtc:6422
>
> Committed: https://crrev.com/fd0d42669204e6dd92a60736bca7ae0196663024
> Cr-Commit-Position: refs/heads/master@{#14430}
TBR=terelius@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:6427, webrtc:6422
Review-Url: https://codereview.webrtc.org/2377303002
Cr-Commit-Position: refs/heads/master@{#14433}
To achieve this some refactoring was done to make it possible to synchronize
access to the DelayBasedBwe in TransportFeedbackAdapter:
- The callback was removed from DelayBasedBwe, it now instead returns its
result.
- TransportFeedbackAdapter was moved to modules/congestion_controller to avoid
unnecessary dependencies.
Reenables previously disabled flaky test. Can no longer reproduce flakiness with gtest-parallel and asan/tsan builds.
BUG=webrtc:6427, webrtc:6422
Review-Url: https://codereview.webrtc.org/2366333003
Cr-Commit-Position: refs/heads/master@{#14430}
Avoids reporting a growing delay (i.e. time b/w current time and oldest packet in the pacer).
BUG=webrtc:6253
Review-Url: https://codereview.webrtc.org/2279283002
Cr-Commit-Position: refs/heads/master@{#14118}
After offline discussion, this was mostly of historical use and may
actually be more of a risk now that we have the paced sender.
BUG=
Review-Url: https://codereview.webrtc.org/2198933004
Cr-Commit-Position: refs/heads/master@{#13643}
A reset of Bwe and rates on network route change triggers
switching of the RemoteBitrateEstimator from DelayBasedBwe
to RemoteBitrateEstimatorAbsSendTime. This is unexpected,
fix this.
BUG=
R=stefan@webrtc.org
Review URL: https://codereview.webrtc.org/2193043002 .
Cr-Commit-Position: refs/heads/master@{#13594}
The plot is constructed by actually running the congestion controller with
the logged rtp headers and rtcp feedback messages to reproduce the same behavior
as in the real call.
R=phoglund@webrtc.org, terelius@webrtc.org
Review URL: https://codereview.webrtc.org/2193763002 .
Cr-Commit-Position: refs/heads/master@{#13574}
Reason for revert:
Breaks downstream targets.
Original issue's description:
> Add BWE plot to event log analyzer.
>
> The plot is constructed by actually running the congestion controller with
> the logged rtp headers and rtcp feedback messages to reproduce the same behavior
> as in the real call.
>
> R=phoglund@webrtc.org, terelius@webrtc.org
>
> Committed: https://crrev.com/2beea2a8c920000ef19eea20cce397507fc3d5e7
> Cr-Commit-Position: refs/heads/master@{#13558}
TBR=phoglund@webrtc.org,stefan@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Review-Url: https://codereview.webrtc.org/2190013002
Cr-Commit-Position: refs/heads/master@{#13559}
The plot is constructed by actually running the congestion controller with
the logged rtp headers and rtcp feedback messages to reproduce the same behavior
as in the real call.
R=phoglund@webrtc.org, terelius@webrtc.org
Review URL: https://codereview.webrtc.org/2188033004 .
Cr-Commit-Position: refs/heads/master@{#13558}
Reason for revert:
Upstream fixes in place, should be OK now.
Original issue's description:
> Revert of Refactor NACK bitrate allocation (patchset #16 id:300001 of https://codereview.webrtc.org/2061423003/ )
>
> Reason for revert:
> Breaks upstream code.
>
> Original issue's description:
> > Refactor NACK bitrate allocation
> >
> > Nack bitrate allocation should not be done on a per-rtp-module basis,
> > but rather shared bitrate pool per call. This CL moves allocation to the
> > pacer and cleans up a bunch if bitrate stats handling.
> >
> > BUG=
> > R=danilchap@webrtc.org, stefan@webrtc.org, tommi@webrtc.org
> >
> > Committed: 5fc59e810b
>
> TBR=tommi@webrtc.org,danilchap@webrtc.org,stefan@webrtc.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=
>
> Committed: https://crrev.com/e5dd44101eca485f5ad12e5f7ce6f6b0d204116b
> Cr-Commit-Position: refs/heads/master@{#13417}
TBR=tommi@webrtc.org,danilchap@webrtc.org,stefan@webrtc.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=
Review-Url: https://codereview.webrtc.org/2146013002
Cr-Commit-Position: refs/heads/master@{#13465}
Also adds a copy of the BWE test suite to the new DelayBasedBwe class.
BUG=webrtc:6079
Review-Url: https://codereview.webrtc.org/2126793002
Cr-Commit-Position: refs/heads/master@{#13428}
Reason for revert:
It keeps breaking upstream.
Original issue's description:
> Reland Issue 2061423003: Refactor NACK bitrate allocation
>
> This is a reland of https://codereview.webrtc.org/2061423003/
> Which was reverted in https://codereview.webrtc.org/2131913003/
>
> The reason for the revert was that some upstream code used
> RtpSender::SetTargetBitrate(). I've added that back as a no-op until we
> it's been brought up to date.
>
> TBR=tommi@webrtc.org
>
> Committed: 05ce4ae31fTBR=tommi@webrtc.org,sprang@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Review-Url: https://codereview.webrtc.org/2130423002
Cr-Commit-Position: refs/heads/master@{#13419}
