2354 Commits

Author SHA1 Message Date
Henrik Boström
d3e765e4eb Disable flaky PeerConnectionSimulcastWithMediaFlowTests on ASAN.
The test is flaking during shutdown in general, but this is especially
apparent on ASAN bots that don't retry. It is related to
heap-use-after-free and is believef to be a test-only problem.

Bug: webrtc:15018
Change-Id: I593c18dbbe45605404b17af74fd7101f118db7bb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299003
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Jeremy Leconte <jleconte@google.com>
Cr-Commit-Position: refs/heads/main@{#39661}
2023-03-23 18:21:42 +00:00
Henrik Boström
adb946054c Ship ability to opt-in to VP9/AV1 simulcast (re-land).
This makes "WebRTC-AllowDisablingLegacyScalability" enabled-by-default,
meaning any app can opt-in to spec-compliant simulcast when
scalabilityMode is specified.

The opt-in criteria is also made more restricitve: you now have to
specify both scalabilityMode and scaleResolutionDownBy to get simulcast,
otherwise you continue to get legacy "single stream" path.

The reason for this is not to cause any surprises in use cases like
[{scalabilityMode:"L1T1", active:true}, {active:false}, {active:false}]
In cases like this where scaleResolutionDownBy is not specified, it
defaults to 4:2:1 if simulcast is used but the legacy path caps it to
one stream, meaning full resolution. By restricing simulcast only to
cases that set scaleResolutionDownBy, we remove the risk of an app
getting a different resolution than expected due to opt-in.

Bug: webrtc:14884, webrtc:15005
Change-Id: I5efb87af60afaeb1e3ff76698d887aaa1f9d63a9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298922
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Evan Shrubsole <eshr@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39660}
2023-03-23 17:53:05 +00:00
Henrik Boström
138c9be310 Revert newly added test that fails on import bot.
To be investigated and re-landed.

NOTRY=True

Bug: webrtc:15028
Change-Id: I3919dd6ab37b6d17959c51cf14191287ab20cdd3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299001
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39658}
2023-03-23 15:53:29 +00:00
Henrik Boström
4baea5b07f Make VP9 simulcast behave like singlecast for single active layer cases.
Various "if streams == 1" cases are updated to "if
IsSinglecastOrAllNonFirstLayersInactive()" in order not to cause subtle
differences between VP9 {active} and VP9 {active,inactive,inactive}.

This CL also affects a line that conditionally sets
`simulcastStream[0].active = codec_active` so it seemed fitting to
improve the test coverage of "if all streams are inactive, don't send".

Bug: webrtc:15028
Change-Id: I8872dc8be0f2dfc1d8914bdba5e6433f9ba8cbfd
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298881
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39656}
2023-03-23 14:49:22 +00:00
Philipp Hancke
b396e2b159 Revert "Only serialize non-stopped RTP header extensions"
This reverts commit be03c0971863c9e6807fcbdb4175754e8242a652.
Causes regression in web projects that
1/ add a stopped-by-default extension in SRD
2/ call createAnswer
3/ munge the stopped-by-default extension back in SLD
4/ create a subsequent offer and expect the extension to be present

BUG=chromium:1051821

Change-Id: If77f77c2c0ac19625f502fb8a07facd151475807
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298744
Commit-Queue: Markus Handell <handellm@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Markus Handell <handellm@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39655}
2023-03-23 14:06:30 +00:00
Henrik Boström
80850ca477 Fix crash happening when changing from legacy to standard VP9.
Attempting to ship "WebRTC-AllowDisablingLegacyScalability" revealed a
DCHECK that happens when negotiating 3 VP9 streams prior to the
setParameters() call:
1. By default, `scalability_mode` is missing, so those 3 streams
   defaulted to legacy SVC, meaning only a single stream is used.
2. Then, setParameters() was called to make
   `encodings[0].scalability_mode = "L2T2_KEY"` and
   `encodings[1-2].active = false`. The inactive streams were just
   dummies and never expected to exist.

