Commit Graph

14 Commits

Author SHA1 Message Date
8f42992787 Move channel creation functions into RtpTransceiver
This breaks the link from sdp_offer_answer.cc to channel.h.

Bug: webrtc:13931
Change-Id: I75608f75713bf4e69013ac5f5b17c19e53d07519
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/261060
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36757}
2022-05-04 11:57:50 +00:00
8591eff520 Reland "Fix bug where we assume new m= sections will always be bundled."
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}
2021-08-11 23:36:28 +00:00
b92f9856b5 Revert "Reland "Fix bug where we assume new m= sections will always be bundled.""
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}
2021-08-06 19:53:09 +00:00
880fa8169b Reland "Use backticks not vertical bars to denote variables in comments for /pc"
Original change's description:
> Revert "Use backticks not vertical bars to denote variables in comments for /pc"
>
> 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}

Bug: webrtc:12338
Change-Id: I96bd229b73613c162d11d75fa4f5934e1b4295c7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/227087
Commit-Queue: Artem Titov <titovartem@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34611}
2021-07-30 22:13:59 +00:00
704a834f68 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}
2021-07-30 00:02:53 +00:00
dc364e5bc2 Revert "Fix bug where we assume new m= sections will always be bundled."
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}
2021-07-28 00:11:43 +00:00
fd05d6f504 Revert "Use backticks not vertical bars to denote variables in comments for /pc"
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}
2021-07-27 22:10:24 +00:00
37ee0f5e59 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}
2021-07-27 20:52:02 +00:00
d2b885fd91 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}
2021-07-23 22:08:00 +00:00
11b92cfa65 Refactoring: Move groups-by-mid into Bundle manager
Bug: webrtc:12837
Change-Id: I2431dbbf8cc291b5f3848d81cf290fd3e97ec15d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/222614
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34324}
2021-06-17 14:35:33 +00:00
de22ab2850 Apply IWYU to jsep_transport_controller/collection
Bug: None
Change-Id: I7b584099fcc26d7d74f2e08a0939a89a9196cd00
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/222612
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34323}
2021-06-17 14:01:11 +00:00
e4eb8af08f libstdc++: fix ostream operator<< usage in JsepTransportCollection
it.second in JsepTransportCollection::IsConsistent() is of type
std::unique_ptr, which has no operator<< in libstdc++. Even if it
would exist, it would just return the pointer value and not the
content.

Bug: chromium:957519
Change-Id: I17c75db47d63f42b33a551ee2b7afbf5c9a3cfde
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/222401
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34289}
2021-06-15 08:00:26 +00:00
4bb81aca75 Make JsepTransportCollection self-managing for transports
This entails instantly deleting the transport when it is no longer
referenced by any MID.

Also adds consistency checks to JsepTransportCollection.

Bug: webrtc:12837
Change-Id: I85775aeb676aac3a9aee74280cc72ac87a0f49b5
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/221982
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34273}
2021-06-11 16:12:11 +00:00
9dea393f17 Move MID/JsepTransport mappings into a new manager object.
This is part of the work to make Bundle handling understandable,
so that we can get it to work right.

Bug: webrtc:12837
Change-Id: I77f046b4bac2d9709460b3b956a2edc3df0cdaac
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/221745
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34261}
2021-06-10 07:10:47 +00:00