This was a side effect of testing out the "gn_check_autofix.py" tool
after running "apply-iwyu -r" on a few files.
Seems worth committing.
Bug: none
Change-Id: I3df446c640d4c4e3d6b15eddbdf66a1a40135f69
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/258024
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36446}
This cl/
1) move WebRtcKeyValueConfig from api/transport to api/ directory.
2) add a test/ScopedKeyValueConfig (compare ScopedFieldTrials).
3) removes usage of webrtc::field_trial:: from the pc/ directory.
4) removes a few unused includes of system_wrappers/field_trial.h.
Bug: webrtc:10335
Change-Id: If29c07900dbe791050b0a5ad05332bedfad035f2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/253903
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36160}
This flag was used as a kill switch in case turning off payload type
demuxing in some Unified Plan cases (https://crbug.com/webrtc/12814)
would cause any issues. That landed way back in M93 and no issues were
ever reported, so time to clean up the flag.
Bug: webrtc:12814
Change-Id: I1970936131384dc0be1cd118e6b0ac877b8c289c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/253240
Auto-Submit: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36109}
Also deleted iwyu script that was not maintained, and deleted
some options that made the script more complex.
Bug: none
Change-Id: I39d8eaa37f12c72ddc127ae145e6a3a80f328316
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/251384
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35966}
A pointer to the transport controller is now maintained on
both the network thread and the signaling thread. We use
thread specific accessors to make it explicit which copy we
are accessing at any given time.
We also move the initial offerer value to the SDP offer/answer
class; this is determined on the basis of SDP offer/answer, so
there is no need to hop to the network thread for that.
Work in progress.
Bug: webrtc:9987
Change-Id: Idbe5a7fbf44f667adcd119e486133cf6e43ab1f5
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/251382
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35965}
This removes a couple of methods from the PeerConnectionSdpMethods
interface.
Bug: webrtc:11995
Change-Id: I0a68178b1f0a99e779e6d7f94d03b493d811f500
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/249794
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35841}
This CL changes the SdpOfferAnswerHandler class to depend on a new class
PerConnectionInternalMethods, which is implemented by PeerConnection.
This means that SdpOfferAnswerHandler no longer depends on
PeerConnectionInterface.
This opens the way for refactoring PeerConnection so that
PeerConnectionInternalMethods is a member object (encapsulation not
inheritance), which will make it possible to break some of the
dependency cycles that make the "peerconnection" target in the BUILD
file so huge.
Bug: webrtc:11995
Change-Id: Ib8413a31c0148b8d8602764b7367dfd3068da58a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/249785
Reviewed-by: Niels Moller <nisse@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35828}
Move deletion of channel objects over to the RtpTransceiver instead
of having it done by SdpOfferAnswer.
The deletion is now also done via PostTask rather than Invoke.
Bug: webrtc:11992, webrtc:13540
Change-Id: I5aff14956d5e572ca8816bbfef8739bb609b4484
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/248170
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35798}
The interface is implemented by the ChannelManager and contains methods
to create and destroy media channel objects as used by a transceiver.
This will subsequently allow us to delete the channel objects from
the transceiver class where ownership really lies rather than from
the outside - which is currently required by some tests that keep
channel objects on the stack. We'll furthermore be able to do the
destruction asynchronously without additional Invoke()s as we do now
which will remove an Invoke when making sdp changes.
With introducing the interface, the following simplifications were made:
* ChannelManager constructed on the signaling thread.
Before, there was an Invoke in the context class, which existed
for the purposes of calling MediaEngine::Init() (which in turn is
only needed for the VoiceEngine). This Invoke has now been moved
into the CM (more tbd).
* The CM now has a pointer to the signaling thread (since that's the
construction thread). That allows us to remove the signaling thread
parameter from the CreateFooChannel methods.
* The ssrc_generator (UniqueRandomIdGenerator) instance for SSRCs moved
from SdpOfferAnswerHandler to the CM, as it's always used in
combination with the CM. This simplifies the CreateFooChannel methods
as well as a couple of other classes that have a CM dependency.
* Removed DestroyFooChannel related code from SdpOfferAnswerHandler since
the channel type detail can be taken care of by the CM.
Bug: webrtc:11992, webrtc:13540
Change-Id: I04938a803734de8489ba31e6212d9eaecc244126
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/247904
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35766}
...and one when destroying a channel object.
This CL removes Init_n() and Deinit_n() from the BaseChannel class.
Channel classes now use SetRtpTransport to do initialization and
uninitialization on the network thread.
Notably if an implementation has called SetRtpTransport() with a valid
transport pointer, it is required that SetRtpTransport be called again
with a nullptr before the channel object can be deleted.
In situations where multiple channels are created, this can mean
a substantial reduction in thread hops. We still hop to the worker
in order to construct the objects - this can probably be avoided
and SetChannel() is still a synchronous operation for the transceivers.
Furthermore, teardown of channel objects also still happens
synchronously and across network/worker/signaling threads.
Bug: webrtc:11992
Change-Id: I68ca7596e181fc82996e3e290733d97381aa5e78
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/246740
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35738}
This is a followup to [1] that moves parts of the SetRemoteDescription
operation into a subclass of SdpOfferAnswerHandler.
[1] https://webrtc-review.googlesource.com/c/src/+/244980/
Bug: webrtc:13540
Change-Id: Ic5d97f9bfd30763f3988f2f6832703ffb819a51d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/245641
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35714}
Remove Init_w and Deinit(), both of which were wrappers around Invoke()
calls from the worker thread to the network thread.
Instead, replace them with Init_n() and Deinit_n() that are currently*
required to be called by external code in order to associate/disassociate
the channels with the transport.
This CL mostly moves things around in order to prepare for upcoming
changes, but it does change channel destruction in the following way:
- When destroying channels, we don't block the worker thread anymore
while uninitialization happens on the network thread. Previously
both signal and worker threads were blocked during the
uninitialization in the ChannelManager.
* In an upcoming CL, Init_n() and Deinit_n() will be called internally
from a different method that's always called on the network thread
when a channel is associated/disassociated with a transceiver. When
we're there, we will have removed several invokes that currently are
a part of constructing/destructing channel objects.
Bug: webrtc:11992
Change-Id: Ibc30447a40749ceb36d37834b0cfc5c5ea60e895
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/246502
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35707}
This is an operation specific subclass of SdpOfferAnswerHandler that in
this first step, takes over the implementation details that before this
CL were implemented in SdpOfferAnswerHandler::DoSetRemoteDescription.
This CL does not change the behavior of the implementation but it does
break up DoSetRemoteDescription into smaller methods and moves the state
related to the SRD operation, into a class that in upcoming steps can
be passed around asynchronously as needed, which will allow us to avoid
blocking threads.
Bug: webrtc:13540
Change-Id: Id2002d2390a4a13725f5967df5b82064b37c7490
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/244980
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35669}
This changes SetLocalDescription/SetRemoteDescription to just resetting
the internal cache timestamp for the legacy stats handler instead of
performing a full update, which can be costly.
Bug: webrtc:13557
Change-Id: I93971dbd7abf33c0d0f2836f9c17ba4550f41a00
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/245645
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35661}
This part is specific to unified plan and doesn't need most of
the state related to the remote description (and doesn't return an
error).
Bug: none
Change-Id: I0de66bdb2e925072a6d9010e4444e75d4574ae04
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/245102
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Auto-Submit: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35642}
This is just a step to reduce the size of ApplyRemoteDescription to make
refactoring it easier (and ultimately support async operations).
Bug: none
Change-Id: Idb950c35f585a887d6640278b6edfdd0c7cec3fa
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/245101
Auto-Submit: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35641}
This is a slight refactoring while doing some other changes, so not
strictly necessary, but the error param is always supplied in practice
so it made sense to update the tests to reflect that, test that error
values are reported in (at least) some cases and remove the additional
code that checks for whether or not error information is requested.
Bug: none
Change-Id: Ia5739a18ea2beb6970eabf9d809c24dfa43466b1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/244097
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35632}
This simplifies the work that happens on the worker thread in
preparation of avoiding having to go to the worker at all.
Bug: webrtc:11993
Change-Id: I13f063bdecce8efdb978ef1976c819019f30e020
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/244082
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@{#35610}
Add implementation of RTC_DCHECK_NOTREACHED equal to the RTC_NOTREACHED.
The new macros will replace the old one when old one's usage will be
removed. The idea of the renaming to provide a clear signal that this
is debug build only macros and will be stripped in the production build.
Bug: webrtc:9065
Change-Id: I4c35d8b03e74a4b3fd1ae75dba2f9c05643101db
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/237802
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Artem Titov <titovartem@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35348}
which is the maxium length allowed by one-byte header extensions
BUG=webrtc:12517
Change-Id: I003105d3566a34b5b7affb84ffe69b7705973ee3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/237400
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Philipp Hancke <philipp.hancke@googlemail.com>
Cr-Commit-Position: refs/heads/main@{#35333}
Removes the ability to accept nonencrypted answers to encrypted offers.
Fixes some logic around bundled sessions and requirement for
transport parameters.
Bug: webrtc:11066
Change-Id: I56d8628d223614918a1e5260fdb8a117c8c02dbd
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/236344
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35298}
Changes one preexisting enum-to-string function to use the new format.
Also changes the RTC_LOG macros that created collisions with ToString,
for tidiness, and documents the recommended function form.
Bug: webrtc:13272
Change-Id: Ic8bb54ed31402ba32675b142d796cf276ee78df5
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/235722
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35296}
This makes it possible to invoke methods on the transceiver object
from any thread.
Also makes a few of the mock observer objects thread-safe, to allow
testing when the main thread is not the signaling thread.
Bug: webrtc:13183
Change-Id: Ic97efef71a21c3075700a028103061032f8d2bcc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/232120
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35010}
Also change return type of FinalRefCountedObject::Release() to
RefCountReleaseStatus, for consistency with other refcount classes.
Bug: webrtc:12701
Change-Id: I37c325e78ba7ae3e220b618da02cb243604ca4cc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/229590
Reviewed-by: Tommi <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#34849}
This is a reland of commit 704a834f685eb96c9fcf891ca345557bef4d138a,
after it was reverted in order to merge a CL to M93.
Original change's description:
> Fix bug where we assume new m= sections will always be bundled.
>
> A recent change [1] assumes that all new m= sections will share the
> first BUNDLE group (if one already exists), which avoids generating
> ICE candidates that are ultimately unnecessary. This is fine for JSEP
> endpoints, but it breaks the following scenarios for non-JSEP endpoints:
>
> * Remote offer adding a new m= section that's not part of any BUNDLE
> group.
> * Remote offer adding an m= section to the second BUNDLE group.
>
> The latter is specifically problematic for any application that wants
> to bundle all audio streams in one group and all video streams in
> another group when using Unified Plan SDP, to replicate the behavior of
> using Plan B without bundling. It may try to add a video stream only
> for WebRTC to bundle it with audio.
>
> This is fixed by doing some minor re-factoring, having BundleManager
> update the bundle groups at offer time.
>
> Also:
> * Added some additional validation for multiple bundle groups in a
> subsequent offer, since that now becomes relevant.
> * Improved rollback support, because now rolling back an offer may need
> to not only remove mid->transport mappings but alter them.
>
> [1]: https://webrtc-review.googlesource.com/c/src/+/221601
>
> Bug: webrtc:12906, webrtc:12999
> Change-Id: I4c6e7020c0be33a782d3608dee88e4e2fceb1be1
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/225642
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Reviewed-by: Henrik Boström <hbos@webrtc.org>
> Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#34544}
Bug: webrtc:12906, webrtc:12999
Change-Id: Id6acab2e2d7430c65f4b6a1d7372388a70cc18ab
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/228465
Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34728}
This reverts commit 704a834f685eb96c9fcf891ca345557bef4d138a.
Reason for revert: Reverting this in order to revert
https://webrtc-review.googlesource.com/c/src/+/221601, so we can
merge that revert to M93.
Original change's description:
> Reland "Fix bug where we assume new m= sections will always be bundled."
>
> This is a reland of d2b885fd91909f1b17fb11292a8c989d5d883b22, after
> making sure transports that are just being kept alive in case of
> rollback don't contribute to connection state, which broke a WPT.
>
> Original change's description:
> > Fix bug where we assume new m= sections will always be bundled.
> >
> > A recent change [1] assumes that all new m= sections will share the
> > first BUNDLE group (if one already exists), which avoids generating
> > ICE candidates that are ultimately unnecessary. This is fine for JSEP
> > endpoints, but it breaks the following scenarios for non-JSEP endpoints:
> >
> > * Remote offer adding a new m= section that's not part of any BUNDLE
> > group.
> > * Remote offer adding an m= section to the second BUNDLE group.
> >
> > The latter is specifically problematic for any application that wants
> > to bundle all audio streams in one group and all video streams in
> > another group when using Unified Plan SDP, to replicate the behavior of
> > using Plan B without bundling. It may try to add a video stream only
> > for WebRTC to bundle it with audio.
> >
> > This is fixed by doing some minor re-factoring, having BundleManager
> > update the bundle groups at offer time.
> >
> > Also:
> > * Added some additional validation for multiple bundle groups in a
> > subsequent offer, since that now becomes relevant.
> > * Improved rollback support, because now rolling back an offer may need
> > to not only remove mid->transport mappings but alter them.
> >
> > [1]: https://webrtc-review.googlesource.com/c/src/+/221601
> >
> > Bug: webrtc:12906, webrtc:12999
> > Change-Id: I4c6e7020c0be33a782d3608dee88e4e2fceb1be1
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/225642
> > Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> > Reviewed-by: Henrik Boström <hbos@webrtc.org>
> > Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#34544}
>
> Bug: webrtc:12906, webrtc:12999
> Change-Id: I68bf988b1918dd2d51de76e53e4fd696fea5a09b
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/227120
> Reviewed-by: Henrik Boström <hbos@webrtc.org>
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#34596}
TBR=hta@webrtc.org
Bug: webrtc:12906, webrtc:12999
Change-Id: I129d9eb3b9831317fa24b0263db191027246cb99
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/227821
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34666}
This is a reland of d2b885fd91909f1b17fb11292a8c989d5d883b22, after
making sure transports that are just being kept alive in case of
rollback don't contribute to connection state, which broke a WPT.
Original change's description:
> Fix bug where we assume new m= sections will always be bundled.
>
> A recent change [1] assumes that all new m= sections will share the
> first BUNDLE group (if one already exists), which avoids generating
> ICE candidates that are ultimately unnecessary. This is fine for JSEP
> endpoints, but it breaks the following scenarios for non-JSEP endpoints:
>
> * Remote offer adding a new m= section that's not part of any BUNDLE
> group.
> * Remote offer adding an m= section to the second BUNDLE group.
>
> The latter is specifically problematic for any application that wants
> to bundle all audio streams in one group and all video streams in
> another group when using Unified Plan SDP, to replicate the behavior of
> using Plan B without bundling. It may try to add a video stream only
> for WebRTC to bundle it with audio.
>
> This is fixed by doing some minor re-factoring, having BundleManager
> update the bundle groups at offer time.
>
> Also:
> * Added some additional validation for multiple bundle groups in a
> subsequent offer, since that now becomes relevant.
> * Improved rollback support, because now rolling back an offer may need
> to not only remove mid->transport mappings but alter them.
>
> [1]: https://webrtc-review.googlesource.com/c/src/+/221601
>
> Bug: webrtc:12906, webrtc:12999
> Change-Id: I4c6e7020c0be33a782d3608dee88e4e2fceb1be1
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/225642
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Reviewed-by: Henrik Boström <hbos@webrtc.org>
> Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#34544}
Bug: webrtc:12906, webrtc:12999
Change-Id: I68bf988b1918dd2d51de76e53e4fd696fea5a09b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/227120
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34596}
This reverts commit d2b885fd91909f1b17fb11292a8c989d5d883b22.
Reason for revert: Speculative revert for Chromium importer
Original change's description:
> Fix bug where we assume new m= sections will always be bundled.
>
> A recent change [1] assumes that all new m= sections will share the
> first BUNDLE group (if one already exists), which avoids generating
> ICE candidates that are ultimately unnecessary. This is fine for JSEP
> endpoints, but it breaks the following scenarios for non-JSEP endpoints:
>
> * Remote offer adding a new m= section that's not part of any BUNDLE
> group.
> * Remote offer adding an m= section to the second BUNDLE group.
>
> The latter is specifically problematic for any application that wants
> to bundle all audio streams in one group and all video streams in
> another group when using Unified Plan SDP, to replicate the behavior of
> using Plan B without bundling. It may try to add a video stream only
> for WebRTC to bundle it with audio.
>
> This is fixed by doing some minor re-factoring, having BundleManager
> update the bundle groups at offer time.
>
> Also:
> * Added some additional validation for multiple bundle groups in a
> subsequent offer, since that now becomes relevant.
> * Improved rollback support, because now rolling back an offer may need
> to not only remove mid->transport mappings but alter them.
>
> [1]: https://webrtc-review.googlesource.com/c/src/+/221601
>
> Bug: webrtc:12906, webrtc:12999
> Change-Id: I4c6e7020c0be33a782d3608dee88e4e2fceb1be1
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/225642
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Reviewed-by: Henrik Boström <hbos@webrtc.org>
> Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#34544}
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: webrtc:12906, webrtc:12999
Change-Id: I00179d7573f322ad539ff16cad1f85320cfb2270
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/227081
Reviewed-by: Björn Terelius <terelius@google.com>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34578}
This reverts commit 37ee0f5e594dd772ec6d620b5e5ea8a751b684f0.
Reason for revert: Revert in order to be able to revert https://webrtc-review.googlesource.com/c/src/+/225642
Original change's description:
> Use backticks not vertical bars to denote variables in comments for /pc
>
> Bug: webrtc:12338
> Change-Id: I88cf10afa5fc810b95d2a585ab2e895dcc163b63
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/226953
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Commit-Queue: Artem Titov <titovartem@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#34575}
TBR=hta@webrtc.org,titovartem@webrtc.org,webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com
Change-Id: I5eddd3a14e1f664bf831e5c294fbc4de5f6a88af
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:12338
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/227082
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Commit-Queue: Björn Terelius <terelius@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34577}
A recent change [1] assumes that all new m= sections will share the
first BUNDLE group (if one already exists), which avoids generating
ICE candidates that are ultimately unnecessary. This is fine for JSEP
endpoints, but it breaks the following scenarios for non-JSEP endpoints:
* Remote offer adding a new m= section that's not part of any BUNDLE
group.
* Remote offer adding an m= section to the second BUNDLE group.
The latter is specifically problematic for any application that wants
to bundle all audio streams in one group and all video streams in
another group when using Unified Plan SDP, to replicate the behavior of
using Plan B without bundling. It may try to add a video stream only
for WebRTC to bundle it with audio.
This is fixed by doing some minor re-factoring, having BundleManager
update the bundle groups at offer time.
Also:
* Added some additional validation for multiple bundle groups in a
subsequent offer, since that now becomes relevant.
* Improved rollback support, because now rolling back an offer may need
to not only remove mid->transport mappings but alter them.
[1]: https://webrtc-review.googlesource.com/c/src/+/221601
Bug: webrtc:12906, webrtc:12999
Change-Id: I4c6e7020c0be33a782d3608dee88e4e2fceb1be1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/225642
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34544}
When the transport is terminated, if an error has occured, it will
be propagated to the channels.
When such errors can happen at the SCTP level (e.g. out of resources),
RTCError may contain an error code matching the definition at
https://www.iana.org/assignments/sctp-parameters/sctp-parameters.xhtml#sctp-parameters-24
If the m= line is rejected or removed from SDP, an error will again be sent
to the data channels, signaling their unexpected transition to closed.
Bug: webrtc:12904
Change-Id: Iea3d8aba0a57bbedb5d03f0fb6f7aba292e92fe8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/223541
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34386}
We want to turn off PT based demux because SSRC-based endpoints that
send media prematurely (which is a popular non-standard behavior still
heavily in use) can otherwise get incorrect mappings and unsignalled
ssrc issues because of the PT demux path.
This CL disables PT based demuxing when the MID header extension is
present on all m= sections in the SDP for that kind (audio/video), not
caring if it was in the offer or answer. However if PT demuxing has been
used in the past then it is always allowed. This ensures PT is off by
default but that either offer or answer can enable PT and once it has
been on it is also possible to get early media with PT.
- Want PT-based demux? The MID header extension has to be removed in
either the offer or the answer. Follow-up O/As allow PT demuxing if
possible.
- Want to use MID or SSRC demuxing? Great, you don't need PT-based demux
and won't mind that we turned it off for you.
The reason for disabling PT demux at offer time (if MID is present)
instead of waiting for the SDP answer is because by the time the SDP
answer arrives, early media could have triggered PT demux and caused
incorrect mappings. The safe thing is to assume a spec-compliant
endpoint until proven otherwise.
However if PT demux is ever enabled, then from that point on we always
allow PT-based demux in follow-up O/A exchanges. This ensures we don't
drop packets in follow-up exchanges. The fact that PT-based demux is
disabled during the initial offer should not matter because before the
initial O/A exchange we don't have fingerprints.
This change only affects Unified Plan and bundled groups. Existing test
coverage ensuring we do not break legacy endpoints:
[1] https://source.chromium.org/chromium/chromium/src/+/main:third_party/webrtc/pc/peer_connection_integrationtest.cc;l=1156
[2] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/external/wpt/webrtc/protocol/rtp-demuxing.html;l=59
UnsignaledStreamTest is also updated to test the interesting setups.
A kill-switch is added in case we want to disable this change.
Bug: webrtc:12814
Change-Id: I807a82a543325753633aaef698e06cb4c9dfebaa
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/221101
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34251}
It was found from Chrome tracing that worker packet progression in
https://webrtc.github.io/samples/src/content/peerconnection/negotiate-timing/
during renegotiation of 100 transceivers is hindered by a multi-hundred
millisecond Invoke from the signaling to the worker thread. This
causes audio impairment.
Fix this by splitting the single Invoke into a series of Invokes,
allowing packets received during the renegotiation to be processed
between the worker invocations.
Experimental data of negotiation from 1 to 100 video transceivers
WebRtcDistinctWorkerThread OFF, before change:
4415.60 milliseconds, audio impairment 29760
4216.00 milliseconds, audio impairment 25560
4298.40 milliseconds, audio impairment 25440
WebRtcDistinctWorkerThread OFF, after change:
4258.70 milliseconds, audio impairment 26280
4255.50 milliseconds, audio impairment 25920
4363.10 milliseconds, audio impairment 25200
WebRtcDistinctWorkerThread ON, before change:
4407.80 milliseconds, audio impairment 24840
4541.00 milliseconds, audio impairment 26160
4377.80 milliseconds, audio impairment 17040
WebRtcDistinctWorkerThread ON, after change:
4364.80 milliseconds, audio impairment 0
4174.30 milliseconds, audio impairment 0
4309.00 milliseconds, audio impairment 0
We should reconsider this split after lazy decoders and decoder stream
projects have shipped, see
- bugs.webrtc.org/12462
- crbug.com/1157227
- crbug.com/1187289
Bug: webrtc:12840, webrtc:12462, chromium:1157227, chromium:1187289
Change-Id: I8e3b3943bd76f09da74b457690799415335b51f5
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/221103
Commit-Queue: Markus Handell <handellm@webrtc.org>
Reviewed-by: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34202}
Reland of commit a743303211b89bbcf4cea438ee797bbbc7b59e80
Previously, RTP header extensions with encryption had been filtered
if the encryption had been activated (not the other way around) which
was likely an unintended logic inversion.
In addition, it ensures that encrypted RTP header extensions are only
negotiated if RTP header extension encryption is turned on. Formerly,
which extensions had been negotiated depended on the order in which
they were inserted, regardless of whether or not header encryption was
actually enabled, leading to no extensions being sent on the wire.
Further changes:
- If RTP header encryption enabled, prefer encrypted extensions over
non-encrypted extensions
- Add most extensions to list of extensions supported for encryption
- Discard encrypted extensions in a session description in case encryption
is not supported for that extension
- Mark FindHeaderExtensionByUri without filter argument as deprecated
Bug: webrtc:11713
Change-Id: I52a5ade1b94bc01d1c2a35cb56023684fcaf9982
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/219081
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34129}
The number of invokes will vary based on number of receivers/senders,
so we can't have a fixed number there.
Bug: none
Change-Id: Iab3d529a5935c4b6cc95e9da6637acd880614972
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/219101
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34027}
The channel classes have stored the negotiated headers but a more
natural place to store them is in the RtpTransceiver class where
RtpHeaderExtension state is managed as well as the implementation of
the only method that depends on the stored state,
HeaderExtensionsNegotiated().
Also adding a TODO for further improvements where we're unnecessarily
storing state in the channel classes for the purposes of the transports.
Bug: webrtc:12726
Change-Id: If36668e3e49782ddeada23ebed126ee2c4935b8c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/216691
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33917}
This changes the notification to a single std::function pointer
instead of being a sigslot::signal1<> collection.
Summary:
* Remove SignalFirstPacketReceived_, the last sigslot member variable.
(still inherits from sigslot::has_slots<>)
* BaseChannel doesn't post to the signaling thread anymore. The only
reason that remains for the signaling_thread_ variable, is for
thread checking.
* Remove BaseChannel's reliance on MessageHandlerAutoCleanup
(still inherits from MessageHandler)
RtpTransceiver is the consumer of this event. That class is also the
class that sits between the PC classes and the channel object, holding
a pointer to the channel and managing calls that come in on the
signaling thread, such as SetChannel. The responsibility of delivering
the first packet received on the signaling thread is now with
RtpTransceiver:
* RtpTransceiver always requires a ChannelManager instance. Previously
this variable was sometimes set, but it's now required.
* Updated tests in rtp_transceiver_unittest.cc to include a
ChannelManager as well as fix them to include call expectations for
mock sender and receivers.
Bug: webrtc:11993, webrtc:11988
Change-Id: If49d6be157cd7599fa6fe3a42cd0a363464e3a74
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/215979
Commit-Queue: Tommi <tommi@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33853}
In this CL, JsepTransportController and MediaSessionDescriptionFactory
are updated not to assume that there only exists at most a single BUNDLE
group but a list of N groups. This makes it possible to create multiple
BUNDLE groups by having multiple "a=group:BUNDLE" lines in the SDP.
This makes it possible to have some m= sections in one group and some
other m= sections in another group. For example, you could group all
audio m= sections in one group and all video m= sections in another
group. This enables "send all audio tracks on one transport and all
video tracks on another transport" in Unified Plan. This is something
that was possible in Plan B because all ssrcs in the same m= section
were implicitly bundled together forming a group of audio m= section and
video m= section (even without use of the BUNDLE tag).
PeerConnection will never create multiple BUNDLE groups by default, but
upon setting SDP with multiple BUNDLE groups the PeerConnection will
accept them if configured to accept BUNDLE. This makes it possible to
accept an SFU's BUNDLE offer without having to SDP munge the answer.
C++ unit tests are added. This fix has also been verified manually on:
https://jsfiddle.net/henbos/to89L6ce/43/
Without fix: 0+2 get bundled, 1+3 don't get bundled.
With fix: 0+2 get bundled in first group, 1+3 get bundled in second
group.
Bug: webrtc:10208
Change-Id: Iaf451fa5459c484730c8018274166ef154b19af8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/214487
Reviewed-by: Taylor <deadbeef@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33838}