Without simulcast support this is OK, because both 1) and 2) are
interpreted to have a single stream. But with simulcast support, 1) is
interpreted as single stream and 2) as three streams (1 active, 2
inactive). This should be roughly the same setup, but our code treats
them differently.

The DCHECK crash was a mismatch in number of streams in one of the
layers.

The fix is to re-create the streams when the number of streams change
for this reason. The new test revealed other issues and fixes too:
- Support for multiple spatial layers (e.g. "L2T2_KEY") when multiple
  encodings exist but only one encoding is active.
- Allow inactive layers not to have a scalability mode set.

A laundry list (https://crbug.com/webrtc/15028) has been created to
update known places doing "if streams == 1" that need to do "if
active streams == 1" instead.

Credit:
  The RecreateWebRtcStream() fix is based on eshr@'s POC from
  https://webrtc-review.googlesource.com/c/src/+/298565.

Bug: webrtc:15016
Change-Id: I909a3f83a4ef53562894549ade0a870b208cec7e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298443
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Evan Shrubsole <eshr@google.com>
Cr-Commit-Position: refs/heads/main@{#39651}
2023-03-23 10:46:17 +00:00
Tommi
9a2938b24c Remove HasSctpDataChannels() and just use HasDataChannels()
Bug: none
Change-Id: I372cd386ad8ec057237889860802865836baaa0c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298821
Auto-Submit: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39650}
2023-03-23 08:38:33 +00:00
Tommi
c61eee28f3 Split up GetSctpSslRole to include an _n variant.
This allows the SslRole to be queried from the network thread which
will simplify some code paths and avoid thread hopping.

The next steps will be to remove GetSctpSslRole and only query the
DTLS role on the network thread and start combining other operations.

Bug: webrtc:11547
Change-Id: I222dc838fc5ee274a294c8d81d38b5a4ea8fea1f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298302
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39642}
2023-03-22 16:47:28 +00:00
Harald Alvestrand
041ecb87f5 New PeerConnectionFactory::CreateVideoTrack with refcounted source
Bug: webrtc:15017
Change-Id: I04c794d8959583bb4cc5c3898f4175783ec49f16
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/249363
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39635}
2023-03-22 09:10:27 +00:00
Philipp Hancke
3e224c7fe1 Clean up TURN server limit killswitch
after the limitation to 32 TURN servers shipped in M110

BUG=webrtc:13195

Change-Id: I247e5b164188751d94eb9f4fb93aadf1dd645d2a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298308
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#39634}
2023-03-22 08:21:22 +00:00
Tommi
8efaec62b1 Add thread checker to SctpSidAllocator
Also changing AllocateSid to return StreamId instead of bool.

Bug: webrtc:11547
Change-Id: I776e917300ddfdbb79e78c01ef880209ec2c5917
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298301
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39630}
2023-03-22 00:28:02 +00:00
Tommi
9296a16f9d Remove SctpDataChannel::Init()
This is a small tweak to explicitly remove this second construction
step from SctpDataChannel (async call to OnTransportReady) and move
it over to DataChannelController, which is where OnTransportReady()
is called from otherwise.

Bug: webrtc:11547
Change-Id: Ie86fa85cbb79b405248f88b47d5920c7f163dba6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297921
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39627}
2023-03-21 17:20:38 +00:00
Philipp Hancke
016bd7514d Make GetNegotiatedHeaderExtensions return all header extensions
so the size and order corresponds to the local capabilities.
The direction may differ.

BUG=chromium:1051821

Change-Id: Icf5312237b8ed137f822c9f7dd35f70a01d2df99
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298043
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#39623}
2023-03-21 14:33:41 +00:00
Tommi
1fabbac6b6 Update SctpTransportInternal to use RTCError.
This avoids a couple of layers of error code conversion, reduces
dependency on cricket error types and allows us to preserve error
information from dcsctp. Along the way remove SendDataResult.

