* The pointer isn't needed for this notification. Arguably using
the internal id is more consistent with the stats code.
* Using the int makes it safer down the line to post the operation
from the network thread to the signaling thread rather than post
an object reference.
Bug: none
Change-Id: I1e9eb31d8386dca3feaa90ee3267ea98eb3e81ec
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299144
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39696}
Make DataChannelController's AddSctpDataStream and
RemoveSctpDataStream be required to be called on the network thread.
This moves blocking calls within those methods over to the
SctpDataChannel class instead.
For production code there's no functional change in this CL. However, this CL:
1) Introduces an actual dedicated network thread to
DataChannelController and SctpDataChannel tests.
2) Removes two data_channel_transport() checks inside DCC that
were being done on the wrong thread (signaling) and
3) introduces a network calling block to SctpDataChannel, where more
network thread related work needs to be done and can be bundled.
(to be done in follow-up CLs).
Bug: webrtc:11547
Change-Id: I6787ac395e61d4a25ae3a74a123e3357cbb46b54
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298052
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39688}
All tests do this already except for RTCStatsCollectorTest.
Bug: none
Change-Id: I318f45a2c79b3d07ca6c92902ebb4f0622ec3200
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297862
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39576}
to RTCInboundRtpStreamStats and RTCOutboundRtpStreamStats respectively
which follows the camel-casing convention used elsewhere.
The old name is kept around as an alias for a limited amount of time
to allow upgrading dependencies.
BUG=webrtc:14973
Change-Id: Ibf4e65933fd6cc2e7e89955042f6f8fb0f6c7853
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296261
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#39497}
This reverts commit 18c869bc36.
Reason for revert: Added a field trial that allows landing the code without affecting performance in prod.
This CL also incorporates subsequent CLs that also had to be reverted.
Original change's description:
> Revert "Use two MediaChannels for 2 directions."
>
> This reverts commit 8981a6fac3.
>
> Reason for revert: Quality regression detected.
>
> Original change's description:
> > Use two MediaChannels for 2 directions.
> >
> > This CL separates the two directions of MediaChannel into two separate objects that do not couple with each other.
> >
> > The notable API change is that receiver local SSRC now has to be set explicitly - before, it was done implicitly when the send-side MediaChannel had a stream added to it.
> >
> > Bug: webrtc:13931
> > Change-Id: I83c2e3c8e79f89872d5adda1bc2899f7049748b3
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/288400
> > Commit-Queue: Harald Alvestrand <hta@webrtc.org>
> > Reviewed-by: Henrik Boström <hbos@webrtc.org>
> > Cr-Commit-Position: refs/heads/main@{#39340}
>
> No-Try: true
> Bug: webrtc:13931
> Change-Id: I791997ad9eff75c3ac9cd2e4bbacf5bc6c3a3a79
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/295663
> Commit-Queue: Harald Alvestrand <hta@webrtc.org>
> Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#39445}
Bug: webrtc:13931
Change-Id: I1318910a685188e2b846c9040e1efc04c2c894ac
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296080
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39494}
DataChannelController used WeakPtr to clear outstanding references
upon destruction - except for the case of SctpDataChannel where we
had a pointer+flag for the same purpose. This change updates
SctpDataChannel and FakeDataChannelController to use a consistent
approach.
Bug: none
Change-Id: I0248471c241365a2c0de76afbb37302115650194
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/295820
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39464}
It turns out that there were several sigslot instances across data
channel, pc and stats classes that in practice only served as means
to update two counters in RTCStatsCollector. There's already a
notification path that's suitable.
This also fixes a case where the PC instance sat in the middle
of notifications from datachannels to the datachannel controller.
Bug: webrtc:11943
Change-Id: Ic60b76021584019f82085f6651230fe2fe82d465
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/295781
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39456}
This reverts commit 8981a6fac3.
Reason for revert: Quality regression detected.
Original change's description:
> Use two MediaChannels for 2 directions.
>
> This CL separates the two directions of MediaChannel into two separate objects that do not couple with each other.
>
> The notable API change is that receiver local SSRC now has to be set explicitly - before, it was done implicitly when the send-side MediaChannel had a stream added to it.
>
> Bug: webrtc:13931
> Change-Id: I83c2e3c8e79f89872d5adda1bc2899f7049748b3
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/288400
> Commit-Queue: Harald Alvestrand <hta@webrtc.org>
> Reviewed-by: Henrik Boström <hbos@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#39340}
No-Try: true
Bug: webrtc:13931
Change-Id: I791997ad9eff75c3ac9cd2e4bbacf5bc6c3a3a79
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/295663
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39445}
This CL separates the two directions of MediaChannel into two separate objects that do not couple with each other.
The notable API change is that receiver local SSRC now has to be set explicitly - before, it was done implicitly when the send-side MediaChannel had a stream added to it.
Bug: webrtc:13931
Change-Id: I83c2e3c8e79f89872d5adda1bc2899f7049748b3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/288400
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39340}
This is done by allowing implementations of AudioDeviceModule to
implement the GetStats() method. The default implementation returns
nullopt, in which case RTCAudioPlayoutStats will not be visible in the
stats.
Bug: webrtc:14653
Change-Id: I8e4aa6f1b8fcfa47a30f633d28a4013191752e20
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/290563
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Fredrik Hernqvist <fhernqvist@google.com>
Reviewed-by: Henrik Andreassson <henrika@webrtc.org>
Reviewed-by: Olga Sharonova <olka@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39115}
This is a reland of commit 81aab48878
See diff between Patch Set 1 and latest Patch Set.
The original CL broke this WPT[1] because getStats() with the receiver
as the selector stopped working in the event of unsignalled SSRCs due
to the receiver not knowing what the SSRC was.
This fix is to query media_channel_ for the unsignalled SSRC in the
event that the receiver does not know the SSRC.
[1] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/external/wpt/webrtc/simulcast/setParameters-active.https.html
Original change's description:
> Remove 'trackId' dependency in stats selector algorithm.
>
> In preparation for the deletion of deprecated 'track' stats, the
> stats selector algorithm needs to be rewritten not to use 'trackId'.
>
> This is achieved by finding RTP stats by their SSRC, as obtained via
> getParameters(). This unfortunately adds a block-invoke (in the sender
> case the block-invoke happens inside GetParametersInternal and in the
> receiver case the block-invoke is explicit at the calling place), but
> it can't be helped and it's just once per getStats() call and only if
> the selector argument is used.
>
> Bug: webrtc:14175
> Change-Id: If0e14cdbdc76d141e0042e43757970893bf32119
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/289101
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Commit-Queue: Henrik Boström <hbos@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#38981}
Bug: webrtc:14175, webrtc:14811
Change-Id: I0d16724af4efeb93d50e36dbfcc798564daff5c0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/290600
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39010}
making it clear what unit is being used.
BUG=webrtc:13756
Change-Id: I6354d35a8e02bb93a905ccf32cb0b294b4813e41
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/289460
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39008}
This reverts commit 81aab48878.
Reason for revert: external/wpt/webrtc/simulcast/setParameters-active.https.html is failing with this change
Original change's description:
> Remove 'trackId' dependency in stats selector algorithm.
>
> In preparation for the deletion of deprecated 'track' stats, the
> stats selector algorithm needs to be rewritten not to use 'trackId'.
>
> This is achieved by finding RTP stats by their SSRC, as obtained via
> getParameters(). This unfortunately adds a block-invoke (in the sender
> case the block-invoke happens inside GetParametersInternal and in the
> receiver case the block-invoke is explicit at the calling place), but
> it can't be helped and it's just once per getStats() call and only if
> the selector argument is used.
>
> Bug: webrtc:14175
> Change-Id: If0e14cdbdc76d141e0042e43757970893bf32119
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/289101
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Commit-Queue: Henrik Boström <hbos@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#38981}
Bug: webrtc:14175
Change-Id: Id1cbe892250fe88bd6db0b47269bcefa346709b4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/290502
Commit-Queue: Christoffer Jansson <jansson@google.com>
Auto-Submit: Henrik Boström <hbos@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Christoffer Jansson <jansson@google.com>
Cr-Commit-Position: refs/heads/main@{#38993}
The tests should say something about RTPs too, because what they say
about track will soon be irrelevant. A drive-by fix is to GetStatsOfType
as RTCOutboundRTPStreamStats instead of RTPStreamStats, this is because
of a limitation with using T::kType for runtime type information...
GetStatsOfType<T> does not work for Ts higher up in the stats hierarchy,
so it would always have returned an empty list even if
RTCOutboundRTPStreamStats were present. (Room for improvement here, but
that's a different issue.)
Bug: webrtc:14175
Change-Id: I0235bc0b66c52081859ee621c58249a6b6e98738
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/288583
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38984}
In preparation for the deletion of deprecated 'track' stats, the
stats selector algorithm needs to be rewritten not to use 'trackId'.
This is achieved by finding RTP stats by their SSRC, as obtained via
getParameters(). This unfortunately adds a block-invoke (in the sender
case the block-invoke happens inside GetParametersInternal and in the
receiver case the block-invoke is explicit at the calling place), but
it can't be helped and it's just once per getStats() call and only if
the selector argument is used.
Bug: webrtc:14175
Change-Id: If0e14cdbdc76d141e0042e43757970893bf32119
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/289101
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38981}
This is in the webrtc-stats spec at
https://www.w3.org/TR/webrtc-stats/#dom-rtcoutboundrtpstreamstats-scalabilitymode.
This adds the scalability mode to CodecSpecificInfo which is used to
plumb the modes for each simulcast layer.
TBR=orphis@webrtc.org
Tested: Compiled into Chrome and confirmed the scalability mode set for AV1, VP9, VP8 and H264 software encoders in chrome://webrtc-internals.
Bug: webrtc:14730
Change-Id: I71ceba8f6485a4f4a73e0856031b8d5f16f913f2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/285085
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Commit-Queue: Evan Shrubsole <eshr@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38847}
As the exposure of power efficient stats to JavaScript are limited as
to reduce the fingerprinting surface to getStats, a new RTCStatsMember
derivation, RTCLimitedStatsMember, was added in this change. This sets
the exposure criteria of the stat on the type, which keeps the size of
the RTCStatsMember class the same and allows for extension in the future
for new types of stat restrictions.
Bug: webrtc:14483
Change-Id: Ib0303050a112441ba2416fd5f004dd8be26b47ca
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/279021
Commit-Queue: Evan Shrubsole <eshr@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38576}
These have all been moved to "inbound-rtp" and now that upstream
projects have migrated we can delete the old location.
Unblocks https://crbug.com/webrtc/14175
Bug: webrtc:14521, webrtc:14524
Change-Id: Ia2bfa399d62304cc0ead0e65c340dfad20acc530
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/281183
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Auto-Submit: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38532}
These metrics were not only non-standard, but residing in the
non-standard "track" stats object that we want to delete. As per
https://github.com/w3c/webrtc-stats/issues/695#issuecomment-1259611462
these metrics are no longer needed because we already have
inbound-rtp.totalInterFrameDelay/totalSquaredInterFrameDelay which is
basically the same thing.
// mac_rel infra failures are unrelated
NOTRY=True
Bug: webrtc:14522
Change-Id: I565da42514a93f15532ba8357dd006547a5296ee
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/278100
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38509}
This information is now readily available. Let's expose it.
In practise we don't pace audio by default and the delay is ~0, however
we can tell that this metric is working as intended by setting
PacingController's pace_audio_ to true via the "WebRTC-Pacer-BlockAudio"
field trial. In this case chrome://webrtc-internals/ plots neats graphs
for audio send delay.
Bug: webrtc:10635
Change-Id: Iecfd93bb84ec61e5d54232769a9e7a500601b199
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/280523
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38483}
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 is exposing something that is already exposed in the legacy
getStats() API and is only available if the "video-timing" header
extension is used. Adding this metric here should unblock legacy
getStats() API deprecation. The follow-up to unship or standardize this
metric is tracked by https://crbug.com/webrtc/14586.
Bug: webrtc:14587
Change-Id: Ic3d45b0558d7caf4be2856a4cd95b88db312f85e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/279860
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38444}
This includes the stats dictionaries that have been made obsolete in
the spec and whose IDs are prefixed "DEPRECATED_":
- RTCMediaStreamTrackStats
- RTCMediaStreamStats
There is an ongoing experiment to unship these stats dictionaries in
Chrome (https://crbug.com/1374215). Marking then as [[deprecated]] helps
alert other dependencies that these classes are deprecated.
In the meantime, the "DEPRECATED_RTCMediaStreamTrackStats" prefix makes
it possible to use the deprecated classes.
# Unrelated infra failures
NOTRY=True
Bug: webrtc:14175, webrtc:14419
Change-Id: I498d370294058a628278e1e5b027cd12e24ad31a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/279700
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38439}
They may be non-standard, but they shouldn't be on a stats dictionary
that is deprecated (track is going away soon-ish). By moving them to
inbound-rtp they can continue to exist beyond track deprecation and
live in the right place in case we decide to standardize them later.
To help downstream projects transitions, the metrics are temporarily
available in both old and new locations. Delete of old location will
happen in a follow-up CL. TODOs added.
Bug: webrtc:14524
Change-Id: I2008060fa4ba76cde859d9144d2bb9648c7ff9af
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/278200
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38315}
These metrics were recently standardized. Part of the standardization
effort was to move them from obsolete "track" stats (on track for
deprecation and removal: https://crbug.com/webrtc/14175) into the
"inbound-rtp" stats which are not deprecated.
To ease transition for downstream projects, the metrics are temporarily
duplicated in both the old and new locations. In a follow-up CL, they
will be deleted from "track".
Bug: webrtc:14521
Change-Id: I0d9036472607a8c717ec823a458a79a49ddb80c7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/278080
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38308}
According to a pprof, creating RTCCodecStats is one of the places where
we spend the most CPU time in the event of creating hundreds of them:
https://screenshot.googleplex.com/B6QNDvvoX8dK5vk
The lifetime was recently updated so that we no longer have to risk
creating hundreds of them, here is the relevant section:
https://w3c.github.io/webrtc-stats/#codec-dict*
This allows code simplifications and the deletion of
ProduceCodecStats_n since we can now do a lazy instantiation of codec
stats at the point of being referenced.
Bug: webrtc:14444
Change-Id: I342c5bfebe6a4be0359da3ea106692c7a217779e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/275763
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38209}
Unlike the cache of the entire stats report which is time limited, this
certificate cache is valid for an unlimited amount of time, but is
cleared at ClearCachedStatsReport() which is already called on each
SLD/SRD call. Since certificates can only change by negotiation, this
cache is ensured to always be invalidated when certificates change.
Since ClearCachedStatsReport() can happen for other reasons than
certificates changing we may clear the cache more often then is
necessary, but arguably this is seldom enough that we don't have to
create a separate "ClearCertificateStats()" method. Keep it simple?
The cache specifically avoids rtc::SSLCertChain::GetStats which
trigger rtc::SSLCertificate::GetStats and rtc::Base64::EncodeFromArray.
Bug: webrtc:14458
Change-Id: I5f95a4a5eb51cc4462147270fdae7bb9fb7bc822
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/276602
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38205}
In non-BUNDLE use cases, it is possible for multiple RTP streams to have
the same SSRC (as long as the SSRC is unique within the same transport).
This CL adds support for "outbound-rtp" and "inbound-rtp" stream stats
to have the same SSRC on different transports by adding the transport to
the stats ID. This avoids multiple RTP stream stats having the same
stats ID and fixes the problem. It's a stupid use case, but it should
work.
There could still be a stats ID collision in the event of multiple
"remote-inbound-rtp" or "remote-outbound-rtp" reference the same SSRC
but on separate transports for the same reason, and would require the
same fix... but one bug at a time. Not addressed in this CL.
Bug: webrtc:14443
Change-Id: I1a2ffd79fc67c2765e6dbd1ccc6828d4e91c4589
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/275769
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38201}
To properly handle SSRC collisions in non-BUNDLE we need to change how
RTP stats IDs are generated, but that is a riskier change to be dealt
with in a separate CL.
For now, we just make sure that crashing is not a possibility during
SSRC collisions as a mitigation for https://crbug.com/1361612. This is
achieved by adding a TryAddStats() method to RTCStatsReport returning
whether successful.
Bug: chromium:1361612
Change-Id: I8577ae4c84a7c1eb3c7527e9efd8d1b0254269a3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/275766
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38197}
The spec says: "Represents the total number of connectivity check
requests sent (not including retransmissions)."
I was surprised to find candidate-pair.requestsSent wired up to
`sent_ping_requests_before_first_response`, which is the subset of
`sent_ping_requests_total` that happened when `recv_ping_responses`
was 0. This is not what the spec says.
By wiring it up to `sent_ping_requests_total` instead, the modern
getStats implementation of "requestsSent" will match the legacy
getStats implementation which is already wired up to this value.
// Unrelated bot issues
NOTRY=True
Bug: webrtc:14425
Change-Id: Ia53c9711ee7a13e596ae0eacf6066b97d9a1face
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/274174
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38025}
Ultimately, IDs should be random according to spec[1], so we shouldn't
rely on the ID to convey easily readable information. By making the IDs
shorter we reduce the overhead of string copies and make report dumps a
little bit smaller.
Drive-by: Add "DEPRECATED_" prefic to the RTCMediaStreamStats ID.
[1] https://w3c.github.io/webrtc-pc/#dom-rtcstats-id
# Examples of IDs before and after this CL #
RTCDataChannel_3
-> D3
RTCPeerConnection
-> P
RTCTransport_0_1
-> T01
RTCCodec_RTCTransport_0_1_100_minptime=10;useinbandfec=1
-> CIT01_100_minptime=10;useinbandfec=1
RTCInboundRTPAudioStream_6666
-> IA6666
RTCAudioSource_1
-> SA1
RTCOutboundRTPAudioStream_2943129392
-> OA2943129392
RTCRemoteInboundRtpAudioStream_3541280085
-> RIA3541280085
RTCIceCandidate_6cWRqicY
-> I6cWRqicY
RTCIceCandidatePair_6cWRqicY_haEcM2xD
-> CP6cWRqicY_haEcM2xD
RTCCertificate_FD1:BC:58:90:DF:E8:40:58:8D:04:91:44:93:4E:6C:52:9E:F0:14:98:AA:67:7B:8B:C8:30:C8:31:D0:84:1B:BF
-> CFD1:BC:58:90:DF:E8:40:58:8D:04:91:44:93:4E:6C:52:9E:F0:14:98:AA:67:7B:8B:C8:30:C8:31:D0:84:1B:BF
DEPRECATED_RTCMediaStreamTrack_receiver_3
-> DEPRECATED_TI3
RTCMediaStream_45a6e766-5d1a-40f9-a55c-ea8fdefcde49
-> DEPRECATED_S45a6e766-5d1a-40f9-a55c-ea8fdefcde49
Bug: webrtc:14416, webrtc:14419
Change-Id: I11f0a8b8354203fea1df1093d8864a6d47ee71e6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/273709
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37992}
The code incorrectly assumed that codecs exist on a per-mid/transceiver
basis, but codec payload types are unique on a per-transport basis and
in practise most applications use BUNDLE (single transport for the
entire PC).
This CL makes the codecs per-transport instead of per-transceiver. We
still need to iterate transceivers because codecs are exposed on a
per-transceiver basis and as shown in
https://jsfiddle.net/henbos/7kqxgnr8/ it is possible for FMTP lines to
be different on different m= sections despite BUNDLE.
Manual testing shows that this CL brings down the number of "codec"
stats in Google Meet 50p from 872 objects to 43 objects.
Bug: webrtc:14414
Change-Id: Ic854b31bd595799554b99fff22cbd48264ebd141
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/273707
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Reviewed-by: Philipp Hancke <phancke@microsoft.com>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37989}
There's no way to add a deprecation warning unique to using
RTCMediaStreamTrackStats, but we could signal to users that it is
deprecated by adding "DEPRECATED_" to its ID.
This could break apps with hardcoded assumptions about what the stats
IDs are, but apps doing this are using the API incorrectly anyway, so
if anyone is affected by this change that would be a good time to
remove any dependency on this (see https://crbug.com/webrtc/10656
regading the fact that IDs should be unpredictable).
Bug: webrtc:14175
Change-Id: I6242c4efc08e9570420c00af5aaf491b1af819f1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/269004
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37595}
This CL also removes the existing non-standard implementation of the metric.
Bug: webrtc:14147, webrtc:11789
Change-Id: I70fd1c451dfd59380fe5ce959086f37b31697c16
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/265360
Reviewed-by: Artem Titov <titovartem@webrtc.org>
Commit-Queue: Ivo Creusen <ivoc@webrtc.org>
Reviewed-by: Sam Zackrisson <saza@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Ivo Creusen <ivoc@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37441}
increasing precision since summing up rounded values leads to
a rounding error, in particular for small frames which take very
little time to decode.
BUG=webrtc:12526,webrtc:13756
Change-Id: I647c702808856a002c746ed9f115aa9bcaddc1f3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/262810
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Philipp Hancke <philipp.hancke@googlemail.com>
Cr-Commit-Position: refs/heads/main@{#37249}
This is what allowed us to remove "transceiver" stats from the spec.
Bug: webrtc:14191
Change-Id: I687a2dd97de016832005cb4271f6e1a0e0560cd3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/266022
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Philipp Hancke <philipp.hancke@googlemail.com>
Cr-Commit-Position: refs/heads/main@{#37247}
This should allow standard stats users not to have to rely on the
obsolete "track" stats.
Bug: webrtc:14174
Change-Id: I24e5e1478ee47c73c12fcdecf7314f41fcc76bc7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/266020
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37246}
This currently only exists as a goog legacy stat and has no spec
equivalent according to
https://docs.google.com/document/d/1z-D4SngG36WPiMuRvWeTMN7mWQXrf1XKZwVl3Nf1BIE/edit
Yet it is useful to debug issues sometimes. Exposing it as a
nonstandard stat will make it show up in chrome://webrtc-internals,
removing a need to switch to the legacy stats API there.
BUG=webrtc:14118
Change-Id: I506357ad54ff33df3ba46fb81558aa32187ac8e9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264420
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37055}
