The purpose is to ensure send socket buffers are not overfilled at high
pacing rates.
Bug: chromium:1354491
Change-Id: Ic6f473080292f84a2a099b85fb5817f7e14e7355
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/323000
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40911}
This CL introduces a new feature enabling video packet send batches.
The feature is enabled via
PeerConnectionInterface
::RTCConfiguration
::MediaConfig
::enable_send_packet_batching.
PacketOptions have been augmented with attribute "batchable" (set for
all video packets) and attribute "last_packet_in_batch" which gives
injected AsyncPacketSockets a chance to understand when a batch begins
and ends.
When the feature is on, packets are collected in RtpSenderEgress. On
reception of OnBatchComplete from PacingController, RtpSenderEgress
sends the collected batch, setting "last_packet_in_batch" to true
in the last packet.
Bug: chromium:1439830
Change-Id: I1846b9d4a8a0efd227d617691213a2e048bdc8a2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/303720
Commit-Queue: Markus Handell <handellm@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40012}
If the real-time clock makes a sudden and large shift forward while
there is at least one packet in the queue, the pacer would previously
try to "replay" the behavior it would have done while it was asleep.
This can lead to grand bursts of padding packets, which is
undesireable.
This CL mitigates this problem by forwarding the internal state clock
to (now - 50ms) if there is nothing to do but generate padding.
Bug: webrtc:11340, b/258509536
Change-Id: I5b8a130d938dd2566f72c1946c139f50e099e5a9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/285380
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38752}
We have seen a few instances in a down-stream project where the circuit
breaker is still triggering and causing issues.
This CL makes the threshold configurable and adds more debug logging to
try and get to the bottom of this rarely occuring bug.
Bug: webrtc:11340, b/258509536
Change-Id: I92674d446b926ad66538ff9c8be2a32a3d95b057
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/283762
Reviewed-by: Emil Lundmark <lndmrk@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38664}
This CL adds the ability to forward aborted retransmission notifications
to specified RTP modules, as well as a way to find the RTX ssrc
associated with a media SSRC.
These will both be used by upcoming logic that can selectively flush
given streams from the pacer queue.
Bug: webrtc:11340
Change-Id: Ief3be47e4fd7dc5a1499bc21890e8979400ecb44
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/274706
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38050}
This adds a (default off) flag which makes retransmissions be processed
immediately, just like audio packets normally are.
This might increase send rates and thus losses in some cases, but will
also reduce retranmission delays especially when timer slack or bursting
is used. Usefuleness TBD via experiment.
Bug: chromium:1354491
Change-Id: Icaa83125bfb30826ce72e6e786963d411e05ea57
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/272483
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37926}
A recent cleanup cl (r36900) had an unintended side-effect.
If the queue-time limit is expected to be hit, we adjust the pacing
bitrate up to make sure all packets are sent within the nominal time
frame.
However after that change we stopped adjusting the pacing rate back to
normal levels when queue clears - at least not until the next BWE
update (which is fairly often - but not immediate).
This CL fixes that, and also makes sure whe properly update the
adjusted media rate on enqueu, dequeue and set rate calls.
Bug: webrtc:10809
Change-Id: If00dc35169f1a1347fea6eb44fdb2868282ed3b7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/265387
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37178}
Allows the PacerController to send packets in bursts. If there are enqued packets, or a packet is enqueued while the pacer have a small media debt, an enqued packet is allowed to be sent immediately as long as the debt is smaller than the set burst interval.
Bug: b/233850913
Change-Id: Ibb0fa63c97409ca23b9fa7148b5ff6ce8c4517e2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264462
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37098}
Ensure each test create its own PacerController.
Move (most) operations on the pacer controller to the actual test. (the
rest should be moved too eventually....)
Use only one test fixture.
Bug: none
Change-Id: I0b8eee9d2c2f91f7102858a1a544e45e8b0b7b5d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264120
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37017}
Instead of using field trials in BitrateProber for probe duration, use values provided in ProbeClusterConfig from GoogCC.
Field trials are instead read in ProbeController.
To avoid having to do a thread jump for every ProbeClusterConfig, RtpPacketPacer interface is changed to RtpPacketPacer::CreateProbeClusters(std::vector<ProbeClusterConfig>
Deprecates field trial "WebRTC-Bwe-ProbingConfiguration"
Change-Id: I3991e4b54770601855a3af2d6a16678f11d41c31
Bug: webrtc:14027
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/261265
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36911}
This completes the removal of the legacy pacer.
Bug: webrtc:10809
Change-Id: I8962ad56aa673f46b2c0e2cf8a5630e2c9942c92
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/262421
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36900}
When congestion window is used, two different mechanisms can currently
update the outstanding data state in the pacer:
* OnPacketSent() withing the pacer itself, when a packet is sent
* UpdateOutstandingData(), when RtpTransportControllerSend either:
a. Receives an OnPacketSent() callback (increase outstanding data)
b. Receives transport feedback (decrease outstanding data)
This creates a lot of calls to UpdateOutstandingData(), more than one
per sent packet. Each requires locking and/or thread jumps. To avoid
that, this CL moves the congestion window state to
RtpTransportController send - and we only post a congested flag down
the the pacer when the state is changed.
The only benefit I can see is of the old way is we prevent sending
new packets immedately when the window is full, rather than in some
edge cases queue extra packets on the network task queue before the
congestion signal is received. That should be rare and benign.
I think this simplified logic, which is easier to read and more
performant, is a better tradeoff.
Bug: webrtc:13417
Change-Id: I326dd88db86dc0d6dc685c61920654ac024e57ef
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/255600
Auto-Submit: Erik Språng <sprang@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36220}
This CL also removes dependency on the legacy field trial methods.
Bug: webrtc:11926
Change-Id: I53feeee86b92878cf0f2b8ebdce3d101f9e04014
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/255381
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Auto-Submit: Erik Språng <sprang@webrtc.org>
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Commit-Queue: Björn Terelius <terelius@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36205}
The pacer status is never changed unless MaybeProcessPackets() is
called. This CL removes the scheduler, and updates pacer status after
every MaybeProcessPackets().
Bug: webrtc:10809, webrtc:13417
Change-Id: Ib5f18decf44c1596c0a716d799600a72b2332abd
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/239120
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35489}
When new packets are enqueued after a dead-period where media debt is
zero, that time slice should not be used to reduce the debt for the
new packet.
Bug: webrtc:10809
Change-Id: Ifb960548e6aa349b79f37743cbfed78a5c937a13
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/234081
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35143}
This is expected to yield slightly higher bandwidth estimates when
probing is used, since it reduces a bias in how packet sizes are counted.
Bug: webrtc:11780
Change-Id: I6a4a3af0c50670d248dbe043a4d9da60915e3699
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/187491
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32394}
This CL adds a parameter to the BirateProber field trial config, which
allows the prober to actually discard probe cluster is pacer scheduling
is too delayed. Today it just keeps going at a too low rate.
Some refactoring was needed anyway, so also switch to using unit types
in more places.
Initially keeps legacy behavior default, to verify no perf regressions.
Bug: webrtc:11780
Change-Id: I9edd114773b10a8d86b54a1a0398a4052aab9dd5
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/179090
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31756}
This is a reland of 19df870d92
Patchset 1 is the original.
Subsequent patchset changes threadchecker that crashed with downstream
code.
Original change's description:
> Reland "Allows FEC generation after pacer step."
>
> This is a reland of 75fd127640
>
> Patchset 2 contains a fix. Old code can in factor call
> RtpRtcpImpl::FetchFec(). It should only be a noop since deferred fec
> is not supported there - we shouldn't crash.
>
> Original change's description:
> > Allows FEC generation after pacer step.
> >
> > Split out from https://webrtc-review.googlesource.com/c/src/+/173708
> > This CL enables FEC packets to be generated as media packets are sent,
> > rather than generated, i.e. media packets are inserted into the fec
> > generator after the pacing stage rather than at packetization time.
> >
> > This may have some small impact of performance. FEC packets are
> > typically only generated when a new packet with a marker bit is added,
> > which means FEC packets protecting a frame will now be sent after all
> > of the media packets, rather than (potentially) interleaved with them.
> > Therefore this feature is currently behind a flag so we can examine the
> > impact. Once we are comfortable with the behavior we'll make it default
> > and remove the old code.
> >
> > Note that this change does not include the "protect all header
> > extensions" part of the original CL - that will be a follow-up.
> >
> > Bug: webrtc:11340
> > Change-Id: I3fe139c5d53968579b75b91e2612075451ff0f5d
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/177760
> > Commit-Queue: Erik Språng <sprang@webrtc.org>
> > Reviewed-by: Sebastian Jansson <srte@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#31558}
>
> Bug: webrtc:11340
> Change-Id: I2ea49ee87ee9ff409044e34a777a7dd0ae0a077f
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/177984
> Commit-Queue: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Sebastian Jansson <srte@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#31613}
Bug: webrtc:11340
Change-Id: Ib741c8c284f523c959f8aca454088d9eee7b17f8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/178600
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31619}
This reverts commit 19df870d92.
Reason for revert: Downstream project failure
Original change's description:
> Reland "Allows FEC generation after pacer step."
>
> This is a reland of 75fd127640
>
> Patchset 2 contains a fix. Old code can in factor call
> RtpRtcpImpl::FetchFec(). It should only be a noop since deferred fec
> is not supported there - we shouldn't crash.
>
> Original change's description:
> > Allows FEC generation after pacer step.
> >
> > Split out from https://webrtc-review.googlesource.com/c/src/+/173708
> > This CL enables FEC packets to be generated as media packets are sent,
> > rather than generated, i.e. media packets are inserted into the fec
> > generator after the pacing stage rather than at packetization time.
> >
> > This may have some small impact of performance. FEC packets are
> > typically only generated when a new packet with a marker bit is added,
> > which means FEC packets protecting a frame will now be sent after all
> > of the media packets, rather than (potentially) interleaved with them.
> > Therefore this feature is currently behind a flag so we can examine the
> > impact. Once we are comfortable with the behavior we'll make it default
> > and remove the old code.
> >
> > Note that this change does not include the "protect all header
> > extensions" part of the original CL - that will be a follow-up.
> >
> > Bug: webrtc:11340
> > Change-Id: I3fe139c5d53968579b75b91e2612075451ff0f5d
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/177760
> > Commit-Queue: Erik Språng <sprang@webrtc.org>
> > Reviewed-by: Sebastian Jansson <srte@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#31558}
>
> Bug: webrtc:11340
> Change-Id: I2ea49ee87ee9ff409044e34a777a7dd0ae0a077f
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/177984
> Commit-Queue: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Sebastian Jansson <srte@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#31613}
TBR=sprang@webrtc.org,srte@webrtc.org
Change-Id: I3b2b25898ce88b64c2322f68ef83f9f86ac2edb0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:11340
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/178563
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31614}
This is a reland of 75fd127640
Patchset 2 contains a fix. Old code can in factor call
RtpRtcpImpl::FetchFec(). It should only be a noop since deferred fec
is not supported there - we shouldn't crash.
Original change's description:
> Allows FEC generation after pacer step.
>
> Split out from https://webrtc-review.googlesource.com/c/src/+/173708
> This CL enables FEC packets to be generated as media packets are sent,
> rather than generated, i.e. media packets are inserted into the fec
> generator after the pacing stage rather than at packetization time.
>
> This may have some small impact of performance. FEC packets are
> typically only generated when a new packet with a marker bit is added,
> which means FEC packets protecting a frame will now be sent after all
> of the media packets, rather than (potentially) interleaved with them.
> Therefore this feature is currently behind a flag so we can examine the
> impact. Once we are comfortable with the behavior we'll make it default
> and remove the old code.
>
> Note that this change does not include the "protect all header
> extensions" part of the original CL - that will be a follow-up.
>
> Bug: webrtc:11340
> Change-Id: I3fe139c5d53968579b75b91e2612075451ff0f5d
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/177760
> Commit-Queue: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Sebastian Jansson <srte@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#31558}
Bug: webrtc:11340
Change-Id: I2ea49ee87ee9ff409044e34a777a7dd0ae0a077f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/177984
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31613}
This reverts commit 980cadd02c.
Reason for revert: Problematic code now fix.
Original change's description:
> Revert "Lets PacingController call PacketRouter directly."
>
> This reverts commit 848ea9f0d3.
>
> Reason for revert: Part of changes that may cause deadlock
>
> Original change's description:
> > Lets PacingController call PacketRouter directly.
> >
> > Since locking model has been cleaned up, PacingController can now call
> > PacketRouter directly - without having to go via PacedSender or
> > TaskQueuePacedSender.
> >
> > Bug: webrtc:10809
> > Change-Id: I181f04167d677c35395286f8b246aefb4c3e7ec7
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/175909
> > Reviewed-by: Sebastian Jansson <srte@webrtc.org>
> > Commit-Queue: Erik Språng <sprang@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#31342}
>
> TBR=sprang@webrtc.org,srte@webrtc.org
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug: webrtc:10809
> Change-Id: I1d7d5217a03a51555b130ec5c2dd6a992b6e489e
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/178021
> Reviewed-by: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Sebastian Jansson <srte@webrtc.org>
> Commit-Queue: Erik Språng <sprang@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#31563}
TBR=sprang@webrtc.org,srte@webrtc.org
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: webrtc:10809
Change-Id: I8bea1a5b1b1f618b697e4b09d83c9aac08099593
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/178389
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31600}
This reverts commit 848ea9f0d3.
Reason for revert: Part of changes that may cause deadlock
Original change's description:
> Lets PacingController call PacketRouter directly.
>
> Since locking model has been cleaned up, PacingController can now call
> PacketRouter directly - without having to go via PacedSender or
> TaskQueuePacedSender.
>
> Bug: webrtc:10809
> Change-Id: I181f04167d677c35395286f8b246aefb4c3e7ec7
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/175909
> Reviewed-by: Sebastian Jansson <srte@webrtc.org>
> Commit-Queue: Erik Språng <sprang@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#31342}
TBR=sprang@webrtc.org,srte@webrtc.org
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: webrtc:10809
Change-Id: I1d7d5217a03a51555b130ec5c2dd6a992b6e489e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/178021
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31563}
This reverts commit 75fd127640.
Reason for revert: Breaks downstream test
Original change's description:
> Allows FEC generation after pacer step.
>
> Split out from https://webrtc-review.googlesource.com/c/src/+/173708
> This CL enables FEC packets to be generated as media packets are sent,
> rather than generated, i.e. media packets are inserted into the fec
> generator after the pacing stage rather than at packetization time.
>
> This may have some small impact of performance. FEC packets are
> typically only generated when a new packet with a marker bit is added,
> which means FEC packets protecting a frame will now be sent after all
> of the media packets, rather than (potentially) interleaved with them.
> Therefore this feature is currently behind a flag so we can examine the
> impact. Once we are comfortable with the behavior we'll make it default
> and remove the old code.
>
> Note that this change does not include the "protect all header
> extensions" part of the original CL - that will be a follow-up.
>
> Bug: webrtc:11340
> Change-Id: I3fe139c5d53968579b75b91e2612075451ff0f5d
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/177760
> Commit-Queue: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Sebastian Jansson <srte@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#31558}
TBR=sprang@webrtc.org,srte@webrtc.org
Change-Id: Ie714e5f68580cbd57560e086c9dc7292a052de5f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:11340
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/177983
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31559}
Split out from https://webrtc-review.googlesource.com/c/src/+/173708
This CL enables FEC packets to be generated as media packets are sent,
rather than generated, i.e. media packets are inserted into the fec
generator after the pacing stage rather than at packetization time.
This may have some small impact of performance. FEC packets are
typically only generated when a new packet with a marker bit is added,
which means FEC packets protecting a frame will now be sent after all
of the media packets, rather than (potentially) interleaved with them.
Therefore this feature is currently behind a flag so we can examine the
impact. Once we are comfortable with the behavior we'll make it default
and remove the old code.
Note that this change does not include the "protect all header
extensions" part of the original CL - that will be a follow-up.
Bug: webrtc:11340
Change-Id: I3fe139c5d53968579b75b91e2612075451ff0f5d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/177760
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31558}
Since locking model has been cleaned up, PacingController can now call
PacketRouter directly - without having to go via PacedSender or
TaskQueuePacedSender.
Bug: webrtc:10809
Change-Id: I181f04167d677c35395286f8b246aefb4c3e7ec7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/175909
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31342}
Today when the pacing debt is cleared, we blindly ask for 50 bytes of
padding, which is above a static magic number for RTX payload padding.
Instead, we should adjust the target size based on the current padding
rate. The old pacer sort-of does this, it allows the budget to grow up
to one process interval (usually 5ms).
This CL makes the dynamic pacer also use a duration as target, by
default 5ms to match old pacer but with a trial to allow tweaking it.
This will be important for good behavior due to
https://bugs.chromium.org/p/webrtc/issues/detail?id=11508
Bug: webrtc:10809
Change-Id: I9c14acc5730c6e2e0d7821adf5fb058b8d5487c6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/173687
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31091}
The TaskQueuePacedSender today has some inefficiencies:
* Enqueuing a packet will trigger a MaybeProcessPackets() call, but it
won't actually run immediately even if it should - instead it will
schedule a new call in at least 1ms. This incurs delays and extra
CPU overhead.
* Sometimes thread wakeups are scheduled simply in order to do
book-keeping: ProcessPackets() will be called when the media debt has
gone down to 0 even if there is no packet in the queue, in order to
check if we should send padding.
This CL fixes that by called ProcessPackets() immediately if it is
actually time to do so, and by immediately determining when padding
should be sent without having a separate call to drain media debt.
Bug: webrtc:10809
Change-Id: I4870e86e6de2ce4197463fd5b788ad4717fc7177
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/172842
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31010}
Non-paced audio should be sent "immediately", but in several places that
was determined by looking at the current time - which can lead to
inconsistencies.
E.g. if a packet is enqueued and ProcessPackets() is called 1ms later,
the pacer should see NextSendTime() as 1ms ago, so that buffer levels
are cleared at the right pace.
Bug: webrtc:10809
Change-Id: I04a169f3df3e28a5c8ef7fa8a042b9c482c307ce
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/172845
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31002}
EnqueuePackets() would reset the last process time if the queue
and media budgets were empty. This was done without reducing the
padding debt.
The result of this was that, given an existing debt, and an interval
between audio packets that is less than the drain time for the padding
debt, padding would not be sent at all.
Now, before adding a new packet, we reduce the padding debt if the
packet queue is empty.
Bug: webrtc:10809
Change-Id: I116169522c215257febd32e17abab45f1a7d609f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/171808
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30949}
Specifically, if dynamic pacer (i.e. TaskQueuePacer) was enabled while
AccountForAudio was set to true, the pacer would pace audio packets.
This should only happen when the WebRTC-Pacer-BlockAudio field trial is
enabled.
Bug: webrtc:10809
Change-Id: If5edc77de88ca9866abeb3b47e171df50673299e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/172082
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30938}
This is in preparation of an upcoming CL that will propagate this
information through the TransportFeedbackAdapter.
Bug: webrtc:10932
Change-Id: Ic2a026b5ef72d6bf01e698e7634864fedc659b4e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/168220
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Commit-Queue: Björn Terelius <terelius@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30476}
After experimentation, not pacing audio is better. This is controlled
by the field trial WebRTC-Pacer-BlockAudio. This change keeps the flag,
but changes the behaviour such that it defaults to Disabled. However,
audio can still be paced if one chooses by enabling the field trial.
Bug: webrtc:11257
Change-Id: I5b23a82bb6708c007cf8dfb40065c821eefdc4e3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/165381
Commit-Queue: Evan Shrubsole <eshr@google.com>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Christoffer Rodbro <crodbro@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30262}
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}
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}
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}
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}
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}