Bug: none
Change-Id: I1ad18a8f0b2fb181745b19c49f36f270708720c0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298305
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39619}
2023-03-21 13:57:47 +00:00
Tommi
4c842224e1 Adopt StreamId in SctpDataChannelControllerInterface
Bug: webrtc:11547
Change-Id: Iea2d706228b5a533eb7fae84613462165d7c9b54
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298300
Auto-Submit: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39618}
2023-03-21 13:45:51 +00:00
Henrik Boström
00029fe97e Reduce flakiness of PeerConnectionSimulcastWithMediaFlowTests.
Prior to this CL we would EXPECT_TRUE_WAIT until
HasOutboundRtpExpectedResolutions() confirmed that we achieved the
maximum expected resolution on all simulcast layers. This was meant to
catch bugs in case the wrong layers were configured with the wrong
layer resolutions.

The problem is that if CPU or BW adaptation kicks in, all layers get
downscaled by some factor and the test may not always recover in time,
e.g. if running on slow slow bots.

This CL relaxes the expectation only to fail if the resolution
exceeds what we expect, not if they are smaller. This is not as air
tight but it should still catch most bugs of interest and reduce
flakiness.

This was reported in comment https://crbug.com/webrtc/15018#c14 but
note that this CL does not attempt to fix the other ASAN issue.

Bug: webrtc:15018
Change-Id: I3305bdade5d1626b09aa5c67217bdedb22cdd876
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298563
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Evan Shrubsole <eshr@google.com>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39615}
2023-03-21 12:18:02 +00:00
Danil Chapovalov
a2d85e4565 Use absl::string_view type as parameter for RTCError message
Bug: webrtc:13579
Change-Id: Ia9f90e6c3b008fc614d378cae4c407becfc597c9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298447
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39610}
2023-03-20 18:16:10 +00:00
Tommi
e9aa8675d7 Remove SctpDataChannelControllerInterface::ConnectDataChannel
Bug: webrtc:11547
Change-Id: I389cb641746ef892106c22fd46b8d70218b99f58
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297421
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39608}
2023-03-20 14:57:44 +00:00
Henrik Boström
75ea06f0fa Revert "Ship ability to opt-in to VP9/AV1 simulcast."
This reverts commit 75990b9a8f98ea2d597a31472fb778ec4d55f698.

Reason for revert: Breaks downstream, a use case of having three VP9
encodings, scalability mode only specified on the first layer
(L2T2_KEY) and the other two layers not having a scalability mode but
also being active=false appears to trigger a DCHECK in
call/rtp_video_sender.cc:501. More investigation needed

Original change's description:
> Ship ability to opt-in to VP9/AV1 simulcast.
>
> With this unflagging, an app can opt-in to simulcast when using multiple
> encodings by specifying RTCRtpEncodingParameters.scalabilityMode. This
> ensures backwards-compat with apps relying on 3 encodings to mean SVC
> who traditionally have not specified scalabilityMode.
>
> It fixes the spec/API bug of asking for simulcast and not getting
> simulcast. The field trial exists only as a kill-switch with a TODO to
> remove it.
>
> This ships initial support, however note that the VP9/AV1 simulcast uses
> SimulcastRateAllocator (just like VP8/H264 simulcast). This rate
> allocator uses more kbps than SvcRateAllocator. This should be revisited
> to avoid significant higher bitrates, for example when comparing VP9
> simulcast to VP9 SVC.
>
> Shipping the ability for apps to opt-in makes it easier to exercise
> these new code paths and allows initial feedback from developers, but
> due to the high bitrate (= same bitrate as VP8/H264 simulcast today)
> many apps may find that VP9 SVC is still more beneficial for BW reasons.
>
> Bug: webrtc:14884, webrtc:15005
> Change-Id: I748aae1adb47acc8a6b79b5852cff6aa47a46f5d
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298046
> Commit-Queue: Henrik Boström <hbos@webrtc.org>
> Reviewed-by: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Evan Shrubsole <eshr@google.com>
> Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#39601}

Bug: webrtc:14884, webrtc:15005
Change-Id: Ic8f77e6a2971f493d6cd8c23faecd435058a8847
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298440
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39605}
2023-03-20 12:47:21 +00:00
Tommi
934a88a460 Make verbose log statements in sctp_data_channel DLOGs
These log statements may have been useful when the initial code was
being written but now it's essentially dead code except for when
debugging while working on the code (and then, enabling the log
statements is simple).

