In order to not make this CL too large I have broken it down into at least two steps. In this CL we only propagate the pacing information part of the way:
webrtc::PacedSender::Process <--- propagate from here
webrtc::PacedSender::SendPacket
webrtc::PacketRouter::TimeToSendPacket
webrtc::ModuleRtpRtcpImpl::TimeToSendPacket <--- to here
webrtc::RTPSender::TimeToSendPacket
webrtc::RTPSender::PrepareAndSendPacket
webrtc::RTPSender::AddPacketToTransportFeedback
webrtc::TransportFeedbackAdapter::AddPacket
webrtc::SendTimeHistory::AddAndRemoveOld <--- goal is to propagte it here
BUG=webrtc:6822
Review-Url: https://codereview.webrtc.org/2628563003
Cr-Commit-Position: refs/heads/master@{#16664}
Video modules are added in reverse order to ensure that the padding order is the same as before, prioritizing high resolution streams.
BUG=webrtc:7043
Review-Url: https://codereview.webrtc.org/2655033002
Cr-Commit-Position: refs/heads/master@{#16329}
This means that smaller probe packets will be allowed at lower bitrates.
BUG=webrtc:7043
Review-Url: https://codereview.webrtc.org/2650393002
Cr-Commit-Position: refs/heads/master@{#16317}
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}
Previously BirateProber was calculating delay between probes based on
the size of the previous probe. Because of that the actual sent bitrate
can deviate greatly from the target value. With this change it uses
total number of bytes in the cluster to estimate delay before each
probe.
BUG=webrtc:6952
Review-Url: https://codereview.webrtc.org/2613543003
Cr-Original-Commit-Position: refs/heads/master@{#15971}
Committed: 599c5011e7
Review-Url: https://codereview.webrtc.org/2613543003
Cr-Commit-Position: refs/heads/master@{#16127}
Reason for revert:
Speculative revert.
Linux memcheck bot started failing a lot at the time of this cl. Doesn't look related at first glance, but we don't have another lead yet.
Original issue's description:
> Fix BitrateProber to match the requested bitrate more precisely
>
> Previously BirateProber was calculating delay between probes based on
> the size of the previous probe. Because of that the actual sent bitrate
> can deviate greatly from the target value. With this change it uses
> total number of bytes in the cluster to estimate delay before each
> probe.
>
> BUG=webrtc:6952
>
> Review-Url: https://codereview.webrtc.org/2613543003
> Cr-Commit-Position: refs/heads/master@{#15971}
> Committed: 599c5011e7TBR=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:6952
Review-Url: https://codereview.webrtc.org/2626473004
Cr-Commit-Position: refs/heads/master@{#15979}
Previously BirateProber was calculating delay between probes based on
the size of the previous probe. Because of that the actual sent bitrate
can deviate greatly from the target value. With this change it uses
total number of bytes in the cluster to estimate delay before each
probe.
BUG=webrtc:6952
Review-Url: https://codereview.webrtc.org/2613543003
Cr-Commit-Position: refs/heads/master@{#15971}
Instead of having to specify a bitrate and how many packets to use,
the BitrateProber will now use the bitrate to calculate how many
bytes it will use to probe that bitrate instead.
For now, |kMinProbeDurationMs| is set to 15 ms which means that probing
at 1900 kbps will result in 1900/8*0.015 = 3.5 kB used. Since we can
expect packets to be smaller at the beginning of a stream (500 to 700
bytes) this will result in 7 to 5 packets sent for that bitrate, and
should work very similar to how the current initial probing works.
A minimum of 5 packets are always sent.
BUG=webrtc:6822
Review-Url: https://codereview.webrtc.org/2609113003
Cr-Commit-Position: refs/heads/master@{#15899}
This moves some GN check configurations out of .gn to individual
targets.
The now checked target is:
"//webrtc/modules/pacing/*"
BUG=webrtc:6828
NOTRY=True
Review-Url: https://codereview.webrtc.org/2594523003
Cr-Commit-Position: refs/heads/master@{#15729}
There's no longer any need to make the two arguments have the same
signedness, so we can drop the "u" suffix on literal integer
arguments.
NOPRESUBMIT=true
BUG=webrtc:6645
Review-Url: https://codereview.webrtc.org/2535593002
Cr-Commit-Position: refs/heads/master@{#15280}
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}
Previously AlrDetector was measuring amount of data sent in each 100ms
interval and would enter ALR mode after 5 consecutive intervals when
average bandwidth usage doesn't exceed 30% of the current estimate
estimate. This meant that an application that uses only slightely more
than 6% of total bandwidth may stay out of ALR mode, e.g. if it sends
a frame of size BW*30ms every 0.5 seconds. 100ms is too short interval
to average over, particularly when frame-rate falls below 10fps.
With this change AlrDetector averages BW usage over last 500ms. It then
enters ALR state when usage falls below 30% and exits it when usage
exceeds 50%.
BUG=webrtc:6332
R=philipel@webrtc.org, stefan@webrtc.org
Review URL: https://codereview.webrtc.org/2503643003 .
Cr-Commit-Position: refs/heads/master@{#15109}
Prior to this change, FlexFEC packets that were paced would be lost in
the RTPSender, since they were not stored in a packet history. This CL
introduces such a packet history, as well as the needed wireup for
higher layers to be aware that the particular RTPSender is able to
send FlexFEC packets with a particular SSRC.
Updated RTPSender unit test to reflect the fact that paced packets
are now actually sent.
BUG=webrtc:5654
Review-Url: https://codereview.webrtc.org/2491293002
Cr-Commit-Position: refs/heads/master@{#15066}
This is a simple application limited region detector that is quite conservative
at the moment. We detect as being application-limited if we see sending rate
as less than 30% of the estimated bandwidth over 500 ms. The moment we detect
a single burst above 30% over a 100 ms period, we consider ourselves network
limited.
This class is currently not used. A follow up CL will leverage this to enable probing.
BUG=webrtc:6332
Review-Url: https://codereview.webrtc.org/2340763004
Cr-Commit-Position: refs/heads/master@{#14505}
Currently, BitrateProber does not scale higher than 2 Mbps to 6 Mbps. The actual
number is dependent on the size of the last packet. If a packet of around 250
bytes is used for probing, it fails above 2 Mbps.
BitrateProber now provides a recommendation on probe size instead of a
packet size. PacedSender utilizes this to decide on the number of packets
per probe. This enables BitrateProber to scale up-to higher bitrates.
Tests with chromoting show it stalls at about 10 Mbps (perhaps due to the
limitation on the simulation pipeline to deliver packets).
BUG=webrtc:6332
Review-Url: https://codereview.webrtc.org/2347023002
Cr-Commit-Position: refs/heads/master@{#14503}
Reason for revert:
Fix backward compatibility support
Original issue's description:
> Revert of Unify rtcp packet setters (patchset #8 id:130001 of https://codereview.webrtc.org/2348623003/ )
>
> Reason for revert:
> Breaks compilation of internal downstream project.
>
> Original issue's description:
> > Unify rtcp packet setters
> > Renamed setters in rtcp classes
> > from WithField to SetField
> > from WithItem to AddItem or SetItems
> > from From to SetSenderSsrc
> > from To to SetMediaSsrc
> > Some redundant or unsued setters removed.
> > Pass-by-const& replaced with pass-by-value when appropriate.
> >
> > BUG=webrtc:5260
> >
> > Committed: https://crrev.com/20e77c7b8a9f19942ef3c3c4f1fa3888b2cd54ea
> > Cr-Commit-Position: refs/heads/master@{#14393}
>
> TBR=sprang@webrtc.org,stefan@webrtc.org,danilchap@webrtc.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=webrtc:5260
>
> Committed: https://crrev.com/efc6e41866662e0922858fbce1d9ee3bdd0637ed
> Cr-Commit-Position: refs/heads/master@{#14400}
TBR=sprang@webrtc.org,stefan@webrtc.org,kjellander@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:5260
Review-Url: https://codereview.webrtc.org/2370313002
Cr-Commit-Position: refs/heads/master@{#14402}
Reason for revert:
Breaks compilation of internal downstream project.
Original issue's description:
> Unify rtcp packet setters
> Renamed setters in rtcp classes
> from WithField to SetField
> from WithItem to AddItem or SetItems
> from From to SetSenderSsrc
> from To to SetMediaSsrc
> Some redundant or unsued setters removed.
> Pass-by-const& replaced with pass-by-value when appropriate.
>
> BUG=webrtc:5260
>
> Committed: https://crrev.com/20e77c7b8a9f19942ef3c3c4f1fa3888b2cd54ea
> Cr-Commit-Position: refs/heads/master@{#14393}
TBR=sprang@webrtc.org,stefan@webrtc.org,danilchap@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:5260
Review-Url: https://codereview.webrtc.org/2372713005
Cr-Commit-Position: refs/heads/master@{#14400}
Renamed setters in rtcp classes
from WithField to SetField
from WithItem to AddItem or SetItems
from From to SetSenderSsrc
from To to SetMediaSsrc
Some redundant or unsued setters removed.
Pass-by-const& replaced with pass-by-value when appropriate.
BUG=webrtc:5260
Review-Url: https://codereview.webrtc.org/2348623003
Cr-Commit-Position: refs/heads/master@{#14393}
This changes most non-test related rtc_source_set targets to be
rtc_static_library instead. Targets without any .cc files are excluded.
This should bring back the build behavior we used to have with GYP
(i.e. same symbols exported in the libjingle_peerconnection.a file, which
are used by some downstream projects).
After doing an Android build with these changes:
$ nm --defined-only -g -C out/Release/lib.unstripped/libjingle_peerconnection_so.so | grep -i createpeerconnectionf
00077c51 T Java_org_webrtc_PeerConnectionFactory_nativeCreatePeerConnectionFactory
$ nm --defined-only -g -C out/Release/obj/webrtc/api/libjingle_peerconnection.a | grep -i createpeerconnectionf
00000001 T webrtc::CreatePeerConnectionFactory(rtc::Thread*, rtc::Thread*, rtc::Thread*, webrtc::AudioDeviceModule*, cricket::WebRtcVideoEncoderFactory*, cricket::WebRtcVideoDecoderFactory*)
00000001 T webrtc::CreatePeerConnectionFactory()
See https://chromium.googlesource.com/chromium/src/+/master/tools/gn/docs/cookbook.md#Note-on-static-libraries
for more details on this.
NOTICE: This should be further cleaned up in the future, to reduce
binary bloat and unnecessary linking time. Right now it's more
important to restore the desired build output though.
BUG=webrtc:6410, chromium:630755
Review-Url: https://codereview.webrtc.org/2361623004
Cr-Commit-Position: refs/heads/master@{#14364}
Remove common_inherited_config from the targets and add it to the
template instead.
BUG=webrtc:6187
NOTRY=True
Review-Url: https://codereview.webrtc.org/2311843002
Cr-Commit-Position: refs/heads/master@{#14069}
Remove common_config from the targets' config and add
it to the template instead.
BUG=webrtc:6187
NOTRY=True
Review-Url: https://codereview.webrtc.org/2300413002
Cr-Commit-Position: refs/heads/master@{#14063}
Defines the rtc_executable, rtc_source_set, rtc_test and
rtc_static_library templates.
These templates provide no functionality yet, but will enable common
configuration to be introduced, avoiding repetition in every target
Changes summary:
- Prepend rtc_ to test, source_set, executable and static_library targets
- Change "configs -= [" to "suppressed_configs += ["
- Include webrtc/build/webrtc.gni where it wasn't included yet
- Delete import("//testing/test.gni"), since rtc_test makes it unnecessary.
BUG=webrtc:6187
TBR=henrik.lundin@webrtc.org,tommi@webrtc.org
NOTRY=True
Review-Url: https://codereview.webrtc.org/2301053002
Cr-Commit-Position: refs/heads/master@{#14043}
- Removes unnecessary casts to compute timedelta.
- Renames ProbingState for clarity. This should help when we probe mid-call.
- Enables probing by default to avoid checking on each incoming packet.
- Removes duplicate probing state tracking in paced sender. These duplicate
states were conflicting at times.
- Removes passing through packets for bug 5307 which seems long fixed.
- Cleanup handling of time_last_send_ms and avoid side effects of changing
probing state at TimeUntilNextProbe().
- Clear cluster data when probing is restarted to avoid having old data after a reset.
BUG=5859
R=stefan@webrtc.org
Review URL: https://codereview.webrtc.org/2182603002 .
Cr-Commit-Position: refs/heads/master@{#13612}
The intended behavior of the pacer is that it should send audio (i.e high priority) packets immediatelly even if the pacer is paused and even if the available media budget has been used. The existing code will not send audio if the pacing budget has been used. It will normally send audio when the pacer is paused, but if the media budget was already used up when the pacer was paused, it would neither send audio nor update the budget.
Updated the implementatation and the unit test. Added logging for when the pacer is paused, and when it receives a zero bitrate estimate.
BUG=webrtc:6155
Review-Url: https://codereview.webrtc.org/2193183003
Cr-Commit-Position: refs/heads/master@{#13592}
Retransmissions are supposed to be sent before normal packets by the pacer, but the current implementation will only use it if the second packet is a retransmission and the first packet is not. It misses the case where the first packet is retransmission and the second packet is not.
This CL fixes the comparator and adds a unit test.
Also changed the SendAndExpectPacket function to propagate the retransmission flag to the expectations. Previously, all packets were expected to be normal packets.
BUG=webrtc:6124
Review-Url: https://codereview.webrtc.org/2156063004
Cr-Commit-Position: refs/heads/master@{#13502}
1. It moves calculation of the needed padding to VideoSendStream instead of ViEEncoder and only does it once per send Stream instead of every time the network estimate changes.
2. The maximum amount of padding sent was prior to this cl calculated and updated based on network estimate changes. However, it can only change based on encoder configuration changes and if send streams are added or removed. This cl change the VideoSendStream/VieEncoder to notify the BitrateAllocator of changes to the needed padding bitrate and for BitrateAllocator to notify Call of these changes.
3. Fixed an issue in the SendPacer where it could send a padding packet before sending a real packet. This caused the test EndToEndTest.RestartingSendStreamPreservesRtpStatesWithRtx to fail with these refactorings since the pacer suddenly could send a padding packet before the encoder had produced its first frame.
BUG=webrtc:5687
Review-Url: https://codereview.webrtc.org/1993113003
Cr-Commit-Position: refs/heads/master@{#13149}
