Webrtc-pc: Should the spec describe addStream/onaddstream as legacy API?

[youennf]

Related discussion in https://github.com/w3c/web-platform-tests/issues/5533

addStream and onaddstream are widely used but not specified in the WebRTC specification.
This API is more convenient to use from a web developer point of view for simple A/V calls then addTrack/ontrack.
This API is available in Edge, Chrome and Firefox while addTrack is only supported in Firefox.

From a pragmatic point of view, it seems like the webrtc spec should describe this legacy API, in terms of existing API like addTrack for instance.

[nils-ohlmeier]

This API is more convenient to use from a web developer point of view for simple A/V calls then addTrack/ontrack.

Then write a library or some code which simplifies the API usage, but don't bloat the native API to support a simple and a more complex version.

This API is available in Edge, Chrome and Firefox while addTrack is only supported in Firefox.

Then get the implementations which are lacking behind the spec to finally implement what the spec is and don't give them excuses to implement what ever they want.

From a pragmatic point of view, it seems like the webrtc spec should describe this legacy API, in terms of existing API like addTrack for instance.

I think the WebRTC specs are way to complex already. Adding sections about legacy stuff will only bloat them and cause confusion on what has to be implemented and what is available. How about you create a separate document/spec which describes the old, legacy APIs.

[youennf]

This API is more convenient to use from a web developer point of view for simple A/V calls then addTrack/ontrack.
Then write a library or some code which simplifies the API usage, but don't bloat the native API to support a simple and a more complex version.

The point I was trying to make was just that web developers will not have a huge incentive to move towards addTrack given addStream is often more convenient. Contrary to callback/promise APIs.
This might mean that addStream might remain in browsers for a long time.

This API is available in Edge, Chrome and Firefox while addTrack is only supported in Firefox.
Then get the implementations which are lacking behind the spec to finally implement what the spec is and don't give them excuses to implement what ever they want.

As a browser implementor, not shipping addStream might be a problem (currently under assessment in WebKit).
As a browser implementor, implementing addStream without a spec or a path towards convergence is a problem.

I think the WebRTC specs are way to complex already. Adding sections about legacy stuff will only bloat them and cause confusion on what has to be implemented and what is available. How about you create a separate document/spec which describes the old, legacy APIs.

Agreed on the complexity of the WebRTC spec.
WebRTC is already describing some legacy APIs for which the migration path is already quite clear (callback to promise e.g.).
It is not describing the single legacy API that has widespread use, unclear interoperability and unclear migration path.

[youennf]

I am not urgently advocating for adding addStream actually.
I am more interested in finding a good plan to fix the addStream thing.

[fippo]

the larger issue here is how the WG envisions to deal with making necessary (*) changes like moving from stream-based to track-based model. For "web developers" there is little incentive to make that switch and with Chrome not implementing this model it is of little practical relevance.

(*) given that things seem to work one can argue whether they were necessary at all

[ibc]

The main problem is that the "1.0" spec has been dramatically changed for years, so now we have lot of legacy code, many apps replying on it, and some browsers (AKA Chrome) keeping too much old stuff, own specs and not implementing the current standard. Somehow I understand it given how long it has taken for the spec to stabilize.

[foolip]

https://crbug.com/697059 is the tracking issue for this in Blink, where I also included getLocalStreams, getRemoteStreams and the MediaStreamEvent interface. (getStreamById is https://crbug.com/698163, and seems quite feasible to remove.)

I wrote these tests for existence: https://jsbin.com/qirujiq/edit?html,output

That shows that Chrome and Edge already ship all of these APIs, and Firefox ships everything except the removeStream method.

WebKit has everything except the onremovestream event handler attribute in their IDL:
https://github.com/WebKit/webkit/blob/master/Source/WebCore/Modules/mediastream/RTCPeerConnection.idl

We have use counters for most of this in Chrome:

• addStream
• removeStream
• getLocalStreams
• getRemoteStreams

I defer to @guidou @henbos to say what we'd prefer happen with these APIs.

[foolip]

@jan-ivar, can you say something about how the MediaStream-based and MediaStreamTrack-based APIs can co-exist in Gecko? In particular, how do getLocalStreams and getRemoteStreams work if only individual tracks were added?

[foolip]