Low-Coverage-Reason: CL modifies VERBOSE log lines that aren't currently covered.
Bug: none
Change-Id: Id9a45fe53574d39ff3feba08c596e0ac4ce294fc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297760
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39604}
2023-03-20 11:24:42 +00:00
Tommi
dc90a9c583 [DataChannelController] Don't fire events from within a loop
Removes a couple of temporary workarounds added in this CL:
https://webrtc-review.googlesource.com/c/src/+/297981

Bug: webrtc:15004
Change-Id: I93e025725be1db56528726cc9f79740e366b8a68
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298200
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39602}
2023-03-20 11:06:16 +00:00
Henrik Boström
75990b9a8f Ship ability to opt-in to VP9/AV1 simulcast.
With this unflagging, an app can opt-in to simulcast when using multiple
encodings by specifying RTCRtpEncodingParameters.scalabilityMode. This
ensures backwards-compat with apps relying on 3 encodings to mean SVC
who traditionally have not specified scalabilityMode.

It fixes the spec/API bug of asking for simulcast and not getting
simulcast. The field trial exists only as a kill-switch with a TODO to
remove it.

This ships initial support, however note that the VP9/AV1 simulcast uses
SimulcastRateAllocator (just like VP8/H264 simulcast). This rate
allocator uses more kbps than SvcRateAllocator. This should be revisited
to avoid significant higher bitrates, for example when comparing VP9
simulcast to VP9 SVC.

Shipping the ability for apps to opt-in makes it easier to exercise
these new code paths and allows initial feedback from developers, but
due to the high bitrate (= same bitrate as VP8/H264 simulcast today)
many apps may find that VP9 SVC is still more beneficial for BW reasons.

Bug: webrtc:14884, webrtc:15005
Change-Id: I748aae1adb47acc8a6b79b5852cff6aa47a46f5d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298046
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Evan Shrubsole <eshr@google.com>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39601}
2023-03-20 10:41:15 +00:00
Tommi
6e70aa5905 Delete unused peer_connection_sdp_methods target
Bug: none
Change-Id: Id911670035c517556648cb601c122798544f4b58
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298303
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39599}
2023-03-20 07:37:55 +00:00
Henrik Boström
6b233bb40e Exercise AV1 simulcast paths in tests (re-upload).
- Patch Set 1: Re-land of 297982.
- Patch Set 2: Skip test (return early) if AV1 is not available.

Original CL description (297982):

This is something we get "for free" with the
"WebRTC-AllowDisablingLegacyScalability" field trial that has been
wired up to support VP9 simulcast.

This test works and passes, however the ramp-up time is pretty bad.
- VP9 simulcast takes approximately 4 seconds to ramp up.
- VP9 SVC takes approximately 16 seconds to ramp up.
- AV1 simulcast takes approximately 22 seconds to ramp up.

A TODO is added (webrtc:15006) and the test is given extra timeout,
a full minute to get bytes flowing on all layers.

Despite ramp-up being bad, it's important to test that AV1 simulcast
is in fact working to avoid regressions due to obsolete assumptions
about which codec do or do not support simulcast. AV1 simulcast is an
opt-in feature so there is no harm in the API not being perfect yet.

