Commit Graph

35 Commits

Author SHA1 Message Date
c0dad89bed Revert of Separating SCTP code from BaseChannel/MediaChannel. (patchset #14 id:240001 of https://codereview.webrtc.org/2564333002/ )
Reason for revert:
Hitting DCHECK in chromium's WebrtcTransportTest.TerminateDataChannel and WebrtcTransportTest.DataStreamLate. Will investigate and reland.

Original issue's description:
> Separating SCTP code from BaseChannel/MediaChannel.
>
> The BaseChannel code is geared around RTP; the presence of media engines,
> send and receive streams, SRTP, SDP directional attribute negotiation, etc.
> It doesn't make sense to use it for SCTP as well. This separation should make
> future work both on BaseChannel and the SCTP code paths easier.
>
> SctpDataEngine now becomes SctpTransport, and is used by WebRtcSession
> directly. cricket::DataChannel is also renamed, to RtpDataChannel, so it
> doesn't get confused with webrtc::DataChannel any more.
>
> Beyond just moving code around, some consequences of this CL:
> - We'll now stop using the worker thread for SCTP. Packets will be
>   processed right on the network thread instead.
> - The SDP directional attribute is ignored, as it's supposed to be.
>
> BUG=None
>
> Review-Url: https://codereview.webrtc.org/2564333002
> Cr-Commit-Position: refs/heads/master@{#15906}
> Committed: 67b3bbe639

TBR=pthatcher@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=None

Review-Url: https://codereview.webrtc.org/2614813003
Cr-Commit-Position: refs/heads/master@{#15908}
2017-01-05 04:28:21 +00:00
67b3bbe639 Separating SCTP code from BaseChannel/MediaChannel.
The BaseChannel code is geared around RTP; the presence of media engines,
send and receive streams, SRTP, SDP directional attribute negotiation, etc.
It doesn't make sense to use it for SCTP as well. This separation should make
future work both on BaseChannel and the SCTP code paths easier.

SctpDataEngine now becomes SctpTransport, and is used by WebRtcSession
directly. cricket::DataChannel is also renamed, to RtpDataChannel, so it
doesn't get confused with webrtc::DataChannel any more.

Beyond just moving code around, some consequences of this CL:
- We'll now stop using the worker thread for SCTP. Packets will be
  processed right on the network thread instead.
- The SDP directional attribute is ignored, as it's supposed to be.

BUG=None

Review-Url: https://codereview.webrtc.org/2564333002
Cr-Commit-Position: refs/heads/master@{#15906}
2017-01-05 02:38:02 +00:00
b4e426e8db RTCIceCandidateStats.transportId added and collected.
BUG=webrtc:6756, chromium:632723, chromium:627816

Review-Url: https://codereview.webrtc.org/2597963002
Cr-Commit-Position: refs/heads/master@{#15875}
2017-01-02 17:59:31 +00:00
6769c49418 RTC[In/Out]boundRTPStreamStats: qpSum,framesDecoded,framesEncoded added.
Recently added to the spec:
RTCRTPStreamStats.qpSum - https://w3c.github.io/webrtc-stats/#dom-rtcrtpstreamstats-qpsum
RTCInboundRTPStreamStats.framesDecoded - https://w3c.github.io/webrtc-stats/#dom-rtcinboundrtpstreamstats-framesdecoded
RTCOutboundRTPStreamStats.framesEncoded - https://w3c.github.io/webrtc-stats/#dom-rtcoutboundrtpstreamstats-framesencoded

These are added and collected. However, the qpSum is only collected in
the outbound case. It should be collected in the inbound case before
closing crbug.com/657855

BUG=chromium:657854, chromium:657855, chromium:657856

Review-Url: https://codereview.webrtc.org/2588373005
Cr-Commit-Position: refs/heads/master@{#15872}
2017-01-02 16:35:13 +00:00
06495bcbb7 RTCIceCandidatePairStats.[state/priority] added, ConnectionInfo updated.
State and priority added to ConnectionInfo. The Connection::State enum
is replaced by IceCandidatePairState enum class.

At P2PTransportChannel::GetStats, Connection::stats is called, producing
ConnectionInfo for the connection that is then filled in with additional
values from the Connection. This is refactored so that all values are
set by Connection::stats.

RTCStatsCollector is updated to surface the ConnectionInfo stats.

BUG=webrtc:6755, chromium:633550, chromium:627816

Review-Url: https://codereview.webrtc.org/2597423003
Cr-Commit-Position: refs/heads/master@{#15870}
2017-01-02 16:08:18 +00:00
f415f8ae73 Removed RTCStatsCollector::ProducePartialResultsOnWorkerThread.
No stats are collected by it, remove to reduce unnecessary thread hops.

BUG=webrtc:6875, chromium:627816

Review-Url: https://codereview.webrtc.org/2583193002
Cr-Commit-Position: refs/heads/master@{#15862}
2017-01-02 12:28:51 +00:00
02d2a92d92 RTCStatsReport::AddStats DCHECKs that the ID is unique.
Previously it was allowed to call AddStats with stats of the same ID
multiple times.

This revealed a few things:
- Local and remote streams can have the same label.
  RTCMediaStreamStats's ID is updated to include "local"/"remote".
- The same certificate can show up multiple times (e.g. for local and
  remote in a loopback), so we skip creating RTCCertificateStats for the
  same certificate multiple times

BUG=chromium:627816

Review-Url: https://codereview.webrtc.org/2593503003
Cr-Commit-Position: refs/heads/master@{#15730}
2016-12-21 09:29:05 +00:00
b78306a7d3 Fix segfault when PeerConnection is destroyed during stats collection.
RTCStatsCollector relies on PeerConnection and its WebRtcSession. If the
PeerConnection is destroyed, reference counting keeps the
RTCStatsCollector alive until the request has completed. But the request
is using PeerConnection/WebRtcSession resources that are destroyed in
~PeerConnection().

To get around this problem, RTCStatsCollector::WaitForPendingRequest()
is added, which is invoked at ~PeerConnection().

Integration test added, it caused a segmentation fault before this
change / EXPECT failure.

BUG=chromium:627816

Review-Url: https://codereview.webrtc.org/2583613003
Cr-Commit-Position: refs/heads/master@{#15674}
2016-12-19 13:06:57 +00:00
df6075a77f RTCStatsCollector: Utilize network thread to minimize thread hops.
(This is a re-upload of https://codereview.webrtc.org/2567243003/, the
CQ stopped working there.)

The previously used WebRtcSession::GetTransportStats did a synchronous
invoke per channel (voice, video, data) on the signaling thread to the
network thread - e.g. 3 blocking invokes.

It is replaced by WebRtcSession::GetStats[_s] which can be invoked on
the signaling thread or on any thread if a ChannelNamePairs argument is
present (provided by WebRtcSession::GetChannelNamePairs on the signaling
thread).

With these changes, and changes allowing the getting of certificates
from any thread, the RTCStatsCollector can turn the 3 blocking thread
invokes into 1 non-blocking invoke.

BUG=webrtc:6875, chromium:627816

Review-Url: https://codereview.webrtc.org/2583883002
Cr-Commit-Position: refs/heads/master@{#15672}
2016-12-19 12:58:02 +00:00
3168c7a04b Rename RTCOutboundRTPStreamStats *_rtt members to *_round_trip_time.
The spec renamed these recently:
https://w3c.github.io/webrtc-stats/#since-21-sep-2016*

BUG=chromium:627816

Review-Url: https://codereview.webrtc.org/2576383002
Cr-Commit-Position: refs/heads/master@{#15630}
2016-12-15 14:17:15 +00:00
e10e6d1f47 RTCOutboundRTPStreamStats.roundTripTime: Only report non-negative values.
Underlying stats gatherers may otherwise default it to -1.

BUG=chromium:669877, chromium:627816

Review-Url: https://codereview.webrtc.org/2562703007
Cr-Commit-Position: refs/heads/master@{#15625}
2016-12-15 09:54:38 +00:00
9a394f0649 Skip RTCMediaStreamTrackStats.echoReturnLoss[Enhancement] default value.
Due to the Chromium implementation[1] of GetAudioProcesssingStats,
echoReturnLoss and echoReturnLossEnhancement could default to -100 when
no value was available. This should be improved by using rtc::Optional
or AudioProcessorInterface::GetStats being able to return false, but
this requires a bunch of refactoring.

In the meantime we "blacklist" the value -100 which is a nonsense value
anyway. In that case echoReturnLoss[Enhancement] is correctly left
undefined.

[1] https://cs.chromium.org/chromium/src/content/renderer/media/media_stream_audio_processor_options.cc?sq=package:chromium&dr=C&rcl=1481530670&l=461

BUG=chromium:669877

Review-Url: https://codereview.webrtc.org/2573443002
Cr-Commit-Position: refs/heads/master@{#15611}
2016-12-14 15:58:30 +00:00
e448dd5355 RTCIceCandidatePairStats.consentRequestsSent set by RTCStatsCollector
and requestsSent is updated.

Before:
  requestsSent = total ping requests
Now
  requestsSent = pings sent before first response
  consentRequestsSent = pings after first response

Spec: https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatepairstats-consentrequestssent

BUG=chromium:633550

Review-Url: https://codereview.webrtc.org/2558293002
Cr-Commit-Position: refs/heads/master@{#15541}
2016-12-12 09:22:59 +00:00
02cd4d6dfc RTCInboundRTPStreamStats.packetsLost set by RTCStatsCollector.
Spec: https://w3c.github.io/webrtc-stats/#dom-rtcinboundrtpstreamstats-packetslost

BUG=chromium:657855

Review-Url: https://codereview.webrtc.org/2559973002
Cr-Commit-Position: refs/heads/master@{#15507}
2016-12-09 12:19:50 +00:00
d82f5125b7 RTCIceCandidatePairStats.requestsReceived defined by RTCStatsCollector.
Spec: https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatepairstats-requestsreceived

BUG=chromium:633550

Review-Url: https://codereview.webrtc.org/2565463002
Cr-Commit-Position: refs/heads/master@{#15506}
2016-12-09 12:12:45 +00:00
0583b286e4 Collecting RTCIceCandidatePairStats.transport_id and improved unittests.
RTCIceCandidatePairStats.transport_id is set to the related
RTCTransportStats' id.

Unittest for RTCIceCandidatePairStats is updated to do EXPECT_EQ
between actual and an expected hardcoded dictionary. The previous way of
testing, ExpectReportContainsCandidatePair, is removed.

(ExpectReportContainsCandidate still exist, we might want to replace
this by EXPECT_EQ testing in a follow up.)

Unittest for RTCTransportStats is similarly updated and
ExpectReportContainsTransportStats is removed. A bug was uncovered where
the "rtcp_connection_info.best_connection = true" case was not tested
(a copy of rtcp_connection_info was used in the test, modifying that had
no affect on the test) - fixed.

rtcstats_integrationtest.cc updated to take transport_id into account.

In order to reuse an updated version of expected_rt[c]p_transport in the
unittest, timestamps are ignored by RTCStats::operator==.

BUG=chromium:627816, chromium:653873, chromium:653873, webrtc:6755

Review-Url: https://codereview.webrtc.org/2527113002
Cr-Commit-Position: refs/heads/master@{#15316}
2016-11-30 09:50:36 +00:00
0adb8285b1 RTCCodecStats[1] added.
RTCStatsCollector supports "payloadType", "codec" and "clockRate".
"channels", "parameters" and "implementation" need to be supported
before closing crbug.com/659117.

[1] https://w3c.github.io/webrtc-stats/#codec-dict*

BUG=chromium:659117, chromium:627816, chromium:657854
NOTRY=True

Review-Url: https://codereview.webrtc.org/2509803004
Cr-Commit-Position: refs/heads/master@{#15207}
2016-11-23 10:32:14 +00:00
820f578f2b RTCInboundRTPStreamStats's [fir/pli/nack]_count are collected for video.
Previously this was only collected for RTCOutboundRTPStreamStats video,
with no comment saying it was missing for Inbound. (nack_count should be
collected vor audio as well but this is currently not available - there
is already an existing comment about this in rtcstats_objects.h.)

BUG=chromium:657855, chromium:657854, chromium:627816

Review-Url: https://codereview.webrtc.org/2515293002
Cr-Commit-Position: refs/heads/master@{#15185}
2016-11-22 11:16:56 +00:00
82ebe02491 Correct stats for RTCPeerConnectionStats.dataChannels[Opened/Closed].
DataChannel.SignalOpened and unittests added.
PeerConnection.SignalDataChannelCreated added and wired up to
RTCStatsCollector.OnDataChannelCreated on RTCStatsCollector
construction.
RTCStatsCollector.OnSignalOpened/Closed added and wired up on
OnDataChannelCreated.
rtcstatscollector_unittest.cc updated, faking that channels are opened
and closed.

I did not want to use DataChannelObserver because it is used for more
than state changes and there can only be one observer (unless code is
updated). Since DataChannel already had a SignalClosed it made sense to
add a SignalOpened.

Having OnSignalBlah in RTCStatsCollector is new in this CL but will
likely be needed to correctly handle RTPMediaStreamTracks being added
and detached independently of getStats. This CL establishes this
pattern.

(An integration test will be needed for this and all the other stats to
make sure everything is wired up correctly and test outside of a
mock/fake environment, but this is not news.)

BUG=chromium:636818, chromium:627816

Review-Url: https://codereview.webrtc.org/2472113002
Cr-Commit-Position: refs/heads/master@{#15059}
2016-11-14 09:41:56 +00:00
1b35d4cdf1 Don't copy loop variables when we don't have to
This was suggested by clang-tidy.

BUG=none

Review-Url: https://codereview.webrtc.org/2489693002
Cr-Commit-Position: refs/heads/master@{#15018}
2016-11-10 13:15:41 +00:00
09bc128603 RTCMediaStream[Track]Stats added.
Not all members are collected by RTCStatsCollector and detached tracks
are not visible in the returned stats. This needs to be addressed before
closing crbug.com/660827 and crbug.com/659137

BUG=chromium:660827, chromium:659137, chromium:627816

Review-Url: https://codereview.webrtc.org/2467873005
Cr-Commit-Position: refs/heads/master@{#14978}
2016-11-08 14:29:26 +00:00
eeafe94f28 RTCInboundRTPStreamStats[1] added.
Not all stats are collected in this CL, this must be addressed before
closing the issue.

[1] https://w3c.github.io/webrtc-stats/#inboundrtpstats-dict*

  Re-landed after having to be reverted
  https://codereview.webrtc.org/2470683002/ due to depending on a CL
  that was reverted. Now that that has re-landed
  https://codereview.webrtc.org/2470703002/ this is ready to re-land.

BUG=chromium:627816, chromium:657855, chromium:657854
R=hta@webrtc.org
TBR=deadbeef@webrtc.org

Review-Url: https://codereview.webrtc.org/2465173003
Cr-Commit-Position: refs/heads/master@{#14868}
2016-11-01 10:00:24 +00:00
6ded190864 RTCOutboundRTPStreamStats[1] added.
This also adds RTCRTPStreamStats[2] which it derives from. Not all stats
are supported in this CL, this must be addressed before closing the
issue.

RTCStatsReport also gets a timestamp and ToString.

[1] https://w3c.github.io/webrtc-stats/#outboundrtpstats-dict*
[2] https://w3c.github.io/webrtc-stats/#streamstats-dict*

  This was previously reverted https://codereview.webrtc.org/2465223002/
  because RTCStatsReport::Create added a new parameter not used by
  Chromium unittests. Temporarily added a default value to the argument
  to be removed after rolling and updating Chromium.

BUG=chromium:627816, chromium:657856, chromium:657854
TBR=deadbeef@webrtc.org

Review-Url: https://codereview.webrtc.org/2470703002
Cr-Commit-Position: refs/heads/master@{#14866}
2016-11-01 08:50:52 +00:00
7eaa83622b Revert of RTCOutboundRTPStreamStats added. (patchset #3 id:80001 of https://codereview.webrtc.org/2456463002/ )
Reason for revert:
Breaks Chrome FYI.
peerconnection_unittest calls RTCStatsReport::Create without  parameters.

Original issue's description:
> RTCOutboundRTPStreamStats[1] added.
>
> This also adds RTCRTPStreamStats[2] which it derives from. Not all stats
> are supported in this CL, this must be addressed before closing the
> issue.
>
> RTCStatsReport also gets a timestamp and ToString.
>
> [1] https://w3c.github.io/webrtc-stats/#outboundrtpstats-dict*
> [2] https://w3c.github.io/webrtc-stats/#streamstats-dict*
>
> BUG=chromium:627816, chromium:657856, chromium:657854
>
> Committed: https://crrev.com/69e9cb08285f6cbcab547c7a5e6aa668fa6f2d29
> Cr-Commit-Position: refs/heads/master@{#14860}

TBR=hta@webrtc.org,deadbeef@webrtc.org,hbos@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:627816, chromium:657856, chromium:657854

Review-Url: https://codereview.webrtc.org/2465223002
Cr-Commit-Position: refs/heads/master@{#14863}
2016-11-01 06:52:28 +00:00
4ed075034a Revert of RTCInboundRTPStreamStats added. (patchset #4 id:100001 of https://codereview.webrtc.org/2452043002/ )
Reason for revert:
Dependend cl Breaks Chrome FYI.
peerconnection_unittest anropar RTCStatsReport::Create without  parameters.

Original issue's description:
> RTCInboundRTPStreamStats[1] added.
>
> Not all stats are collected in this CL, this must be addressed before
> closing the issue.
>
> [1] https://w3c.github.io/webrtc-stats/#inboundrtpstats-dict*
>
> BUG=chromium:627816, chromium:657855, chromium:657854
>
> Committed: https://crrev.com/0d7bf169402ea9345d163998f4f7df89229ac470
> Cr-Commit-Position: refs/heads/master@{#14861}

TBR=hta@webrtc.org,deadbeef@webrtc.org,hbos@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:627816, chromium:657855, chromium:657854

Review-Url: https://codereview.webrtc.org/2470683002
Cr-Commit-Position: refs/heads/master@{#14862}
2016-11-01 06:51:00 +00:00
0d7bf16940 RTCInboundRTPStreamStats[1] added.
Not all stats are collected in this CL, this must be addressed before
closing the issue.

[1] https://w3c.github.io/webrtc-stats/#inboundrtpstats-dict*

BUG=chromium:627816, chromium:657855, chromium:657854

Review-Url: https://codereview.webrtc.org/2452043002
Cr-Commit-Position: refs/heads/master@{#14861}
2016-10-31 22:31:09 +00:00
69e9cb0828 RTCOutboundRTPStreamStats[1] added.
This also adds RTCRTPStreamStats[2] which it derives from. Not all stats
are supported in this CL, this must be addressed before closing the
issue.

RTCStatsReport also gets a timestamp and ToString.

[1] https://w3c.github.io/webrtc-stats/#outboundrtpstats-dict*
[2] https://w3c.github.io/webrtc-stats/#streamstats-dict*

BUG=chromium:627816, chromium:657856, chromium:657854

Review-Url: https://codereview.webrtc.org/2456463002
Cr-Commit-Position: refs/heads/master@{#14860}
2016-10-31 21:48:44 +00:00
02ba211a9f Move RTCStatsCollector helper functions to anonymous namespace.
Simple refactoring.
Moved ProduceCertificateStatsFromSSLCertificateStats_s and
ProduceIceCandidateStats_s from member section of RTCStatsCollector
to the anonymous namespace of rtcstatscollector.cc.
The thread check is removed as a result, which makes sense because
the helper function does not know about which thread its input
parameter lives on, that is up to the calling place (which has a thread
check already).

This makes rtcstatscollector.h cleaner, and all ProduceBlahStats
functions are starting points of collecting various stats. (Call all
of them and you get a complete set of stats.)

(Not moving PrepareTransportCertificateStats_s because it is using a
private struct of RTCStatsCollector.)

BUG=627816

Review-Url: https://codereview.webrtc.org/2462573002
Cr-Commit-Position: refs/heads/master@{#14818}
2016-10-28 12:14:58 +00:00
5d79a7cb1f rtcstats_objects.h updated with TODOs about stats not being collected
or not being collected correctly.

These TODOs are already documented and in greater detail in
rtcstatscollector.cc, but if every discrepency is listed in
rtcstats_objects.h it is easier to get an overview of the progress of
the new GetStats API.

BUG=chromium:627816
TBR=hta@webrtc.org
NOTRY=True

Review-Url: https://codereview.webrtc.org/2443163002
Cr-Commit-Position: refs/heads/master@{#14749}
2016-10-24 16:27:17 +00:00
2fa7c67675 RTCTransportStats[1] added, supporting all members.
Address TODO in rtcstatscollector_unittest.cc before closing 653873.

[1] https://w3c.github.io/webrtc-stats/#transportstats-dict*

BUG=chromium:653873, chromium:633550, chromium:627816

Review-Url: https://codereview.webrtc.org/2408363002
Cr-Commit-Position: refs/heads/master@{#14740}
2016-10-24 11:00:12 +00:00
cc555c5019 RTCDataChannelStats[1] added, supporting all stats members.
Also updates MockDataChannel to also mock id, messages_sent, bytes_sent,
messages_received and bytes_received.

[1] https://w3c.github.io/webrtc-stats/#dcstats-dict*

BUG=chromium:654927, chromium:627816

Review-Url: https://codereview.webrtc.org/2420473002
Cr-Commit-Position: refs/heads/master@{#14670}
2016-10-18 19:48:37 +00:00
c47a0c3ac4 RTCIceCandidatePairStats[1] added.
Note: In this initial CL most stats members are missing. This needs to
be addressed before closing the RTCIceCandidatePairStats bug
(crbug.com/633550).

[1] https://w3c.github.io/webrtc-stats/#candidatepair-dict*

BUG=chromium:633550, chromium:627816

Review-Url: https://codereview.webrtc.org/2390693003
Cr-Commit-Position: refs/heads/master@{#14604}
2016-10-11 21:54:55 +00:00
ab9f6e4dea RTCIceCandidateStats[1] added.
The RTCStatsCollector collects candidates from candidate pairs. Note
that there may be other candidates that are not paired with anything,
stats for these should also be produced before closing crbug.com/632723.

[1] https://w3c.github.io/webrtc-stats/#icecandidate-dict*

BUG=chromium:627816, chromium:632723

Review-Url: https://codereview.webrtc.org/2384143002
Cr-Commit-Position: refs/heads/master@{#14565}
2016-10-07 09:18:55 +00:00
6ab97ce0b9 RTCCertificateStats[1] added.
[1] https://w3c.github.io/webrtc-stats/#certificatestats-dict*

BUG=chromium:627816, chromium:629436
NOTRY=True

Review-Url: https://codereview.webrtc.org/2243123002
Cr-Commit-Position: refs/heads/master@{#14484}
2016-10-03 21:17:03 +00:00
74e1a4f96a PeerConnection[Interface]::GetStats(RTCStatsCollectorCallback*) added.
New file structure and targets:

rtc_stats_api
  webrtc/api/stats/rtcstats.h
  webrtc/api/stats/rtcstats_objects.h
  webrtc/api/stats/rtcstatsreport.h

rtc_stats (dep on rtc_stats_api)
  webrtc/stats/rtcstats.cc
  webrtc/stats/rtcstats_objects.cc
  webrtc/stats/rtcstatsreport.cc

libjingle_peerconnection (dep on rtc_stats)
  webrtc/api/rtcstatscollector.cc
  webrtc/api/rtcstatscollector.h

Placing rtc_stats_api headers in this separate target instead of
libjingle_peerconnection avoids a circular dependency
libjingle_peerconnection -> rtc_stats -> libjingle_peerconnection

Code changes:

PeerConnectionInterface::GetStats(RTCStatsCollectorCallback*) added for
the new stats collection API. Implemented by PeerConnection.

BUG=chromium:627816

Review-Url: https://codereview.webrtc.org/2331373004
Cr-Commit-Position: refs/heads/master@{#14246}
2016-09-16 06:33:04 +00:00