Webrtc-pc: specify legacy onaddstream?

[fippo]

I am having trouble porting my application code over from ontrack to onaddstream.

Given bugs like https://bugzilla.mozilla.org/show_bug.cgi?id=1423842 it might be worth specifying how it behaves. The use-case which broke there was roughly this:

pc.onaddstream = (e) => {
  // determine if e.stream is audio-only, video-only or audio-video
  // attach to srcObject
};

ontrack can be used to attach to srcObject but it will fire twice for an audio/video stream which makes it a bit ugly.

[jan-ivar]

I hear two arguments here:

1. specify it so browsers don't accidentally break it implementing transceivers, and
2. Tracks are too hard for users.

On the first one, I'd prefer leaving legacy to browsers here. We missed a timing regression, and have fixed it already.

The second point doesn't seem to be about legacy. We already omit specifying addStream so we shouldn't specify onaddstream either IMHO. The point of the pivot to tracks was to make users think about tracks. Bringing onaddstream back would (still) seem to undermine that, or am I missing something? A good ontrack example might help though.

[jan-ivar]

The message to users here should be "don't rely on legacy".

[fippo]

I hear two arguments here:

No. onaddstream is useful on its own. It happens the first time a stream shows up and lets me subscribe on the stream addtrack and remove events. The case of a stream with no tracks is rather theoretical though ;-)

We missed a timing regression, and have fixed it already.

👏 👏 👏

A good ontrack example might help though.

see example 12. Might be good to show the "is this audio-only or audio-video" decisions that are possible at that stage.

The message to users here should be "don't rely on legacy".

I do not think ignoring that there exists half a decade of usage on the legacy api will be a successful strategy.

[fippo]

btw, I just noticed that while we don't specify addStream, we show how to (trivially) shim it. I can't come up with a similar shim of onaddstream

[jan-ivar]

@fippo Well onaddstream just adds a listener. It's the firing of the addstream event that's hard to polyfill. Can you clarify what the ask here is?

[alvestrand]

@henbos @foolip calling your attention to this bug.

[foolip]

Is this issue just about the "addstream" event or also about addStream()? https://github.com/w3c/webrtc-pc/issues/1125 was closed with no change.

[alvestrand]

We can shim onaddstream (I think) by:

pc.addEventListener(track, function(track, streams) {
   for (stream : streams) {
      if (!pc.__known_streams__.hasMember(stream)) {
          fire pc.stream event
          pc__.known_streams__.push_back(stream);
     }
  }
})

if it's possible to fire an event "as if it had come from the PC". Not sure about that part.

[fippo]

pc.dispatchEvent works, used often enough in adapter (edit: which also happens to have a onaddstream shim that works roughly like what Harald showed)

[henbos]

I think we should either spec all legacy stream APIs (addStream, removeStream, onaddstream, onremovestream) or none of them. Unless we think stream-events are useful in the main sections of the spec (non-legacy).

[foolip]

Firefox doesn't have removeStream() so that case is not as clear cut. Safari does have the removeStream() method but not the onremovestream property, but that might be a mistake, the event could still be fired. @youennf?

[youennf]

Firefox doesn't have removeStream() so that case is not as clear cut. Safari does have the removeStream() method but not the onremovestream property, but that might be a mistake, the event could still be fired. @youennf?

addStream/removeStream are not exposed by default in Safari.
They are only exposed if user explicitly turns on the legacy-API runtime flag.
Plan is to get rid off this legacy API/legacy flag at some point in the future.

adapter.js provides support for them in Safari for legacy websites.

[henbos]

UAs are moving away from stream-based APIs then, good. Then the question is if onaddstream is useful on its own.

Assuming ontrack fires when the stream is in a complete state (#1691), it's not hard to shim onaddstream as @alvestrand suggested above. On the other hand, if users are interested in this it would not be hard to write an ontrack listener like that to get any new streams. Seems a bit redundant, if not for legacy impl reasons?

[foolip]

@youennf, when I collected the Safari 11 data for http://web-confluence.appspot.com I don't think I enabled anything special, and I also see addStream() in Safari 11 in BrowserStack. Are you sure it's disabled by default?

[youennf]

Humm... if so, that is not great. I think we will want to switch off the runtime flag.

[youennf]

Assuming ontrack fires when the stream is in a complete state (#1691), it's not hard to shim onaddstream as @alvestrand suggested above

onaddstream is shimed based on the track event in https://github.com/webrtc/adapter/blob/master/src/js/safari/safari_shim.js

[fippo]

yeah, I was worried about #1691. But the way its written Harald's shim would satisfy the "what kind of stream is it" property I quoted at the top

Not specifying it is then!