Add legacy note about addStream.

[jan-ivar]

Potentially for #1125.

---

Preview | Diff

[fippo]

if we do it then write it like this

RTCPeerConnection.prototype.addStream = function(stream) {
  var pc = this;
  stream.getTracks.forEach(function(track) {
    pc.addTrack(track, stream);
  });
};

as a pre class="example highlight"?

[jan-ivar]

@fippo class="example highlight" didn't work inside a note, so just using pre.

[taylor-b]

Looks good, but should we also acknowledge the behavior described here, where after calling addStream, tracks subsequently added to the stream are automatically associated with the PeerConnection? This makes the polyfill slightly more complicated:

RTCPeerConnection.prototype.addStream = function(stream) {
  var pc = this;
  stream.getTracks().forEach(function(track) {
    pc.addTrack(track, stream);
  });
  // Listen to future changes to the stream, and add/remove tracks automatically.
  oldAddTrack = stream.addTrack;
  oldRemoveTrack = stream.removeTrack;
  stream.addTrack = function(track) {
    pc.addTrack(track, stream);
    oldAddTrack.call(stream, track);
  }
  stream.removeTrack = function(track) {
    pc.removeTrack(track, stream);
    oldRemoveTrack.call(stream, track);
  }
};

I guess this also needs to restore oldAddTrack after removeStream is called, but you get the picture hopefully.

[jan-ivar]

@taylor-b AFAIK that behavior was never in the spec, and isn't web compatible, so I don't think it belongs there. Maybe MDN?

For me the goal of this note is someone searching the spec for "addStream" finding a vestige of it.

[taylor-b]

This is how Chrome has always behaved, and given that Chrome doesn't support addTrack/removeTrack (until very soon!), this is the only way people have had to modify a stream (besides removing it and re-adding it, which has consequences). I can imagine there's a lot of code that looks like this:

stream.addTrack(track);
if (pc.addTrack) {
  pc.addTrack(track, stream);
}

Or some variant thereof.

Also, this behavior was in the standard (or at least was heavily implied) at one point:

If something in the browser changes that causes the RTCPeerConnection object to need to initiate a new session description negotiation, a negotiationneeded event is fired at the RTCPeerConnection object.

In particular, if an RTCPeerConnection object is consuming a MediaStream on which a track is added, by, e.g., the addTrack() method being invoked, the RTCPeerConnection object must fire the "negotiationneeded" event. Removal of media components must also trigger "negotiationneeded".

[jan-ivar]

I can imagine there's a lot of code that looks like this:

stream.addTrack(track);
if (pc.addTrack) {
  pc.addTrack(track, stream);
}

@taylor-b then we should not polyfill automatic addition, or addTrack throws InvalidAccessError.

[taylor-b]

@jan-ivar Oh, you mean this code will start failing once Chrome does implement addTrack? Then replace it with:

if (chrome) {
  stream.addTrack(track);
} else {
  pc.addTrack(track, stream);
}

I'm not an application developer myself, I'm not sure what code people actually write to deal with this.

But my point is, it's behavior that's been around for a long time, which we will need to help people migrate off of. If you don't think it should be mentioned in the note here, that's fine; I just wanted to call it out in case people were unaware of it.

[jan-ivar]

@taylor-b I'm not an application developer eiter, but FWIW it's my understanding that feature-testing (your first example) is considered a good pattern, wheras user-agent testing (the latter) is not.

[jan-ivar]

@taylor-b Lastly, we try to encourage polyfills of new APIs, not legacy ones.

I've added a Migration section to the addStream MDN entry. PTAL.

[taylor-b]

@jan-ivar The migration section you added looks good, thanks.