[jitsi-dev] edge shim: does lib-jitsi-meet rely on remote MediaStream.id values?


#1

Hi,

There is no way to manually create a MediaStream object with a given
id. Instead such a id is a read-only and internally generated
attribute. The ORTC API does not expose MediaStream objects at all,
but just MediaStreamTracks. So the shim manually creates MediaStream
objects.

When inspecting the remote SDP, both MediaStream.id and
MediaStreamTrack.id are signaled (msid and/or mslabel+label in a=ssrc
lines). However, I won't be able to replicate the remote
MediaStream.id.

Is this a problem for the lib? Does the lib relies on remote
MediaStreams matching the id signaled in the remote SDP?

···

--
Iñaki Baz Castillo
<ibc@aliax.net>


#2

BTW: the same for remote MediaStreamTrack.id. In ORTC remote tracks
are created after calling RTCRtpReceiver.receive(), and they have a
random read-only id attribute.

I hope lib-jitsi.meet, just relies on SSRC values to associate tracks and users.

···

2017-05-09 0:51 GMT+02:00 Iñaki Baz Castillo <ibc@aliax.net>:

There is no way to manually create a MediaStream object with a given
id. Instead such a id is a read-only and internally generated
attribute. The ORTC API does not expose MediaStream objects at all,
but just MediaStreamTracks. So the shim manually creates MediaStream
objects.

When inspecting the remote SDP, both MediaStream.id and
MediaStreamTrack.id are signaled (msid and/or mslabel+label in a=ssrc
lines). However, I won't be able to replicate the remote
MediaStream.id.

Is this a problem for the lib? Does the lib relies on remote
MediaStreams matching the id signaled in the remote SDP?

--
Iñaki Baz Castillo
<ibc@aliax.net>


#3

Well, indeed this is a huge problem. Basically, when the
pc.onaddstream and/or the remoteStream.onaddtrack events fire, they
provide a MediaStream or MediaStreamTrack whose id does not exist in
the remote SDP at all. In fact, the lib fails:

[JitsiMeetJS.js] <getGlobalOnErrorHandler>: UnhandledError: null
Script: null Line: null Column: null StackTrace: Error: No SSRC lines
for streamId 4F1EC7D1-2FDA-4938-8989-B82483E581CA for remote track,
media type: audio

So I suggest the following to make this work:

- Add a stream.jitsiI_id and track.jitsi_id properties on remote
MediaStream and MediaStreamTrack instances generated by the ORTC shim.

- Such a jitsi_id value will point to the corresponding stream.id of
track.id signaled in the remote SDP.

- lib-jitsi-meet should always check those jitsi_id fields (or just
use the id if jitsi_id does not exist).

Thoughts?

···

2017-05-09 1:30 GMT+02:00 Iñaki Baz Castillo <ibc@aliax.net>:

2017-05-09 0:51 GMT+02:00 Iñaki Baz Castillo <ibc@aliax.net>:

There is no way to manually create a MediaStream object with a given
id. Instead such a id is a read-only and internally generated
attribute. The ORTC API does not expose MediaStream objects at all,
but just MediaStreamTracks. So the shim manually creates MediaStream
objects.

When inspecting the remote SDP, both MediaStream.id and
MediaStreamTrack.id are signaled (msid and/or mslabel+label in a=ssrc
lines). However, I won't be able to replicate the remote
MediaStream.id.

Is this a problem for the lib? Does the lib relies on remote
MediaStreams matching the id signaled in the remote SDP?

BTW: the same for remote MediaStreamTrack.id. In ORTC remote tracks
are created after calling RTCRtpReceiver.receive(), and they have a
random read-only id attribute.

I hope lib-jitsi.meet, just relies on SSRC values to associate tracks and users.

--
Iñaki Baz Castillo
<ibc@aliax.net>


#4

Please excuse my ignorance. Are "edge shim" and, respectively, this
thread about MediaStream and MediaStreamTrack ids about
https://github.com/webrtc/adapter/tree/master/src/js/edge ?


#5

Actually I mean my work on a RTCPeerConnection Plan-B shim for ORTC (Edge):

https://github.com/ibc/lib-jitsi-meet/tree/edge-pc

···

2017-05-09 7:53 GMT+02:00 Lyubomir Marinov <lyubomir.marinov@jitsi.org>:

Please excuse my ignorance. Are "edge shim" and, respectively, this
thread about MediaStream and MediaStreamTrack ids about
https://github.com/webrtc/adapter/tree/master/src/js/edge ?

--
Iñaki Baz Castillo
<ibc@aliax.net>


#6

Even better, make RTCUtils.getStreamID() return that when Edge, and
add a RTCUtils.getTrackID().

···

2017-05-09 4:33 GMT+02:00 Iñaki Baz Castillo <ibc@aliax.net>:

So I suggest the following to make this work:

- Add a stream.jitsiI_id and track.jitsi_id properties on remote
MediaStream and MediaStreamTrack instances generated by the ORTC shim.