Bug: webrtc:15005, webrtc:15006
Change-Id: Ie8ec9f17c709ef93525e4ea5feb7c95496062add
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298050
Reviewed-by: Jeremy Leconte <jleconte@google.com>
Auto-Submit: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Jeremy Leconte <jleconte@google.com>
Cr-Commit-Position: refs/heads/main@{#39597}
2023-03-18 12:40:12 +00:00
Henrik Boström
0892215dc0 Revert "Ensure AV1 is always available in PeerConnectionSimulcastTests."
This reverts commit 2d3b294e49027607c80766c50f1c3c8d7d4b38b9.

Reason for revert: The CL was believed to make AV1 always available
but it turned out that the import bots still failed due to not
having AV1, so it is better to use the built in factories than
to make custom test-only ones.

Original change's description:
> Ensure AV1 is always available in PeerConnectionSimulcastTests.
>
> Unblocks a WebRTC import where a bot without AV1 support would
> otherwise have been running and failing during setting codec
> preferences.
>
> # Non-chromium bots passed, no need to wait for chromium to land.
> # Want to unblock importer.
> NOTRY=True
>
> Bug: webrtc:15005
> Change-Id: I93c6a0ce5591a057c3a0ee49f6dbaef3676c0e1d
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298021
> Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
> Commit-Queue: Henrik Boström <hbos@webrtc.org>
> Reviewed-by: Jeremy Leconte <jleconte@google.com>
> Cr-Commit-Position: refs/heads/main@{#39592}

Bug: webrtc:15005
Change-Id: I8f0850852edb0d0234000b2d956e2648a9adf904
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298120
Reviewed-by: Jeremy Leconte <jleconte@google.com>
Auto-Submit: Henrik Boström <hbos@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Jeremy Leconte <jleconte@google.com>
Cr-Commit-Position: refs/heads/main@{#39596}
2023-03-18 12:37:00 +00:00
Jeremy Leconte
6a1ded02d9 Revert "Exercise AV1 simulcast paths in tests."
This reverts commit c8ab6c449c84f17fabf8da58456d396bdb5da762.

Reason for revert: new test fails to run upstream

Original change's description:
> Exercise AV1 simulcast paths in tests.
>
> This is something we get "for free" with the
> "WebRTC-AllowDisablingLegacyScalability" field trial that has been
> wired up to support VP9 simulcast.
>
> This test works and passes, however the ramp-up time is pretty bad.
> - VP9 simulcast takes approximately 4 seconds to ramp up.
> - VP9 SVC takes approximately 16 seconds to ramp up.
> - AV1 simulcast takes approximately 22 seconds to ramp up.
>
> A TODO is added (webrtc:15006) and the test is given extra timeout,
> a full minute to get bytes flowing on all layers.
>
> Despite ramp-up being bad, it's important to test that AV1 simulcast
> is in fact working to avoid regressions due to obsolete assumptions
> about which codec do or do not support simulcast. AV1 simulcast is an
> opt-in feature so there is no harm in the API not being perfect yet.
>
> Bug: webrtc:15005, webrtc:15006
> Change-Id: If0158d172647f0462bd6db802406249d93e01871
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297982
> Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
> Commit-Queue: Henrik Boström <hbos@webrtc.org>
> Reviewed-by: Erik Språng <sprang@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#39586}

Bug: webrtc:15005, webrtc:15006
Change-Id: I7da6df8bb51219e7d0acfd3b62b4ec08e25bfdc7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298049
Owners-Override: Jeremy Leconte <jleconte@webrtc.org>
Commit-Queue: Jeremy Leconte <jleconte@google.com>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#39595}
2023-03-17 19:41:44 +00:00
Henrik Boström
2d3b294e49 Ensure AV1 is always available in PeerConnectionSimulcastTests.
Unblocks a WebRTC import where a bot without AV1 support would
otherwise have been running and failing during setting codec
preferences.

# Non-chromium bots passed, no need to wait for chromium to land.
# Want to unblock importer.
NOTRY=True

Bug: webrtc:15005
Change-Id: I93c6a0ce5591a057c3a0ee49f6dbaef3676c0e1d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298021
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Jeremy Leconte <jleconte@google.com>
Cr-Commit-Position: refs/heads/main@{#39592}
2023-03-17 16:30:56 +00:00
Henrik Boström
c8ab6c449c Exercise AV1 simulcast paths in tests.
This is something we get "for free" with the
"WebRTC-AllowDisablingLegacyScalability" field trial that has been
wired up to support VP9 simulcast.

This test works and passes, however the ramp-up time is pretty bad.
- VP9 simulcast takes approximately 4 seconds to ramp up.
- VP9 SVC takes approximately 16 seconds to ramp up.
- AV1 simulcast takes approximately 22 seconds to ramp up.

A TODO is added (webrtc:15006) and the test is given extra timeout,
a full minute to get bytes flowing on all layers.

Despite ramp-up being bad, it's important to test that AV1 simulcast
is in fact working to avoid regressions due to obsolete assumptions
about which codec do or do not support simulcast. AV1 simulcast is an
opt-in feature so there is no harm in the API not being perfect yet.

Bug: webrtc:15005, webrtc:15006
Change-Id: If0158d172647f0462bd6db802406249d93e01871
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297982
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39586}
2023-03-17 09:52:02 +00:00
Tommi
ac87c8df27 Temporary fix to guard against sctp_data_channels_ being modified.
Following https://webrtc-review.googlesource.com/c/src/+/297100
it seems that sctp_data_channels_ gets modified while we're iterating
through it. This temporary fix creates a copy of the array and iterates
through the copy instead of sctp_data_channels_. A follow-up CL (or CLs)
will provide more clarity, testing and regression guards.

Bug: webrtc:15004
Change-Id: I0cb5dfb6829d36b51328875c8c9cfa392ff393a7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297981
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39584}
2023-03-16 23:22:15 +00:00
Florent Castelli
b3d424cd48 Preserve mid of sections added with AddTrack after a rollback
Since AddTrack now has an implicit init_encodings value, it will also
have a StableState saved when associating a transceiver.
That state may not have a saved mid and mline_index, and so on a
rollback, it could blindly reset the mid and mline_index of an
associated transceiver.

This is wrong, the mid and mline_index of associated transceivers
should only be updated when the StableState objects actually
have one saved.

Bug: chromium:1424238
Change-Id: I8e80a04cd072d90200ca7643de892c0ef29b1f1a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297920
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39577}
2023-03-16 11:17:26 +00:00
Tommi
faf33878ef Always require a valid controller when constructing an SctpDataChannel
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}
2023-03-16 10:19:02 +00:00
Philipp Hancke
b3e5969658 stats: use uint64_t for RTCSentRtpStreamStats.packetsSent
spec update from https://github.com/w3c/webrtc-stats/pull/744

