Skip to content
This repository has been archived by the owner on Mar 22, 2022. It is now read-only.

Init/close PeerConnection on enable/disable #430

Merged
merged 12 commits into from
Jul 20, 2020

Conversation

fibann
Copy link
Member

@fibann fibann commented Jun 25, 2020

Ensure proper shutdown across MediaLines & co.

Removed AutoInitializeOnStart, and made Initialize/Uninitialize private,
now PC should be turned on/off by enabling/disabling.

We now create/keep local tracks only when there are both a live source and a
connection. Therefore, local tracks do not persist after the PeerConnection
component is disabled. This is intended, as we cannot keep the track and
attach it to a different PeerConnection if the component is re-enabled.

Removed listeners to tracks removed. Unpairing is handled manually on
renegotiation and on shutdown (MediaLine.UnpairTransceiver).

Fix #375.

@fibann fibann requested a review from djee-ms June 25, 2020 16:38
Ensure proper shutdown across MediaLines & co.

Removed AutoInitializeOnStart, and made Initialize/Uninitialize private,
now PC should be turned on/off by enabling/disabling.

We now create/keep local tracks only when there are both a live source and a
connection. Therefore, local tracks do not persist after the PeerConnection
component is disabled. This is intended, as we cannot keep the track and
attach it to a different PeerConnection if the component is re-enabled.

Removed listeners to tracks removed. Unpairing is handled manually on
renegotiation and on shutdown (MediaLine.UnpairTransceiver).
Avoid accumulating a lot of game objects when running multiple tests in a row, makes test shutdown faster
@fibann fibann requested a review from djee-ms July 17, 2020 17:10
@djee-ms djee-ms added the bug Something isn't working label Jul 18, 2020
@djee-ms djee-ms added this to the 2.0.0 milestone Jul 18, 2020
Copy link
Member

@djee-ms djee-ms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realize it's a bit late, but this design prevents the user from hot-swapping a track on an active transceiver, right? It's a bit unfortunate.

libs/unity/library/Runtime/Scripts/Media/MediaLine.cs Outdated Show resolved Hide resolved
libs/unity/library/Runtime/Scripts/Media/MediaLine.cs Outdated Show resolved Hide resolved
libs/unity/library/Runtime/Scripts/Media/MediaLine.cs Outdated Show resolved Hide resolved
libs/unity/library/Runtime/Scripts/PeerConnection.cs Outdated Show resolved Hide resolved
libs/unity/library/Runtime/Scripts/PeerConnection.cs Outdated Show resolved Hide resolved
libs/unity/library/Tests/Runtime/PeerConnectionTests.cs Outdated Show resolved Hide resolved
libs/unity/library/Tests/Runtime/PeerConnectionTests.cs Outdated Show resolved Hide resolved
Co-authored-by: Jerome Humbert <jerome.humbert@microsoft.com>
@fibann
Copy link
Member Author

fibann commented Jul 20, 2020

this design prevents the user from hot-swapping a track on an active transceiver, right?

I have changed Source.set to swap the new track before disposing the old one so that should make track swapping fast (we can't persist the tracks themselves but recreating them should be cheap enough).

There turned out to be a bug there, fix is now up (plus tests).

@fibann fibann merged commit 986978f into microsoft:master Jul 20, 2020
@fibann fibann deleted the unity-pc-shutdown branch July 20, 2020 14:52
mr-webrtc-buildbot added a commit that referenced this pull request Jul 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot stop a call and start a new call
2 participants