@twilio/webrtc - npm Package Compare versions

Comparing version 1.1.0 to 2.0.0

CHANGELOG.md

@@ -0,1 +1,19 @@
+2.0.0 (January 9, 2018)
+=======================
+
+New Features
+------------
+
+- Added shims for the `RTCRtpSender/RTCRtpReceiver` based APIs. The legacy
+  `MediaStream` based API shims have been removed. (JSDK-1631)
+
+Bug Fixes
+---------
+
+- Previously, we were overwriting MediaStreamTrack IDs with the values signaled
+  in the SDP's MSID attributes in order to maintain compatibility with
+  pre-WebRTC 1.0 behavior. The particular method we used did not take into
+  account the fact that the actual MediaStreamTrack IDs would continue to show
+  in `getStats` results and has been removed.
+
 1.1.0 (October 24, 2017)
@@ -2,0 +20,0 @@ ========================

lib/rtcpeerconnection/chrome.js

@@ -8,2 +8,4 @@ /* globals RTCDataChannel, RTCSessionDescription, webkitRTCPeerConnection */
 var Latch = require('../util/latch');
+var MediaStream = require('../mediastream');
+var RTCRtpSenderShim = require('../rtcrtpsender');
 var updateTracksToSSRCs = require('../util/sdp').updatePlanBTrackIdsToSSRCs;
@@ -42,2 +44,9 @@ var util = require('../util');
 
+  // NOTE(mmalavalli): Because of a bug related to "ontrack", we prevent it
+  // from being delegated to ChromeRTCPeerConnection. For now, this bug
+  // manifests when we run Chrome with the flag: --enable-blink-features=RTCRtpSender
+  // Existing bug: https://bugs.chromium.org/p/chromium/issues/detail?id=774303
+  // Bug filed by us: https://bugs.chromium.org/p/chromium/issues/detail?id=783433
+  util.interceptEvent(this, 'track');
+
   /* eslint new-cap:0 */