BUG=webrtc:14989

Change-Id: I9d0adcf951501bc281054c77bb6bc03e47192523
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/295505
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@{#39575}
2023-03-16 06:46:19 +00:00
Harald Alvestrand
5da3eb0d89 Always ask for an SCTP m-section if datachannels have been used
This removes the behavior of not requesting datachannel if the first
datachannel is closed before the offer is created.

Bug: chromium:1423562
Change-Id: I90eab0f908507e65d9ee3dff51842ee6d61a8aa9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297860
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39570}
2023-03-15 21:54:21 +00:00
Tommi
df3e4caf77 Remove SctpDataChannelControllerInterface::DisconnectDataChannel
Bug: webrtc:11547
Change-Id: Ibc8c64da5289d5cb8c8a777cd0e764f71eb14fa7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297361
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Auto-Submit: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39565}
2023-03-15 15:32:31 +00:00
Tommi
4e1c9570ed Remove cricket::ReceiveDataParams
This struct only contains two member variables now and there isn't
much value added by having it.

Low-Coverage-Reason: No change in coverage, CL modifies uncovered RTC_LOG lines.
Bug: none
Change-Id: I924d450f4c8f8e49b1cfeabaebee9fd5235a90cc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297360
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39563}
2023-03-15 13:27:55 +00:00
Tommi
00264ca712 Remove remaining sigslots from DataChannelController
This includes:
* SignalDataChannelTransportWritable_s
* SignalDataChannelTransportReceivedData_s
* SignalDataChannelTransportChannelClosing_s
* Removing sigslot::has_slots<> inheritance from SctpDataChannel

Instead, we use the existing sctp_data_channels_ vector of channels
known to the DCC to deliver the callbacks.

Bug: webrtc:11943, webrtc:11547
Change-Id: I7935d7505856eedf04981b8ba665ef8419166c1d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297100
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39557}
2023-03-14 21:41:36 +00:00
Henrik Boström
9a5de95af9 Add a flag to control legacy vs spec-compliant scalability mode.
The goal of the VP9 simulcast project is that when `scalability_mode`
is set, multiple encodings are always interpreted as simulcast, even
if VP9 or AV1 is used. This CL makes this so, but only if the flag
"WebRTC-AllowDisablingLegacyScalability" is "/Enabled/". This allows us
to make "SendingThreeEncodings_VP9_Simulcast" EXPECT VP9 simulcast.

