(using no-try due to bot infra issue)
No-try: true
Bug: b/276434297
Change-Id: I33f796b501f96731c4ca76cb62c2331f10c795f1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299708
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39734}
This is needed in order to be able to update the legacy stats
collector to fetch data channel stats from the network thread, which
is part of an upcoming change to data channels.
Bug: webrtc:11547
Change-Id: Ic205b0314b9f11a024d36d714c223cbddd0f3df3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299462
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39732}
With this cl, packets that are discarded in RtpTransport now notifies Call, so that
they can be part of BWE even if they are dropped.
These packets have been recevied on the transport, and has bin decrypted
and parsed and thus can be accounted for.
The un demuxable packets are forwarded to Call similarly how RTCP packets are forwarded.
Bug: webrtc:14928
Change-Id: Ia53349c7b316c4442a3c7aac085a85ec4f4ab9ae
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299262
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39727}
in order to not regress existing use-cases while following rules
described by the specification. This change now makes the existing
regression test pass after the spec-compliant modifications.
BUG=chromium:1051821
Change-Id: Ia384adf9a172ed88b5ec6a3cc5c478764a686cb9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299002
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@{#39726}
The tests now have two ways of addressing the channel under test:
* channel_: All SctpDataChannelTest methods. This way calls get
executed as specified by the proxy and more accurately reflects
production.
* inner_channel: For SctpDataChannel methods that are not a part of
the SctpDataChannelTest interface. Use this to invoke event handlers
etc.
Upcoming changes will include threading changes and changes to the
proxy, so it's important to cover both.
Bug: webrtc:11547
Change-Id: I26c284ece82b9a58e2b5dc4468d124d54012d959
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299264
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39725}
* Rename id_ -> id_s_, add id_n_ and thread guards.
* Same for getters, sid() -> sid_s(), add sid_n()
As more things migrate over to the network thread, we'll only need the
_n variant.
Bug: webrtc:11547
Change-Id: Ic998330f4c81b0f6833967631ac70edc2ca2301c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299141
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39724}
The reason is that:
* SDP/PeerConnection code should use HasUsedDataChannels() instead.
* HasDataChannels() touches the list of data channels on the signaling
thread, which will be a problem when it's moved to the network
thread.
* It's only needed by tests.
Bug: webrtc:11547
Change-Id: Idd47365c429e5f1d6e3812cf558c4e6fefbf733c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299481
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Auto-Submit: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39721}
...when checking if negotiation is needed.
I noticed this call site after we recently updated PeerConnection here:
https://webrtc-review.googlesource.com/c/src/+/297860
Bug: chromium:1423562
Change-Id: Id37c938d731eadfccff44c95ef757a3cabd64936
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299480
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39720}
* Change data channel creation code to return RTCError for more
detailed/accurate errors.
* Move DataChannelController::sid_allocator_ to the network thread.
* Add a temporary duplicate vector of channels on the network thread.
This will eventually be the main vector.
* Delete one test that turns out to be racy (as long as we're using
both the signaling and network threads).
Bug: webrtc:11547, webrtc:12796
Change-Id: I93ab721a09872d075046a907df60e8aee4263371
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298624
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39719}
This flag isn't needed for sctp data channels.
Bug: none
Change-Id: I07b8ba2c5186729b8a5edb4d2bba7b800335ab5d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299074
Auto-Submit: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39701}
which describe the existing behavior that necessitated the revert
b396e2b159b060791954495d68278a55e8f72092
Also change the fake media engine audio clockrate to 8000 instead
of 0 and the fake media engine video payload type to something but
0 as this value seems to be treated specially by the video engine
and is a payload type reserved for PCMU.
BUG=chromium:1051821
Change-Id: Ib0a345d59baba50a565f01685d240e41584367e6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299000
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39699}
The EncoderStreamFactory triggers different code paths depending on
`number_of_streams`: one for simulcast and one for non-simulcast.
The non-simulcast path is desired for both normal streams and SVC
streams.
The simulcast path gives sensible max bitrates for 4:2:1 scenarios, but
when encodings like {active,inactive,inactive} is specified in order to
do standard SVC, the max bps of the first encoding is so low that an
SVC stream will never send more than its first spatial layer (even when
scaleResolutionDownBy is 1).
Because of this, standard SVC is broken. This CL fixes this problem by
using the CreateDefaultVideoStreams() code path instead, which is the
same one that legacy SVC uses. With this fix, legacy and standard SVC
produce the same behavior regarding bitrate.
An added benefit of this is that numberOfSimulcastStreams == 1 in the
standard SVC path as well.
{active,inactive,inactive} tests are updated to verify the full
resolution is achieved after ramp-up. I've also confirmed that this
fixes the bug in Canary, see https://crbug.com/1428098#c2.
Bug: chromium:1428098, webrtc:15041, webrtc:15034
Change-Id: Ia1eb4ff59c4e2a56af833f7ac907a66bca8ea054
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299147
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39697}
* 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}
This reverts commit cac9a55ddf0ba33f7407f707b69d66d01c49073b.
Reason for revert: WebRTC importer consistently fails this test.
Original change's description:
> Add legacy SVC test that all layers can be inactivated.
>
> A larger version of this test was previously landed but got reverted
> due to failures only happening on the importer bot (not on the CQ or
> locally).
>
> This is a smaller version of the test that does something we should
> support: being able to inactive all encodings of a VP9 legacy SVC
> stream.
>
> Let's land and see if any issues are reproducible (expecting revert).
>
> Bug: webrtc:15033
> Change-Id: I88da1facf4ef05299f3392b86a0e3df029ebe264
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299006
> Commit-Queue: Henrik Boström <hbos@webrtc.org>
> Reviewed-by: Evan Shrubsole <eshr@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#39684}
Bug: webrtc:15033
Change-Id: Ia31f125bd6782ed38653c1e5cdcc29a8a0eff874
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299145
Reviewed-by: Andrey Logvin <landrey@webrtc.org>
Commit-Queue: Andrey Logvin <landrey@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
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@{#39686}
Update some uses of scalabilityMode to also set scaleResolutionDownBy.
Nothing changes in these tests because they were either single stream
cases, fallback was happening anyway or the test was doing VP8, but by
specifying scaleResolutionDownBy we make these tests consistent and
explicit about wanting to exercise the "standard path".
Partly a style thing and partly not wanting the test to pass for the
wrong reason.
Bug: None
Change-Id: I1d2e688976a4e6c160e90474e2416a18d795d41b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299078
Auto-Submit: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Evan Shrubsole <eshr@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39685}
A larger version of this test was previously landed but got reverted
due to failures only happening on the importer bot (not on the CQ or
locally).
This is a smaller version of the test that does something we should
support: being able to inactive all encodings of a VP9 legacy SVC
stream.
Let's land and see if any issues are reproducible (expecting revert).
Bug: webrtc:15033
Change-Id: I88da1facf4ef05299f3392b86a0e3df029ebe264
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299006
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Evan Shrubsole <eshr@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39684}
This updates DataChannelController and test classes to use
GetSctpSslRole_n instead and query the role on the network thread.
Along the way this CL makes the init config struct for when constructing
data channels, mandatory. It's now passed via const& instead of by pointer. In practice a valid pointer was always being passed.
Bug: webrtc:11547
Change-Id: I0f4bbf364969cc2dec07871c297ddbef0c175f86
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298307
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39676}
A similar test to this was landed but got reverted due to failing on the
importer bot. That is being investigated here[1]. This CL relands parts
of the original test that we believe are NOT causing issues.
This CL is not expected to fail on the importer, let's land and verify.
[1] https://webrtc-review.googlesource.com/c/src/+/299006
Bug: webrtc:15033
Change-Id: If11e459b17d3606da3ddff0742e5429c642f7e83
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299007
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Evan Shrubsole <eshr@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39666}
This CL is partly a test to see if there's an impact on binary size:
- Not a big difference for binaries (decrease): -776b to -4Kb
- For libraries (libwebrtc.a) it actually increases the size: +40Kb
Secondarily this CL is basically to introduce this pattern to the
code base. In terms of LOC, this makes things slightly more compact.
From:
class Foo {
public:
Foo() {
checker_.Detach();
}
private:
SequenceChecker checker_;
};
To:
class Foo {
public:
Foo() = default;
private:
SequenceChecker checker_{SequenceChecker::kDetached};
};
Bug: none
Change-Id: I59fc34ccea10847e13455a349851ce9a0af458e3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299020
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39664}
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}
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}
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}
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}
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}
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}
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}
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}
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}
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}
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}
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}
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}
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}
- 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}
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}
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}
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}
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}
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}
