Temasys / SkylinkJS

SkylinkJS Javascript WebRTC SDK
http://skylink.io/web
Other
275 stars 57 forks source link

"incomingStream" event is intermittent #216

Closed amangupta26 closed 8 years ago

amangupta26 commented 8 years ago

After joining the room, "incomingStream" is showing intermittent behavior as some times this event is received and some times not. Page has to be refreshed multiple times to get the peer stream.

This is how I am getting skylink(which I guess takes the latest stable release) <script src="//cdn.temasys.com.sg/skylink/skylinkjs/0.6.x/skylink.complete.min.js"></script>

oooookk7 commented 8 years ago

Hi @amangupta26, could you check your logs and copy and paste when "incomingStream" doesn't get triggered.

amangupta26 commented 8 years ago

I am always getting "peerJoined" event whenever a user joins the room. Can you be more specific about logs, I mean which event or method should I look into?

oooookk7 commented 8 years ago

Hi @amangupta26, the logs is the Web Console logs which you can get for an instance in Chrome Developer Tools.

Here are examples how you can get the logs from: Chrome: https://developer.chrome.com/devtools/docs/console Firefox: https://developer.mozilla.org/en/docs/Tools/Web_Console

After opening the console, you may copy and paste the logs here.

amangupta26 commented 8 years ago

skylink.complete.min.js:10 Uncaught TypeError: Cannot read property 'sid' of null

Above error I get occasionally.

oooookk7 commented 8 years ago

Ok noted. Perhaps could you share us your codes on how you are using SkylinkJS?

For preference to keep the sharing of codes and debugging things with you private, we can move our chat over to support.temasys.com.sg.

oooookk7 commented 8 years ago

By any chance as well are you using getUserMedia() before invoking joinRoom()?

amangupta26 commented 8 years ago

No, Following is the order in which method are called,

skylink.init
skylink.joinRoom
skylink.sendStream
oooookk7 commented 8 years ago

Ok. Is the callback function parameter used by any chance?

Sample code with callbacks:

var skylink = new Skylink();

skylink.init('xxxx-api-key-xxxxxx', function (initError, initSuccess) {
   if (initError) {
     // reactToError
     return;
   }

  skylink.joinRoom(function (jRError, jRSuccess) {
     if (jRError) {
        // reactToError
        return;
     }

     skylink.sendStream({ audio: [true/false], video: [true/false] });
  });
});
oooookk7 commented 8 years ago

There is a noted bug that we will resolve in 0.6.12 version with a similar error you posted with .sid being null. But this error will not occur when user has joined the room according to the order of steps you have stated.

amangupta26 commented 8 years ago

No I have not added the callback, I will add and give you relevant console log.

oooookk7 commented 8 years ago

Ok thanks.

oooookk7 commented 8 years ago

@amangupta26 btw tips up if you are trying to make a user join the room with audio and video, you can use this without invoking sendStream() function. Unless you want the user to join the room without any stream and then the user is able to send stream later.

Example with joining room with audio and video:

skylink.joinRoom({ audio: [true/false], video: [true/false] }, function (jRError, jRSuccess) {
     if (jRError) {
        // reactToError
        return;
     }
  });
oooookk7 commented 8 years ago

On another note, I've added the ticket #217 that sounds related to the error thrown. This fix should allow you to invoke getUserMedia() before joinRoom().

amangupta26 commented 8 years ago

Yes, the use case for us is that, we want the user the room and then send the stream later(on a click of button), and hence in joinRoom method, I am setting audio and video as "false".

Also, I applied the callback you asked for, I did not get error callback.

Following three scenarios are happening randomly(suppose A and B are users): 1) Both A and B can see each other 2) A can watch B but B cant or vice-versa 3) Both cant watch each other

When one user cant watch other user, it is because incomingStream is not triggered.

oooookk7 commented 8 years ago

Okay. I would like to check with you if peerJoined event is triggered for A and B in these scenarios.

Also on the other hand, it might be useful to log down using console.log for events iceConnectionState, candidateGenerationState, peerConnectionState and handshakeProgress. This may allow us to delve and gather more information about the connection status.

amangupta26 commented 8 years ago

Alright, I have checked peerJoined , it is getting triggered correctly. I will check for others and let you know.

oooookk7 commented 8 years ago

Thanks!