When we are ready to ship we will remove the need to use the field
trial, but before we ship this we'll want to revisit if
SvcRateAllocator can be updated to support simulcast. (Today if we use
SvcRateAllocator when VP9 simulcast is used, all encodings except the
first one get bitrate=0, causing the test to fail because media is not
flowing on all layers.) For now, a TODO is added.

Bug: webrtc:14884
Change-Id: Ie20ae748b0c0405162f3a1b015ab94956ef83dae
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297340
Reviewed-by: Evan Shrubsole <eshr@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39552}
2023-03-14 12:05:24 +00:00
Tommi
51edb56884 Remove SignalDataChannelTransportChannelClosed_s
This removes one sigslot and also simplifies the teardown procedure
of a data channel when the channel is closed by the transport.
In this case we no longer need an additional async teardown task that
releases the last remaining reference to the channel.

Bug: webrtc:11943, webrtc:11547
Change-Id: I1c170349a6cbb3cb3c5a47d284e3a3d416c92b11
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296981
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39551}
2023-03-14 10:07:22 +00:00
Philipp Hancke
22005ab39b Remove obsolete header extension API names
and update spec link.

BUG=chromium:1051821

Change-Id: I42dbe36b2299f01cb4eb8010c893623fde7472fe
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296702
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#39548}
2023-03-13 14:49:05 +00:00
Tommi
492296cc3c Remove the SctpDataChannel::config_ member variable.
Instead there are direct member variables for the various relevant
states, some weren't needed, some can be const but the `id` member
in particular needs special handling and can't be const.

For dealing with the stream id, we now have SctpSid. A class that does range validation, checks thread safety, handles the special `-1` case (for what's essentially an unsigned 16 bit int). Using a special type
for this also has the effect that range checking happens more
consistently (although I'm not modifying the structs in api/).
With upcoming steps of avoiding thread hops, the ID may need to
migrate to the network thread, which the thread checks will help with.

Along the way, update SctpSidAllocator to use flat_set instead of std::set and moving some of the sctp data channel code to the cc file
to help with more accurately tracking code coverage.

Bug: webrtc:11547
Change-Id: Iea6e7647ab8f93052044c5afbcc449115206b4e9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296444
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39539}
2023-03-12 17:28:14 +00:00
Henrik Boström
9e74e57b8f De-flake SendingThreeEncodings_VP8_Simulcast test.
The test was assuming that after all thee layers have bytesSent > 0 we
would have fully ramped up to the expected resolutions. But there are
reasons why this may not be true, such as if adaptation kicks in.

This CL attempts to de-flake by using kLongTimeoutForRampUp when
checking the resolutions as well.

// Just increasing a timeout...
NOTRY=True

Bug: webrtc:14884
Change-Id: I5ef57ec3e3cc99552c9ae32a6fdf07889ff06ee1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296883
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39534}
2023-03-10 15:26:26 +00:00
Philipp Hancke
d3289d2ec0 Reland "stats: remove RTCRtpInboundRTPStream and RTCRtpoutboundRTPStream aliases"
This is a reland of commit 9671d60925b81baefd4a0d6b05ad539fa4a782d7
after fixing more downstream dependencies

Original change's description:
> stats: remove RTCRtpInboundRTPStream and RTCRtpoutboundRTPStream aliases
>
> after upgrading downstream projects
>
> BUG=webrtc:14973
>
> Change-Id: I5df8e95a1c70b1d6078e255166c36ed01f868b6a
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296820
> Reviewed-by: Christoffer Jansson <jansson@webrtc.org>
> Reviewed-by: Henrik Boström <hbos@webrtc.org>
> Commit-Queue: Philipp Hancke <phancke@microsoft.com>
> Cr-Commit-Position: refs/heads/main@{#39526}

No-Try: True
Bug: webrtc:14973
Change-Id: I33bd99ca211a82ed77e3e8676e00256915fde168
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296881
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39533}
2023-03-10 15:22:01 +00:00
Henrik Boström
4463ff0296 Revert "stats: remove RTCRtpInboundRTPStream and RTCRtpoutboundRTPStream aliases"
This reverts commit 9671d60925b81baefd4a0d6b05ad539fa4a782d7.