(getStreamById is https://crbug.com/698163, and seems quite feasible to remove.)

Trying to remove now. @ShijunS, can you look at https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/11662035/ and @youennf, can you look at https://bugs.webkit.org/show_bug.cgi?id=170845?

[jan-ivar]

@foolip addStream is literally stream.getTracks().forEach(t => this.addTrack(t, stream))

In Gecko (and the spec) whatever streams a track belongs to is irrelevant to the relationship RTCPeerConnection projects to the remote end. In other words, the streams passed in to addTrack are used solely for their msids.

getLocalStreams() in Firefox sadly returns the streams passed in to addTrack, which is wildly misleading except in the typical configuration.

getRemoteStreams() is somewhat useful, though redundant.

[youennf]

@foolip addStream is literally stream.getTracks().forEach(t => this.addTrack(t, stream))

That is the way addStream is also implemented in WebKit behind a runtime flag.
It also seems the way it should be defined, should it be defined somewhere.

[adam-be]

FYI. This has been brought up before #568

[foolip]

@henbos, how do you view the prospects of ever removing these APIs from Chromium? Can they sensibly co-exist with the track-based bits you're working on, or be defined entirely on top of them?

[taylor-b]

@foolip The only thing that's different from Chrome's implementation of addStream and the shim from Jan-Ivar's comment is that we react to changes in the stream. For example: if I call addStream while a stream contains one track, then add a track, then call createOffer, the offer will represent two tracks.

We should measure how often applications actually do this, but I don't anticipate it being very common. And the workaround in JS (once addTrack is available) would be simple. So I don't expect removing this behavior to be a problem.

[foolip]

@taylor-b, how about getLocalStreams() and getRemoteStreams(), I guess those also only work because we keep track of the streams, not the tracks? Would the "wildly misleading" behavior described in https://github.com/w3c/webrtc-pc/issues/1125#issuecomment-294265922 make sense, or what could these methods do if not removed?

[taylor-b]

@foolip The "wildly misleading" part happens when you use getLocalStreams with addTrack. But if you use getLocalStreams with addStream it should be fine.

[fippo]

and its only wildly misleading in the sense that if you use addTrack, then getLocalStreams and iterate all tracks of those streams and think those tracks will be sent over the peerconnection.

[foolip]

@taylor-b, do you mean it'd be fine to match Gecko's behavior in Chromium?

[youennf]

I know some adaptation layers use MediaStream tracks observing to shim addTrack (something like peerConnection.getLocalStreams()[0].addTrack(track)).
This usage is not showing up if RTCPeerConnection.addTrack is available.

[taylor-b]

@foolip Yes, once we implement addTrack I don't see any problem with matching Gecko's behavior with getLocalStreams.

@youennf That's good to know. Do you know specifically what adaptation layers do this, so I can reach out to them once Chrome implements addTrack?

[youennf]

@taylor-b, you might ping tokbox at least.

Moving webrtc samples to addTrack/ontrack might be a good move too in terms of education. Might require some adapter.js shimed version for Chrome.

[stefhak]

Can we close this Issue as the discussion seem to conclude we should not describe addStream/onaddstream?

[foolip]

IMHO, an issue like this should remain open until there's either a plan to add it back to the spec (with tests) or a plan to have it removed from all implementations (with spec bugs and negative tests). The discussion so far is mostly about what it might make sense to spec, not about how to remove it.

[youennf]

There seems to be consensus on how addStream should be implemented if supported.
Would be good to record somehow this consensus.
Maybe filing bugs on Chromium (and Edge?) might be good enough.

[foolip]

Unless there's some implementer who plans to not support addStream, then it would be best to describe it in some spec. Otherwise, we can't write shared web-platform-tests and the results might be less interoperable than with shared tests.

[youennf]

addStream is a legacy API and as other WebRTC legacy API, it might not make it into WebKit.
But it might be shamed so it is best to have a spec to refer to it, ideally in WebRTC legacy API section.

[foolip]

FWIW, if any browser (WebKit-based or otherwise) successfully shipped WebRTC without addStream and didn't run into any breakage, that'd suggest that the compat risk is not as high as it seems. In that case, trying to remove it from the other browsers might be tractable.

[foolip]

However, unless someone who has already shipped it wants to pursue removal, then I do think spec'ing it is best. addStream seems easy, less obvious (to me) is the precise behavior of getLocalStreams and getRemoteStreams.

[fippo]

We're beating around the bush. The decision to move away from addStream and towards addTrack was done a while ago (2013 or 2014?).

Can someone who still remembers the technical reasons for doing this repeat them or point to a summary of the decision?
Does the rationale for the decision still hold? Does it still hold given that in May 2017 native interoperability of addTrack is still not a thing?

✋ 🎤

[stefhak]

The history of us adding the removed 'offerToReceive*' back in with #1005 leading to us (according to @fippo in https://github.com/w3c/webrtc-pc/issues/1383#issuecomment-308691938) being in a rabbit hole tells me we should be careful with pulling deprecated stuff back in.

[fippo]

@stefhak make sure you are sitting comfortably when reading https://bugs.chromium.org/p/webrtc/issues/detail?id=7815

It looks like there are different interpretations of the stream model even and how to do audio->audio/video.

[fippo]

btw, trying to shim addTrack on top of addStream (kids, do not try this at home) in Chrome it turned out that

pc.addTrack(audioStream, audiovideoStream);
pc.getLocalStreams()[0].getTracks(); // contains audio and video

is somewhat consistent and that

pc.addTrack(audioStream, someStream)
...
stream = pc.getLocalStreams()[0];
stream.addTrack(videostream)
pc.addTrack(videostream, stream)

is how people use the combination in the wild.

[stefhak]

@fippo to me it looks like Chrome has a bug when I look at https://bugs.chromium.org/p/webrtc/issues/detail?id=7815
For https://github.com/w3c/webrtc-pc/issues/1125#issuecomment-308708976 I don't fully get the point, can you elaborate?

[fippo]

@stefhak the Chrome behaviour is consistent with the addStream-model on the other side. If a track gets added to a remote stream, onaddtrack is called on the stream without things going through the peerconnection. Now in the addTrack-model it makes no sense. @taylor-b actually mentioned this back in April.

The first part of my second comment was a comment on @jan-ivar's "wildy misleading".
The second part was about unexpected combinations we need to take into account when specifying addStream ontop of addTrack.

[fippo]

@jan-ivar @youennf actually addStream is not quite as simple (at least when used together with addTrack), see @henbos' comment here

pc.addTrack(video, audioVideoStream);
pc.addStream(audioVideoStream)

must not throw an error while changing pc.getSenders(). The naive variant would throw after adding the audio track

[jan-ivar]

@fippo @henbos FWIW I don't think it's worth making addStream add all or nothing in this edge case.

Since addStream is not in the spec, I gather browsers are free to handle legacy as they see fit. But I prefer the narrowest definition of "legacy code" here, which is code already written. I doubt much (if any) of it suffers from this problem.

Firefox has polyfilled addStream for almost 3 years, so for our backwards compat I think it makes sense to stick with what we already have.

[henbos]

SGTM

[foolip]

That sounds great, @jan-ivar! Does anyone volunteer to turn that into spec lingo?

[jan-ivar]

Note I was merely offering advice on implementation. With one browser looking at not implementing addStream, might a note on how to polyfill it suffice?

[foolip]

If more than one implementation wants to keep supporting addStream for the foreseeable future, then having it written down in a spec means that it'll possible to write tests for in web-platform-tests, ensuring a baseline of interoperability.

Are there reasons to not want to spec this like any other feature, other than the work involved with writing the spec change? (Going on vacation, bye!)

[nils-ohlmeier]

The reason I would not want addStream in the spec is, because as a browser developer I want to maintain as little code as possible. As soon as something is in the spec people will expect it to work and be usable. My understanding is that we want to developers to switch to the new implementations as soon as possible so we can delete that old legacy code. Documenting and testing old deprecated code is not helping achieving that goal.

[jan-ivar]

The reason to not spec it is we don't want web developers writing new code using addStream.

addStream is death to having the right mental model of a peer connection. It's API 101: It hurts comprehension to have 3 competing mental models for the same thing when only one is right. Yet addStream/removeStream is the API of least resistance and the most problems. The dumb path.

Problems: if users add or remove tracks from a stream in any way, it becomes unclear fast what pc.addStream and pc.removeStream do. Users may leave tracks behind, send bits twice, or have connections not update to changes at all. They'll suffer poor port-reuse, and slow call-setup.

Least resistance: MDN's addStream page is a sea of red warnings, yet nobody seems to know or care how addTrack and addTransceiver work. They're to addStream what taking cod liver oil is to soda.

Lastly, Firefox is at a disadvantage if people take the dumb path; we don't implement removeStream.

[fippo]

@foolip the interactions between the legacy streams API and the track-based (not to mention transceivers) result in a lot of weird behaviour where we don't even agree what the right thing is. I have to spend a lot of time discussing with @henbos + @jan-ivar here and here

[stefhak]

I don't think there is support for putting addStream/removeStream (or getLocal/RemoteStreams) back into the spec. In addition, the experience from putting offerToReceive back into the spec is not encouraging. I intend to close this Issue with the comment "won't fix" unless I hear strong objection.

[ibc]

Answering the issue question "_Should the spec describe addStream/onaddstream as legacy API?_":

NO. It's proven to be a bad, wrong and problematic API (I won't repeat again all the rationale already given about it).

What we (and all the users) should do is complain against Chrome for keeping it for years while all its competitors have already moved to (at least) the track based API.

And no, addStream() and addTrack() cannot live together.

[stefhak]

Closing this issue as per the discussion above.

[foolip]

FYI, since this issue, Safari has also shipped addStream(), so all of Chrome, Edge, Firefox and Safari now support addStream() and the "addstream" event in some fashion.

[youennf]

addStream(), so all of Chrome, Edge, Firefox and Safari now support addStream() and the "addstream" event in some fashion.

addStream is not exposed by default in Safari and will probably never be.

[foolip]

addStream is not exposed by default in Safari and will probably never be.

I've confirmed on my own MacBook, another physical MacBook and on BrowserStack that Safari 11.0.2 does expose addStream(). Shipped by accident?

[youennf]

nother physical MacBook and on BrowserStack that Safari 11.0.2 does expose addStream(). Shipped by accident?

Yes, this is now fixed in WebKit ToT.

[henbos]

FYI the legacy stream-based APIs (getLocalStreams, getRemoteStreams, addStream, removeStream but not the onaddstream event) are redefined on top of track-based APIs in M66 (PSA).