This repository has been archived by the owner on Mar 22, 2022. It is now read-only.
Add multi-track support #283
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is part 1/N of the change, see global description below ---- Add standalone media track objects in both the C++ and C# library. This enables supporting multiple audio and video tracks per peer connection. For local tracks, the `PeerConnection` interface is updated such that the functionality of `AddLocalAudioTrackAsync()` and `AddLocalVideoTrackAsync()` is split in 2 steps: - the creation step, whose interface is on the local track object itself, creates a standalone track object not associated with any peer connection and not currently used by anything. - the association step, which takes the track and attach it to a peer connection via a transceiver. For remote tracks, the `TrackAdded` and `TrackRemoved` events now generate a new remote track object, which is passed to the event handler as argument. The C# `PeerConnection` object also now keeps a list of all the tracks attached to it, which can therefore be enumerated. Because tracks are standalone objects, methods like `PeerConnection.SetLocalAudioTrackEnabled()` are replaced with a property `LocalAudioTrack.Enabled`, providing a more familiar pattern to C# developers. One important change required for remote tracks to be paired by name is removing the legacy options `RTCOfferAnswerOptions::offer_to_receive_video = true` and `RTCOfferAnswerOptions::offer_to_receive_audio = true` from `CreateOffer()` and `CreateAnswer()`. This change (see #75) has larger implications that it looks when using Unified Plan (default), as previously those options were implicitly making an offer to receive some media, whereas this step is now explicit and the user needs to add some transceiver in receive-only or send-receive mode before making an offer/answer to allow the remote peer to send. A new Unity demo `StandaloneDemo` shows how to use multiple tracks between two peers. The demo is standalone in the sense signaling is hard-coded and does not require an external node-dss server or any other signaling solution. Bug: #152
Add an emulation layer for transceiver-over-plan-b which uses the transceiver C++ classes and emulate the behavior of the Unified Plan transceivers using the track-based Plan B rules.
Merge the implementation of audio and video transceivers, which differ only by their media kind and the tracks that can be attached to them.
Handles to native objects are not owning a reference anymore when not passed from a method creating the object. For example, getting a track from a transceiver returns a "view" handle that is a copy of the one previously returned when the track was created, and should *not* be released while the track is in use. This makes the handle usage more understandable and similar to a pure C API.
The transceiver tests are already testing those methods, no need to test them also in the video tests.
Use template specialization to keep a single copy of the implementation templated on the media kind, to avoid discrepancies.
Local tracks are owned by the user, and do not need to be tracked by the peer connection. The transceiver they are optionally attached to keeps them alive while attached, but otherwise there should be no other strong reference to them. The local track itself keeps a weak back-reference to the transceiver while attached to it.
Remote tracks are owned by transceivers, and are now exclusively accessed through them.
Use a single function `mrsPeerConnectionAddTransceiver()` and pass the media kind as an argument, instead of having 2 separate functions for audio and video transceivers. This simplifies the implementation as well as any wrapper layer above it.
Restore support for video profiles in TestAppUWP, and auto-select most fields for convenience when navigating to the "Add video track" page.
Downgrade the target platform version to 10.0.17763.0 as it looks like more recent ones are not supported by Azure DevOps hosted agents used for CI.
djee-ms
added
enhancement
This was
linked to
issues
Apr 16, 2020
|
Is there anything that needs being looked at, or is it all content from previous reviews? |
fibann
approved these changes
Apr 17, 2020
Fix a logic bug when synchronizing transceivers where matching new wrappers by RTP transceiver was not performed correctly, leading to mismatches and an assertion.
Clean-up PeerConnection.AddDataChannelAsync() to remove unnecessary async/wait, which are useless and can cause some deadlock as the continuation is done on the same thread, so calling Wait() on the returned task was causing a deadlock. Also add some cancellation token to help cancel the task.
Media line indices of tranceivers are monotonically increasing but can form discontinuous list because data channels also consume a 'mid' in the SDP messages. This change fixes transceiver pairing in Unity by making sure transceivers are sorted by media line index, and associate them 1:1 in order with the media lines declared by the user in the peer connection component.
Remove incorrect early out in SceneVideoSender preventing the component from capturing the scene when used in an AR/VR device.
Avoid confusion between transceiver (C# implementation object, only exists on peer connection, negotiated) and media line (declaration of intent by the user, Unity library specific concept) by renaming the PeerConnection component method to AddMediaLine().
fibann
reviewed
Apr 21, 2020
| for (auto&& rtp_tr : rtp_transceivers) { | ||
| const int mline_index = ExtractMlineIndexFromRtpTransceiver(rtp_tr); | ||
| // Create a wrapper if it doesn't exist yet | ||
| if (it_wrapper == wrappers.end()) { | ||
| if ((it_wrapper == wrappers.end()) || ((*it_wrapper)->impl() != rtp_tr)) { |
There was a problem hiding this comment.
What does it mean here if there is another transceiver already?
There was a problem hiding this comment.
The transceivers are sorted by address in memory. You have 2 arrays being iterated in parallel. If one value doesn't match with the other, it means one array (wrappers) is missing an element and you need to insert it.
RTP | Wrapper
----|--------
A | A
B | B v
C | D ^ Need to insert here because C != D
D |
There was a problem hiding this comment.
Yeah it's reasonably clear once you un-hide the surrounding lines 😄
nit: might be worth adding a comment for good measure.
There was a problem hiding this comment.
Yes the comment confuses the reader I think, but thinking about the actual algorithm (sort 2 arrays to avoid O(n^2) comparison) makes things clear.
fibann
approved these changes
Apr 21, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add support for multiple audio and video tracks per peer connection.
This change introduces a new transceiver API closely related to the same-named concept in the WebRTC 1.0 standard. The transceiver API allows the user to add audio and video transceivers to a peer connection, then optionally associate some local track to send some media through those transceivers and/or receive some media from the remote peer via a remote track associated with the transceiver. The new API is more flexible and more powerful, offering more control to the user and opening up many use cases not previously available, at the expense of some moderately increased complexity. In particular it allows:
This change constitutes a major API breaking change, which moves tracks management away from the
PeerConnectionobject to the newTransceiverobject. Users are encouraged to read the migration guide associated with this change to migrate existing v1.0-style applications. In particular, understanding the functioning of transceivers and the asymmetrical role of the caller peer (the peer initiating a session with a new offer) and the callee peer (the peer responding to a remote offer) are critical to be able to use this new API correctly.