This metric was always supposed to be the spec's answer to
googBucketDelay, and is defined as "The total number of seconds that
packets have spent buffered locally before being transmitted onto the
network." But our implementation measured the time between capture and
send, including encode time. This is incorrect and yields a much larger
value than expected.
This CL updated the metric to do what the spec says. Implementation-wise
we measure the time between pushing and popping each packet from the
queue (in modules/pacing/prioritized_packet_queue.cc).
The spec says to increment the delay counter at the same time as we
increment the packet counter in order for the app to be able to do
"delta totalPacketSendDelay / delta packetSent". For this reason,
`total_packet_delay` is added to RtpPacketCounter. (Previously, the
two counters were incremented on different threads and observers.)
Running Google Meet on a good network, I could observe a 2-3 ms average
send delay per packet with this implementation compared to 20-30 ms
with the old implementation. See b/137014977#comment170 for comparison
with googBucketDelay which is a little bit different by design -
totalPacketSendDelay is clearly better than googBucketDelay.
Since none of this depend on the media kind, we can wire up this metric
for audio as well in a follow-up:
https://webrtc-review.googlesource.com/c/src/+/280523
Bug: webrtc:14593
Change-Id: If8fcd82fee74030d0923ee5df2c2aea2264600d4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/280443
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38480}
This new class implements the existing FieldTrialsView interface,
extending it with the verification functionality. For now, the
verification will only be performed if the rtc_strict_field_trials GN
arg is set.
Most classes extending FieldTrialsView today have been converted to
extend from FieldTrialsRegistry instead to automatically perform
verification.
Bug: webrtc:14154
Change-Id: I4819724cd66a04507e62fcc2bb1019187b6ba8c7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/276270
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Emil Lundmark <lndmrk@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38453}
Instead of re-using the sender task queue, a new task queue will
suffice.
Bug: webrtc:14445
Change-Id: Ia7395ace2f0bb66bf9e76e3783b208f2cd0385dc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/275771
Commit-Queue: Evan Shrubsole <eshr@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38332}
- Propagating `RtpPacketInfo::local_capture_clock_offset`, an
existing field that is related to the abs-capture-timestamp
header extension field `estimated_capture_clock_offset`
- Propagated through `SourceTracker::SourceEntry`
Bug: webrtc:10739, b/246753278
Change-Id: I21d9841e4f3a35da5f8d7b31582898309421d524
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/275241
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38129}
The `TimestampExtrapolator` is only used by the `VCMTiming`
class, despite there being references to it from both
`modules/rtp_rtcp/BUILD.gn` and `modules/video_coding/BUILD.gn`.
Bug: webrtc:14111
Change-Id: If1a02a56a0c83b13d619ca08dc76c884fa829369
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/275482
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Johannes Kron <kron@webrtc.org>
Commit-Queue: Rasmus Brandt <brandtr@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38093}
* Make `percentile` configurable and rename class.
* Introduce convenience type `MovingMedianFilter` that
maintains the behaviour of the old class with that name.
* Move home grown moving 95th percentile filter in
`JitterEstimator` to this new utility class.
Bug: webrtc:14151
Change-Id: I17d525b6e0bc98c28568c7dfe94b72eeab4a1ca2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/275311
Commit-Queue: Rasmus Brandt <brandtr@webrtc.org>
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38082}
In some upcoming use cases we might wish to flush pending
retransmissions from the pacer queue. In order to not make those packets
forever inaccessible this CL adds a way to clear their "pending" status
from the packet history.
Bug: webrtc:11340
Change-Id: I9aac44125899a7f1e5a5e5be3390ac07b1e661ad
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/274600
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38037}
that rtc::Location parameter was used only as extra information for the
RTC_CHECKs directly in the function, thus call stack of the crash should
provide all the information about the caller.
Bug: webrtc:11318
Change-Id: Iec6dd2c5de547f3e1601647a614be7ce57a55734
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/270920
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37748}
The data that's used to report the histograms is owned by UlpfecReceiver
and moving the reporting there, simplifies things as configuration
changes happen in RtpVideoStreamReceiver2 (which currently require all
receive streams to be deleted+reconstructed).
Additional updates:
* Consistently using `Clock` for timestamps. Before there was
a mix of Clock and rtc::TimeMillis.
* Update code to use Timestamp and TimeDelta.
Bug: none
Change-Id: I89ca28ec7067a49d6b357315ae733b04e7c5a2e3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/271027
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37729}
instead of using Lock/Unlock attributes, use Assert attribute to annotate code is running on certain task queue or thread.
Such check better matches what is checked, in particular allows to
recheck (and thus better document) currently used task queue
Bug: None
Change-Id: I5bc1c397efbc8342cf7915093b578bb015c85651
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/269381
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37619}
This reverts commit a17651f7d8748905d902eedf34471a0c227ca789.
Reason for revert: triggers failure in downstream test
Original change's description:
> Fix overflow due to rounding in AbsoluteSendTime::To24Bits
>
> Actual rounding is not important for this time as long it is consistent
> during the call: only difference between two absolute send time matter
> Rounding down avoids producing 1 < 24 when value is close to the wrap around boundary.
>
> Bug: None
> Change-Id: Ibbf5bae21bc37eccdc5d4c130a59796ee5108017
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/268001
> Commit-Queue: Åsa Persson <asapersson@webrtc.org>
> Auto-Submit: Danil Chapovalov <danilchap@webrtc.org>
> Reviewed-by: Åsa Persson <asapersson@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#37468}
Bug: None
Change-Id: I90a9c1b174b918b7ede58c3bbdb879b1b67da7b2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/268120
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37473}
Actual rounding is not important for this time as long it is consistent
during the call: only difference between two absolute send time matter
Rounding down avoids producing 1 < 24 when value is close to the wrap around boundary.
Bug: None
Change-Id: Ibbf5bae21bc37eccdc5d4c130a59796ee5108017
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/268001
Commit-Queue: Åsa Persson <asapersson@webrtc.org>
Auto-Submit: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37468}
since the fec packets are initialized to 0 there is no need
to special-case the first packet since
A XOR 0
is the identity operator.
BUG=None
Change-Id: I0cb55283ecdca06f8e3a7b5856ec1f9fbbad1ffb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/251522
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37378}