Reason for revert: Breaks dependencies, will re-land after fixes

Original change's description:
> stats: remove RTCRtpInboundRTPStream and RTCRtpoutboundRTPStream aliases
>
> after upgrading downstream projects
>
> BUG=webrtc:14973
>
> Change-Id: I5df8e95a1c70b1d6078e255166c36ed01f868b6a
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296820
> Reviewed-by: Christoffer Jansson <jansson@webrtc.org>
> Reviewed-by: Henrik Boström <hbos@webrtc.org>
> Commit-Queue: Philipp Hancke <phancke@microsoft.com>
> Cr-Commit-Position: refs/heads/main@{#39526}

Bug: webrtc:14973
Change-Id: I50878526566660d9772f7c8664970eec8bd86341
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296940
Reviewed-by: Philipp Hancke <phancke@microsoft.com>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39530}
2023-03-10 13:24:32 +00:00
Tommi
2a44872da7 Remove DataChannelController::sctp_data_channels_to_free_
Instead, just use the posted task to release the reference to a
pending data channel object.

Bug: none
Change-Id: I34f0bfd604cab88587a892eaa218856c890fc907
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296767
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39527}
2023-03-10 12:43:42 +00:00
Philipp Hancke
9671d60925 stats: remove RTCRtpInboundRTPStream and RTCRtpoutboundRTPStream aliases
after upgrading downstream projects

BUG=webrtc:14973

Change-Id: I5df8e95a1c70b1d6078e255166c36ed01f868b6a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296820
Reviewed-by: Christoffer Jansson <jansson@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#39526}
2023-03-10 12:12:41 +00:00
Tommi
f21354ce0e Transition data channnel to kClosed when not connected to a transport.
If a data channel object was closed (via calling Close()) right after
construction and before attaching to a transport, it would never
transition to the `kClosed` state. This addresses that corner case,
which caused a DCHECK to trigger but might also cause a situation
whereby more than one DC instance existed for a given sctp sid.

Bug: chromium:1421534
Change-Id: Id757c0528f929f2e2daa5343236d7f62e309f6cc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296341
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39513}
2023-03-09 11:20:45 +00:00
Philipp Hancke
c8c25e5bdf Update SetHeaderExtensionsToNegotiate to match specification
following the updates from
  https://github.com/w3c/webrtc-extensions/pull/142

BUG=chromium:1051821

Change-Id: I2d561bad1ddffb412bdd7e66cf62a3cb5fc73791
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296480
Reviewed-by: Markus Handell <handellm@webrtc.org>
Reviewed-by: Markus Handell <handellm@google.com>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39500}
2023-03-08 08:46:35 +00:00
Philipp Hancke
1f98b466b8 stats: rename RTCInboundRTPStreamStats and RTCOutboundRTPStreamStats
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}
2023-03-07 14:27:47 +00:00
Harald Alvestrand
2f55370634 Reland "Use two MediaChannels for 2 directions."
This reverts commit 18c869bc36b342cd4a79947067e52a93a04a7808.

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 8981a6fac3d665beac4a58b9453e6c39988a024f.
>
> 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}
2023-03-07 12:57:35 +00:00
Harald Alvestrand
ff16f5f88e Add a new test binary to check if split-channel works
This test (and binary) will be used to verify that the field trial
for enabling split-MediaChannel works in both "off" and "on" modes,
so that it can be run as a field trial. It is intended to be deleted
once the conversion to split-MediaChannel is complete.

Bug: webrtc:13931
Change-Id: If62d19be9b2f205067b86dc859946279442fdd58
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296322
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Emil Lundmark <lndmrk@webrtc.org>
Reviewed-by: Jeremy Leconte <jleconte@google.com>
Cr-Commit-Position: refs/heads/main@{#39493}
2023-03-07 11:14:31 +00:00