amangupta26 commented 8 years ago

Logs when both users have each others video


common.js:296 handshakeProgress welcome VUdf0PqI9q6Wik4yADnE
common.js:291 peerConnectionState have-remote-offer VUdf0PqI9q6Wik4yADnE
common.js:296 handshakeProgress offer VUdf0PqI9q6Wik4yADnE
common.js:291 peerConnectionState stable VUdf0PqI9q6Wik4yADnE
common.js:296 handshakeProgress answer VUdf0PqI9q6Wik4yADnE
common.js:287 candidateGenerationState completed VUdf0PqI9q6Wik4yADnE
common.js:282 iceConnectionState connected VUdf0PqI9q6Wik4yADnE
common.js:282 iceConnectionState completed VUdf0PqI9q6Wik4yADnE
common.js:291 peerConnectionState have-remote-offer VUdf0PqI9q6Wik4yADnE
common.js:296 handshakeProgress offer VUdf0PqI9q6Wik4yADnE
common.js:291 peerConnectionState stable VUdf0PqI9q6Wik4yADnE
common.js:296 handshakeProgress answer VUdf0PqI9q6Wik4yADnE

handshakeProgress enter VUdf0PqI9q6Wik4yADnE
common.js:296 handshakeProgress enter as77xR32hS4mmR9ZADnG
common.js:296 handshakeProgress welcome as77xR32hS4mmR9ZADnG
common.js:291 peerConnectionState have-local-offer as77xR32hS4mmR9ZADnG
common.js:296 handshakeProgress offer as77xR32hS4mmR9ZADnG
common.js:291 peerConnectionState stable as77xR32hS4mmR9ZADnG
common.js:282 iceConnectionState checking as77xR32hS4mmR9ZADnG
common.js:296 handshakeProgress answer as77xR32hS4mmR9ZADnG
common.js:287 candidateGenerationState completed as77xR32hS4mmR9ZADnG
common.js:282 iceConnectionState connected as77xR32hS4mmR9ZADnG
common.js:282 iceConnectionState completed as77xR32hS4mmR9ZADnG
common.js:291 peerConnectionState have-local-offer as77xR32hS4mmR9ZADnG
common.js:296 handshakeProgress offer as77xR32hS4mmR9ZADnG
common.js:291 peerConnectionState stable as77xR32hS4mmR9ZADnG
common.js:296 handshakeProgress answer as77xR32hS4mmR9ZADnG```
amangupta26 commented 8 years ago

When one user does not have the peer's video

A not peer video handshakeProgress enter cvq0CJL5nOYW4QHiADm5 common.js:296 handshakeProgress enter gdMkdBW3WAIEZMBpADm8 common.js:296 handshakeProgress welcome gdMkdBW3WAIEZMBpADm8 common.js:291 peerConnectionState have-local-offer gdMkdBW3WAIEZMBpADm8 common.js:296 handshakeProgress offer gdMkdBW3WAIEZMBpADm8 common.js:291 peerConnectionState stable gdMkdBW3WAIEZMBpADm8 common.js:282 iceConnectionState checking gdMkdBW3WAIEZMBpADm8 common.js:296 handshakeProgress answer gdMkdBW3WAIEZMBpADm8 common.js:287 candidateGenerationState completed gdMkdBW3WAIEZMBpADm8 common.js:282 iceConnectionState connected gdMkdBW3WAIEZMBpADm8 common.js:282 iceConnectionState completed gdMkdBW3WAIEZMBpADm8 common.js:291 peerConnectionState have-local-offer gdMkdBW3WAIEZMBpADm8 common.js:296 handshakeProgress offer gdMkdBW3WAIEZMBpADm8 common.js:291 peerConnectionState stable gdMkdBW3WAIEZMBpADm8 common.js:296 handshakeProgress answer gdMkdBW3WAIEZMBpADm8

B has peer video

handshakeProgress enter gdMkdBW3WAIEZMBpADm8 common.js:296 handshakeProgress welcome cvq0CJL5nOYW4QHiADm5 common.js:291 peerConnectionState have-remote-offer cvq0CJL5nOYW4QHiADm5 common.js:296 handshakeProgress offer cvq0CJL5nOYW4QHiADm5 common.js:291 peerConnectionState stable cvq0CJL5nOYW4QHiADm5 common.js:296 handshakeProgress answer cvq0CJL5nOYW4QHiADm5 common.js:287 candidateGenerationState completed cvq0CJL5nOYW4QHiADm5 common.js:282 iceConnectionState connected cvq0CJL5nOYW4QHiADm5 common.js:282 iceConnectionState completed cvq0CJL5nOYW4QHiADm5 common.js:291 peerConnectionState have-remote-offer cvq0CJL5nOYW4QHiADm5 common.js:296 handshakeProgress offer cvq0CJL5nOYW4QHiADm5 common.js:291 peerConnectionState stable cvq0CJL5nOYW4QHiADm5 common.js:296 handshakeProgress answer cvq0CJL5nOYW4QHiADm5

oooookk7 commented 8 years ago

Thanks @amangupta26, we will investigate and resolve them. Would keep you updated.

amangupta26 commented 8 years ago

both users has no peer video

handshakeProgress enter X4mtx9XxqhMROFImADnw common.js:296 handshakeProgress welcome 7KhtZqSn3qI7cImMADnv common.js:291 peerConnectionState have-remote-offer 7KhtZqSn3qI7cImMADnv common.js:296 handshakeProgress offer 7KhtZqSn3qI7cImMADnv common.js:291 peerConnectionState stable 7KhtZqSn3qI7cImMADnv common.js:296 handshakeProgress answer 7KhtZqSn3qI7cImMADnv common.js:282 iceConnectionState checking 7KhtZqSn3qI7cImMADnv common.js:287 candidateGenerationState completed 7KhtZqSn3qI7cImMADnv common.js:291 peerConnectionState have-remote-offer 7KhtZqSn3qI7cImMADnv common.js:296 handshakeProgress offer 7KhtZqSn3qI7cImMADnv common.js:291 peerConnectionState stable 7KhtZqSn3qI7cImMADnv common.js:296 handshakeProgress answer 7KhtZqSn3qI7cImMADnv common.js:282 iceConnectionState connected 7KhtZqSn3qI7cImMADnv common.js:282 iceConnectionState completed 7KhtZqSn3qI7cImMADnv

handshakeProgress enter 7KhtZqSn3qI7cImMADnv common.js:296 handshakeProgress enter X4mtx9XxqhMROFImADnw common.js:296 handshakeProgress welcome X4mtx9XxqhMROFImADnw common.js:291 peerConnectionState have-local-offer X4mtx9XxqhMROFImADnw common.js:296 handshakeProgress offer X4mtx9XxqhMROFImADnw common.js:291 peerConnectionState stable X4mtx9XxqhMROFImADnw common.js:282 iceConnectionState checking X4mtx9XxqhMROFImADnw common.js:296 handshakeProgress answer X4mtx9XxqhMROFImADnw common.js:287 candidateGenerationState completed X4mtx9XxqhMROFImADnw common.js:282 iceConnectionState connected X4mtx9XxqhMROFImADnw common.js:282 iceConnectionState completed X4mtx9XxqhMROFImADnw

oooookk7 commented 8 years ago

Just checking. The logs seems fine so far. But to delve a little more. I would need your help to use a hack using your skylink class object:

Getting the ICE connection state:

skylink._peerConnections["<Peer ID>"].iceConnectionState

Getting the Peer connection state:

skylink._peerConnections["<Peer ID>"].signalingState

Getting the ICE gathering state:

skylink._peerConnections["<Peer ID>"].iceGatheringState

I suggest you type those in the console logs when the scenario happens for both peers on each end and tell us the results.

amangupta26 commented 8 years ago

At what stage should I get these logs? On peerJoined will be fine?

oooookk7 commented 8 years ago

You can open the Web Console for both Peers. In this example, I'm opening the Console from Peer B and seeing the Peer connection object of A by entering A's Peer ID (In this example it is "ACiCUEavJmWrOUHBADsr") into the object array.

hack-example-to-view-peerconnection-object-connection-stats

For this example as well, Demo.Skylink is the skylink class object reference.

amangupta26 commented 8 years ago
screen shot 2016-03-29 at 2 52 06 pm screen shot 2016-03-29 at 2 53 05 pm
oooookk7 commented 8 years ago

Thanks @amangupta26. That was useful. Perhaps to check .getRemoteStreams() if it returns an Array with one stream object other than checking iceConnectionState, iceGatheringState or signalingState.

amangupta26 commented 8 years ago

How to check getRemoteStreams?

oooookk7 commented 8 years ago

The same way you get the .iceConnectionState.

For example:

Demo.Skylink._peerConnections["Mh1qJBjVQE6SfkP2ADw8"].getRemoteStreams()

I suggest to do this for both Peer A and B. This checks if stream object was received in the connection object itself.

amangupta26 commented 8 years ago
screen shot 2016-03-29 at 3 03 16 pm screen shot 2016-03-29 at 3 02 21 pm
oooookk7 commented 8 years ago

Ok this seems strange. In regards to the previous format, did you get "incomingStream" event triggered?. Might be useful to add console.log for "incomingStream" event.

The connection status seems to be working fine.

amangupta26 commented 8 years ago

If a user does not receive the peer video then incomingStream is not triggered.

oooookk7 commented 8 years ago

Ok. Do you happen to be using any Media Relay? Additionally, it might be easier to interact with us in support (it's a private session) and share with us your code. We will help you debug to ensure that your use-case works and diagnose what is the issue.

Support link: http://support.temasys.com.sg

amangupta26 commented 8 years ago

In one of the screenshots, the iceConnectionState for one user is completed and for other connected. It is fine?

oooookk7 commented 8 years ago

@amangupta26 it is fine. "connected" means that the ICE agent has found a usable connection but still checking for any other available connections and "completed" means that it found a usable connection but not checking for any other available connections.

oooookk7 commented 8 years ago

We tested the following and it's working for us.

Demo.Skylink.on('incomingStream', function (peerId, stream, isSelf, peerInfo){
  var peerVideo = document.getElementById(peerId);

  if (!peerVideo) {
    peerVideo = document.createElement('video');
    peerVideo.id = peerId;
    peerVideo.autoplay = 'autoplay';
    peerVideo.muted = isSelf;
    document.getElementById('#peer-list').append(peerVideo);
  }
  attachMediaStream(peerVideo, stream);
});

Demo.Skylink.init(config, function (error, success) {
  if (success) {
    Demo.Skylink.joinRoom({
      userData: displayName,
      audio: false,
      video: false
    }, function (jRError, jRSuccess) {
      if (jRSuccess) {
        Demo.Skylink.sendStream({ audio: true, video: true});
      }
    });
  }
});

Please tell us if we are missing anything. In any case, we advise we move to support (open a ticket) and we can start communicating where you can share your codes privately.

You can open a new ticket in support.temasys.com.sg and put this github issue link to the description so I can follow up with you.

Or if you don't mind, you can share your codes in here. (Note that this is public space).

amangupta26 commented 8 years ago

If I am sending audio and video as true in joinRoom and not calling sendStream on button click, things seem to work fine.

amangupta26 commented 8 years ago

sendStream is send after a while when user joins the room for my use case(on a button click). The code you have presented will send the stream as soon as user joins the room.

oooookk7 commented 8 years ago

Ok that's strange. Thanks for the response. Just tips up, sendStream() function works only after user has successfully joined the room. So I suggest to only display the UI button after user has successfully joined the room. If you would like to get the stream before joining the room, user has to use getUserMedia() function (which we have a bug ticket we are working on it for 0.6.12 release).

Demo.Skylink.joinRoom(function (error, success) {
  if (success) {
    document.getElementById('button').style.display = 'block';
  }
});
amangupta26 commented 8 years ago

I think there should be an error callback if user has not joined the room yet and sendStream is invoked?

oooookk7 commented 8 years ago

The sendStream() function itself has a callback that should throw an error if user is not in the room. We are working on that fixes until we clear our priorities we have. For your use-case to work, that's the solution and it would be in a correct steps.

oooookk7 commented 8 years ago

The joinRoom() callback only ties to user to room connection. sendStream() callback pertains if stream is retrieved successfully and sent to connection.

amangupta26 commented 8 years ago

Yes, enabling button after successful callback of joinRoom looks like is working fine.

oooookk7 commented 8 years ago

Ok. Seems like your use-case is settled. If there's no other issues, I'll be closing this ticket

amangupta26 commented 8 years ago

Ya sure. Thanks for your time.

oooookk7 commented 8 years ago

Welcomed. And thanks for using SkylinkJS. :)