- Such a jitsi_id value will point to the corresponding stream.id of
track.id signaled in the remote SDP.

- lib-jitsi-meet should always check those jitsi_id fields (or just
use the id if jitsi_id does not exist).

--
Iñaki Baz Castillo
<ibc@aliax.net>


#7

Why not doing it through define property ?

https://github.com/webrtc/adapter/blob/master/src/js/edge/rtcpeerconnection_shim.js#L735

We use both stream and track IDs, but they are read only. The only
things we ever modify are local SSRCs.

···

On Tue, May 9, 2017 at 5:38 AM, Iñaki Baz Castillo <ibc@aliax.net> wrote:

2017-05-09 4:33 GMT+02:00 Iñaki Baz Castillo <ibc@aliax.net>:

So I suggest the following to make this work:

- Add a stream.jitsiI_id and track.jitsi_id properties on remote
MediaStream and MediaStreamTrack instances generated by the ORTC shim.

- Such a jitsi_id value will point to the corresponding stream.id of
track.id signaled in the remote SDP.

- lib-jitsi-meet should always check those jitsi_id fields (or just
use the id if jitsi_id does not exist).

Even better, make RTCUtils.getStreamID() return that when Edge, and
add a RTCUtils.getTrackID().

--
Iñaki Baz Castillo
<ibc@aliax.net>

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev


#8

Why not doing it through define property ?

https://github.com/webrtc/adapter/blob/master/src/js/edge/rtcpeerconnection_shim.js#L735

I thought about that, but honestly, I've seen overriding a read-only
property work in some versions of a browser (Safari) and failing on
newer versions of the same browser. I'd prefer not going that way.

We use both stream and track IDs, but they are read only. The only
things we ever modify are local SSRCs.

RTCUtils already has a getStreamID() getter. I've made it returns a
custom jitsiRemoteId property on remote MediaStream generated by the
Edge shim, and then added a similar getTrackID(). AFAIS it works.

···

2017-05-09 14:04 GMT+02:00 Paweł Domas <pawel.domas@jitsi.org>:

--
Iñaki Baz Castillo
<ibc@aliax.net>


#9

I've just tested that in Edge (using Object.defineProperty(stream,
XXX)). It "works" but:

- It may stop working in any new version of Edge, or
- newer versions of Edge may just ignore such a getter, and
- things like stream.getTrackById(XXX) just fail.

···

2017-05-09 14:15 GMT+02:00 Iñaki Baz Castillo <ibc@aliax.net>:

2017-05-09 14:04 GMT+02:00 Paweł Domas <pawel.domas@jitsi.org>:

Why not doing it through define property ?

https://github.com/webrtc/adapter/blob/master/src/js/edge/rtcpeerconnection_shim.js#L735

I thought about that, but honestly, I've seen overriding a read-only
property work in some versions of a browser (Safari) and failing on
newer versions of the same browser. I'd prefer not going that way.

--
Iñaki Baz Castillo
<ibc@aliax.net>


#10

If it is expected to be causing issues then we would be probably good
with some utility method in RTCUtils.

But how is "jitsi_id" property different that this getter ? Can we
make 'id' a custom property ? I must be missing something...

···

On Tue, May 9, 2017 at 10:50 AM, Iñaki Baz Castillo <ibc@aliax.net> wrote:

2017-05-09 14:15 GMT+02:00 Iñaki Baz Castillo <ibc@aliax.net>:

2017-05-09 14:04 GMT+02:00 Paweł Domas <pawel.domas@jitsi.org>:

Why not doing it through define property ?

https://github.com/webrtc/adapter/blob/master/src/js/edge/rtcpeerconnection_shim.js#L735

I thought about that, but honestly, I've seen overriding a read-only
property work in some versions of a browser (Safari) and failing on
newer versions of the same browser. I'd prefer not going that way.

I've just tested that in Edge (using Object.defineProperty(stream,
XXX)). It "works" but:

- It may stop working in any new version of Edge, or
- newer versions of Edge may just ignore such a getter, and
- things like stream.getTrackById(XXX) just fail.

--
Iñaki Baz Castillo
<ibc@aliax.net>

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev


#11

But how is "jitsi_id" property different that this getter ? Can we
make 'id' a custom property ? I must be missing something...

I just set stream.jitsiRemoteId = XXX and track.jitsiRemoteId = NNN,
and then in RTCUtils, within the Edge section:

this.getStreamID = function(stream) {
    const id = stream.jitsiRemoteId || stream.id;

    return SDPUtil.filterSpecialChars(id);
};

this.getTrackID = function(track) {
    return track.jitsiRemoteId || track.id;
};

Adding a custom property to a native JS object is safe.

Can we make 'id' a custom property ?

Yes, by using Object.defineProperty, but that's unsafe IMHO due the
rationale given in my previous comment.

···

2017-05-09 17:57 GMT+02:00 Paweł Domas <pawel.domas@jitsi.org>:

--
Iñaki Baz Castillo
<ibc@aliax.net>