@@ -47,4 +56,4 @@ var peerConnection = new PeerConnection(newConfiguration);
   Object.defineProperties(this, {
-    _localStreams: {
-      value: new Set()
+    _localStream: {
+      value: new MediaStream()
     },
@@ -62,2 +71,5 @@ _peerConnection: {
     },
+    _senders: {
+      value: new Map()
+    },
     _signalingStateLatch: {
@@ -112,2 +124,3 @@ value: new Latch()
 
+  peerConnection.addStream(this._localStream);
   util.proxyProperties(PeerConnection.prototype, this, peerConnection);
@@ -118,13 +131,63 @@ }
 
-ChromeRTCPeerConnection.prototype.addStream = function addStream(stream) {
-  if (this._localStreams.has(stream)) {
+// NOTE(mmalavalli): This shim supports our limited case of adding
+// all MediaStreamTracks to one MediaStream. It has been implemented this
+// keeping in mind that this is to be maintained only until "addTrack" is
+// supported natively in Chrome.
+ChromeRTCPeerConnection.prototype.addTrack = function addTrack() {
+  var args = [].slice.call(arguments);
+  if (this._peerConnection.addTrack) {
+    return this._peerConnection.addTrack.apply(this._peerConnection, args);
+  }
+
+  var track = args[0];
+  if (this._peerConnection.signalingState === 'closed') {
+    throw new Error('Cannot add MediaStreamTrack [' + track.id + ', '
+      + track.kind + ']: RTCPeerConnection is closed');
+  }
+
+  var sender = this._senders.get(track);
+  if (sender && sender.track) {
+    throw new Error('Cannot add MediaStreamTrack [' + track.id + ', '
+      + track.kind + ']: RTCPeerConnection already has it');
+  }
+  this._peerConnection.removeStream(this._localStream);
+  this._localStream.addTrack(track);
+  this._peerConnection.addStream(this._localStream);
+
+  sender = new RTCRtpSenderShim(track);
+  this._senders.set(track, sender);
+  return sender;
+};
+
+// NOTE(mmalavalli): This shim supports our limited case of removing
+// MediaStreamTracks from one MediaStream. It has been implemented this
+// keeping in mind that this is to be maintained only until "removeTrack" is
+// supported natively in Chrome.
+ChromeRTCPeerConnection.prototype.removeTrack = function removeTrack(sender) {
+  if (this._peerConnection.removeTrack) {
+    this._peerConnection.removeTrack(sender);
     return;
   }
-  this._localStreams.add(stream);
-  this._peerConnection.addStream(stream);
+  if (this._peerConnection.signalingState === 'closed') {
+    throw new Error('Cannot remove MediaStreamTrack: RTCPeerConnection is closed');
+  }
+
+  var track = sender.track;
+  if (!track) {
+    return;
+  }
+  sender = this._senders.get(track);
+  if (sender && sender.track) {
+    sender.track = null;
+    this._peerConnection.removeStream(this._localStream);
+    this._localStream.removeTrack(track);
+    this._peerConnection.addStream(this._localStream);
+  }
 };
 
-ChromeRTCPeerConnection.prototype.removeStream = function removeStream(stream) {
-  this._localStreams.delete(stream);
-  this._peerConnection.removeStream(stream);
+ChromeRTCPeerConnection.prototype.getSenders = function getSenders() {
+  if (this._peerConnection.getSenders) {
+    return this._peerConnection.getSenders();
+  }
+  return Array.from(this._senders.values());
 };
@@ -257,6 +320,2 @@
 function maybeDispatchTrackEvents(peerConnection, mediaStreamTracks) {
-  if ('ontrack' in PeerConnection.prototype) {
-    return;
-  }
-
   var currentMediaStreamTracks = util.flatMap(peerConnection.getRemoteStreams(), function(mediaStream) {
@@ -263,0 +322,0 @@ return mediaStream.getTracks();

lib/rtcpeerconnection/firefox.js

@@ -6,4 +6,4 @@ /* globals mozRTCPeerConnection, RTCPeerConnection */
 var FirefoxRTCSessionDescription = require('../rtcsessiondescription/firefox');
+var RTCRtpSenderShim = require('../rtcrtpsender');
 var inherits = require('util').inherits;
-var MediaStream = require('../mediastream');
 var updateTracksToSSRCs = require('../util/sdp').updateUnifiedPlanTrackIdsToSSRCs;
@@ -57,11 +57,5 @@ var util = require('../util');
     },
-    _localStream: {
-      value: new MediaStream()
-    },
     _peerConnection: {
       value: peerConnection
     },
-    _remoteStream: {
-      value: new MediaStream()
-    },
     _rollingBack: {
@@ -71,2 +65,5 @@ value: false,
     },
+    _senders: {
+      value: new Map()
+    },
     _tracksToSSRCs: {
@@ -116,23 +113,2 @@ value: new Map()
   util.proxyProperties(PeerConnection.prototype, this, peerConnection);
-
-  // NOTE(mroberts): We use the adapter.js workaround for providing track events.
-  if (!('ontrack' in PeerConnection.prototype)) {
-    peerConnection.addEventListener('addstream', function onaddstream(addStreamEvent) {
-      var mediaStream = addStreamEvent.stream;
-
-      // NOTE(mmalavalli): We are not using MediaStream#onaddtrack event listeners
-      // for shimming PeerConnection#ontrack like we do for Chrome because it
-      // is not yet supported in Firefox (support is slated for Firefox 50).
-      // That's why we don't see this event being used in adapter.js's Firefox
-      // shim.
-      // Reference: https://developer.mozilla.org/en-US/docs/Web/API/MediaStream#Browser_compatibility
-
-      mediaStream.getTracks().forEach(function(mediaStreamTrack) {
-        var newEvent = new Event('track');
-        newEvent.track = mediaStreamTrack;
-        newEvent.streams = [mediaStream];
-        self.dispatchEvent(newEvent);
-      });
-    });
-  }
 }
@@ -142,64 +118,2 @@
 
-// NOTE(mroberts): Until Chrome adds support for getSenders and getReceivers,
-// it makes sense for our RTCPeerConnection adapter to continue providing
-// support for the deprecated getLocalStreams and getRemoteStreams APIs.
-//
-// However, in order to implement these, we break an invariant of the legacy
-// API: we don't return the actual MediaStream objects negotiated. Instead, we
-// reuse the same one for each call to getLocalStreams or getRemoteStreams.
-//
-// This is sufficient for our use of the WebRTC APIs, although it wouldn't be
-// a good fit for others.
-//
-// We also include a workaround for the following Bugzilla issue:
-//
-//     https://bugzilla.mozilla.org/show_bug.cgi?id=1154084
-//
-FirefoxRTCPeerConnection.prototype.getLocalStreams = function getLocalStreams() {
-  if (this._isClosed) {
-    return [];
-  }
-
-  if (!this.getSenders) {
-    return this._peerConnection.getLocalStreams();
-  }
-
-  var stream = this._localStream;
-
-  stream.getTracks().forEach(stream.removeTrack, stream);
-
-  this.getSenders().forEach(function(sender) {
-    var track = sender.track;
-    if (track) {
-      stream.addTrack(track);
-    }
-  });
-
-  return [stream];
-};
-
-// NOTE(mroberts): See comment above.
-FirefoxRTCPeerConnection.prototype.getRemoteStreams = function getRemoteStreams() {
-  if (this._isClosed) {
-    return [];
-  }
-
-  if (!this.getReceivers) {
-    return this._peerConnection.getRemoteStreams();
-  }
-
-  var stream = this._remoteStream;
-
-  stream.getTracks().forEach(stream.removeTrack, stream);
-
-  this.getReceivers().forEach(function(receiver) {
-    var track = receiver.track;
-    if (track) {
-      stream.addTrack(track);
-    }
-  });
-
-  return [stream];
-};
-
 // NOTE(mmalavalli): Firefox throws a TypeError when the PeerConnection's
@@ -217,27 +131,49 @@ // prototype's "peerIdentity" property is accessed. In order to overcome
 
-// NOTE(mmalavalli): Firefox does not support RTCPeerConnection#removeStream(),
-// and calling it will screw up the RTCPeerConnection's internal state. So
-// for now, we don't do anything when it is called.
-// Bugzilla: https://bugzilla.mozilla.org/show_bug.cgi?id=842455
-//
-// UPDATE(mmalavalli): We tried to implement removeStream() using
-// RTCPeerConnection#removeTrack(), but ran into a Firefox bug in the
-// Firefox <--> Chrome interop case, which is mentioned below.
-// Bugzilla: https://bugzilla.mozilla.org/show_bug.cgi?id=1133874
-FirefoxRTCPeerConnection.prototype.removeStream = function removeStream() {};
+// NOTE(mmalavalli): Because we are not delegating to the native
+// RTCPeerConnection#removeTrack(), we have to manually maintain a list of added
+// tracks. So we disable the delegation to the native RTCPeerConnection#addTrack()
+// for now.
+FirefoxRTCPeerConnection.prototype.addTrack = function addTrack() {
+  var args = [].slice.call(arguments);
+  var track = args[0];
+  var sender = this._senders.get(track);
+  if (sender && sender.track) {
+    throw new Error('Cannot add MediaStreamTrack [' + track.id + ', '
+      + track.kind + ']: RTCPeerConnection already has it');
+  }
+  sender = getActiveSenders(this._peerConnection).get(track)
+    || this._peerConnection.addTrack.apply(this._peerConnection, args);
 
-// NOTE(mmalavalli): We implement addStream() by calling
-// RTCPeerConnection#addTrack() on each of the MediaStreamTracks in the
-// given MediaStream that have not already been added.
-FirefoxRTCPeerConnection.prototype.addStream = function addStream(mediaStream) {
-  var localTracks = this._peerConnection.getSenders().map(function(sender) {
-    return sender.track;
-  });
-  var newLocalTracks = util.difference(mediaStream.getTracks(), localTracks);
+  this._senders.set(track, sender);
+  return sender;
+};
 
-  newLocalTracks.forEach(function(mediaStreamTrack) {
-    this._peerConnection.addTrack(mediaStreamTrack, mediaStream);
-  }, this);
+// NOTE(mmalavalli): RTCPeerConnection#removeTrack() has a bug in the
+// Firefox <--> Chrome interop case, which is mentioned below. So we disable
+// its delegation for now. Also, we maintain only one RTCRtpSender per
+// MediaStreamTrack for our use case, and not worry about multiple RTCRtpSenders
+// due to replaceTrack().
+// Bugzilla: https://bugzilla.mozilla.org/show_bug.cgi?id=1133874
+FirefoxRTCPeerConnection.prototype.removeTrack = function removeTrack(sender) {
+  if (this._isClosed) {
+    throw new Error('Cannot remove MediaStreamTrack: RTCPeerConnection is closed');
+  }
+  var track = sender.track;
+  if (!track) {
+    return;
+  }
+  sender = this._senders.get(track);
+  if (sender && sender.track) {
+    this._senders.set(track, new RTCRtpSenderShim(null));
+  }
 };
 
+// NOTE(mmalavalli): Because we are not delegating to the native
+// RTCPeerConnection#removeTrack(), we have to manually maintain a list of added
+// tracks. So we disable the delegation to the native RTCPeerConnection#getSenders()
+// for now.
+FirefoxRTCPeerConnection.prototype.getSenders = function getSenders() {
+  return Array.from(this._senders.values());
+};
+
 FirefoxRTCPeerConnection.prototype.createAnswer = function createAnswer() {
@@ -440,2 +376,15 @@ var args = [].slice.call(arguments);
 
+/**
+ * Gets the active RTCRtpSenders of the RTCPeerConnection.
+ * @param peerConnection
+ * @returns {Map<MediaStreamTrack, RTCRtpSender>}
+ */
+function getActiveSenders(peerConnection) {
+  return new Map(peerConnection.getSenders().filter(function(sender) {
+    return sender.track;
+  }).map(function(sender) {
+    return [sender.track, sender];
+  }));
+}
+
 module.exports = FirefoxRTCPeerConnection;

lib/rtcpeerconnection/safari.js

@@ -7,3 +7,3 @@ /* globals RTCPeerConnection, RTCSessionDescription */
 var Latch = require('../util/latch');
-var TrackMatcher = require('../util/trackmatcher');
+var RTCRtpSenderShim = require('../rtcrtpsender');
 var updateTracksToSSRCs = require('../util/sdp').updatePlanBTrackIdsToSSRCs;
@@ -22,6 +22,3 @@ var util = require('../util');
   util.interceptEvent(this, 'signalingstatechange');
-  util.interceptEvent(this, 'track');
 
-  var trackMatcher = new TrackMatcher();
-
   var peerConnection = new RTCPeerConnection(configuration);
@@ -38,5 +35,2 @@
     },
-    _localStreams: {
-      value: new Map()
-    },
     _peerConnection: {
@@ -53,4 +47,4 @@ value: peerConnection
     },
-    _remoteStreams: {
-      value: new Set()
+    _senders: {
+      value: new Map()
     },
@@ -141,19 +135,2 @@ _signalingStateLatch: {
 
-  peerConnection.addEventListener('track', function ontrack(event) {
-    var sdp = self.remoteDescription ? self.remoteDescription.sdp : null;
-    if (sdp) {
-      trackMatcher.update(sdp);
-      var id = trackMatcher.match(event.track.kind);
-      if (id) {
-        Object.defineProperty(event.track, 'id', {
-          value: id
-        });
-      }
-    }
-    event.streams.forEach(function(stream) {
-      self._remoteStreams.add(stream);
-    });
-    self.dispatchEvent(event);
-  });
-
   util.proxyProperties(RTCPeerConnection.prototype, this, peerConnection);
@@ -244,43 +221,53 @@ }
 
-SafariRTCPeerConnection.prototype.addStream = function addStream(stream) {
-  if (this._localStreams.has(stream)) {
-    return;
+// NOTE(mmalavalli): Because we are not delegating to the native
+// RTCPeerConnection#removeTrack(), we have to manually maintain a list of added
+// tracks. So we disable the delegation to the native RTCPeerConnection#addTrack()
+// for now. Also, we maintain only one RTCRtpSender per MediaStreamTrack for our
+// use case, and not worry about multiple RTCRtpSenders due to replaceTrack().
+SafariRTCPeerConnection.prototype.addTrack = function addTrack() {
+  var args = [].slice.call(arguments);
+  var track = args[0];
+  var sender = this._senders.get(track);
+  if (sender && sender.track) {
+    throw new Error('Cannot add MediaStreamTrack [' + track.id + ', '
+      + track.kind + ']: RTCPeerConnection already has it');
   }
-  var tracks = new Set(stream.getTracks());
-  this._peerConnection.getSenders().forEach(function(sender) {
-    if (tracks.has(sender.track)) {
-      tracks.delete(sender.track);
-    }
-  });
-  tracks.forEach(function(track) {
-    try {
-      this.addTrack(track, stream);
-    } catch (error) {
-      // Do nothing.
-    }
-  }, this);
-  this._localStreams.set(stream, new Set(tracks));
-};
+  sender = getActiveSenders(this._peerConnection).get(track)
+    || this._peerConnection.addTrack.apply(this._peerConnection, args);
 
-SafariRTCPeerConnection.prototype.removeStream = function removeStream(stream) {
-  // NOTE(mroberts): We can't really remove tracks right now, at least if we
-  // ever want to add them back...
-  //
-  //     https://bugs.webkit.org/show_bug.cgi?id=174327
-  //
-  // var tracks = this._localStreams.get(stream) || new Set();
-  // this.getSenders().forEach(function(sender) {
-  //   if (tracks.has(sender.track)) {
-  //     this.removeTrack(sender);
-  //   }
-  // }, this);
-  this._localStreams.delete(stream);
+  // NOTE(mmalavalli): webrtc-adapter has a bug where the "addTrack" shim
+  // does not return an RTCRtpSender and returns undefined instead. An issue
+  // [https://github.com/webrtc/adapter/issues/714] has been filed. For now,
+  // we manually get the RTCRtpSender associated with the added track and
+  // return it.
+  sender = sender || getActiveSenders(this._peerConnection).get(track);
+  this._senders.set(track, sender);
+  return sender;
 };
 
-SafariRTCPeerConnection.prototype.getLocalStreams = function getLocalStreams() {
-  return Array.from(this._localStreams.keys());
+// NOTE(mroberts): We can't really remove tracks right now, at least if we
+// ever want to add them back...
+//
+//     https://bugs.webkit.org/show_bug.cgi?id=174327
+//
+SafariRTCPeerConnection.prototype.removeTrack = function removeTrack(sender) {
+  if (this._isClosed) {
+    throw new Error('Cannot remove MediaStreamTrack: RTCPeerConnection is closed');
+  }
+  var track = sender.track;
+  if (!track) {
+    return;
+  }
+  sender = this._senders.get(track);
+  if (sender && sender.track) {
+    this._senders.set(track, new RTCRtpSenderShim(null));
+  }
 };
 
-SafariRTCPeerConnection.prototype.getRemoteStreams = function getRemoteStreams() {
-  return Array.from(this._remoteStreams);
+// NOTE(mmalavalli): Because we are not delegating to the native
+// RTCPeerConnection#removeTrack(), we have to manually maintain a list of added
+// tracks. So we disable the delegation to the native RTCPeerConnection#getSenders()
+// for now.
+SafariRTCPeerConnection.prototype.getSenders = function getSenders() {
+  return Array.from(this._senders.values());
 };
@@ -381,2 +368,15 @@
 
+/**
+ * Gets the active RTCRtpSenders of the RTCPeerConnection.
+ * @param peerConnection
+ * @returns {Map<MediaStreamTrack, RTCRtpSender>}
+ */
+function getActiveSenders(peerConnection) {
+  return new Map(peerConnection.getSenders().filter(function(sender) {
+    return sender.track;
+  }).map(function(sender) {
+    return [sender.track, sender];
+  }));
+}
+
 module.exports = SafariRTCPeerConnection;

package.json

 {
   "name": "@twilio/webrtc",
-  "version": "1.1.0",
+  "version": "2.0.0",
   "description": "WebRTC-related APIs and shims used by twilio-video.js",
@@ -54,4 +54,4 @@ "scripts": {
     "watchify": "^3.9.0",
-    "webrtc-adapter": "^5.0.3"
+    "webrtc-adapter": "^6.0.1"
   }
 }

README.md

@@ -79,2 +79,5 @@ twilio-webrtc.js
   more information.
+* Does not depend on the native "track" event (currently behind the flag: `--enable-blink-features=RTCRtpSender`) because
+  of [this bug](https://bugs.chromium.org/p/chromium/issues/detail?id=774303), which partly refers to "ontrack" not firing when expected.
+  We have filed [this bug](https://bugs.chromium.org/p/chromium/issues/detail?id=783433) specifically for this.
 
@@ -88,9 +91,9 @@ #### Firefox
   change the previously negotiated DTLS role in an answer, which breaks Chrome.
-* Adds "track" event support, as per the workaround in [webrtc-adapter](https://github.com/webrtc/adapter/blob/master/src/js/firefox/firefox_shim.js#L14).
-* Implements the legacy `getLocalStreams` and `getRemoteStreams` methods.
-* Provides a workaround for [this bug](https://bugzilla.mozilla.org/show_bug.cgi?id=1154084),
-  where `getLocalStreams` and `getRemoteStreams` throw when called in signaling state `closed`.
 * Provides a workaround for [this bug](https://bugzilla.mozilla.org/show_bug.cgi?id=1363815),
   where the browser throws when `RTCPeerConnection.prototype.peerIdentity` is accessed.
-* The legacy `addStream` method is implemented in terms of the `addTrack` method.
+* Provides a shim for the `removeTrack` method in order to work around [this bug](https://bugzilla.mozilla.org/show_bug.cgi?id=1133874).
+* Provides a shim for the `addTrack` method. Since `removeTrack` is shimmed, there is a necessity to
+  maintain an explicit list of added `RTCRtpSender`s.
+* Provides a shim for the `getSenders` method. Since `removeTrack` is shimmed, there is a necessity to
+  maintain an explicit list of added `RTCRtpSender`s.
 
@@ -102,8 +105,11 @@ #### Safari
   `MediaStreamTrack`.
-* Provides a workaround for [this bug](https://bugs.webkit.org/show_bug.cgi?id=174519), where
-  the `MediaStreamTrack`s cannot be identified correctly due to the lack of `MID`s in the sdp.
 * Provides a workaround for [this bug](https://bugs.webkit.org/show_bug.cgi?id=174323), where
   trying to access the `localDescription` or `remoteDescription` throws an exception.
-* The legacy `addStream` method is implemented in terms of the `addTrack` method.
-* Implements the legacy `getLocalStreams` and `getRemoteStreams` methods.
+* Provides a shim for the `removeTrack` method in order to work around [this bug](https://bugs.webkit.org/show_bug.cgi?id=174327).
+* Provides a shim for the `addTrack` method. Since `removeTrack` is shimmed, there is a necessity to
+  maintain an explicit list of added `RTCRtpSender`s.
+* Provides a workaround for [this bug](https://github.com/webrtc/adapter/issues/714), where webrtc-adapter's shimmed
+  `addTrack` method does not return the `RTCRtpSender` associated with the added track.
+* Provides a shim for the `getSenders` method. Since `removeTrack` is shimmed, there is a necessity to
+  maintain an explicit list of added `RTCRtpSender`s.
 
@@ -110,0 +116,0 @@ ### RTCSessionDescription

No alert changes

Improved metrics

Total package byte prevSize

94703

increased by0.54%

Number of lines in readme file

124

increased by5.08%

Worsened metrics

Lines of code

2213

decreased by-2.68%

No dependency changes