webRTC-io / webrtc.io-client

drop-in client code for webrtc.io Highly experimental technology
143 stars 96 forks source link

Uncaught TypeError, video streams not working #30

Open FREEZX opened 11 years ago

FREEZX commented 11 years ago

Testing the demo at http://webrtc.dennis.is/ i noticed the following:

Whenever i refresh the site in two different tabs simultaneously and allow them both within a short interval of each other, one of the tabs reports errors and both tabs have no video stream. This occurs once in a few tries. The errors reported in the console are the following:

Uncaught TypeError: Cannot call method 'setRemoteDescription' of undefined webrtc.io.js:233

Uncaught TypeError: Cannot call method 'addIceCandidate' of undefined webrtc.io.js:123

Sometimes errors are not reported but the two different tabs do not have the two streams attached.

dennismartensson commented 11 years ago

This is a very unusual real life problem. But i thing both the tabs starts to create the connection at the same time.

FREEZX commented 11 years ago

This is an issue that affects usability of webRTC. The only workaround for now is to enforce one of the users to create the connection and then invite the other person

mudcube commented 11 years ago

I've attempted to recreate this issue quite a few times, both on http://webrtc.dennis.is and on https://localhost/webrtc/ which auto authorizes [since it's SSL] after the first load (so both tabs load pretty much as closely to the exact same time), but haven't had any luck in reproducing.

FREEZX commented 11 years ago

Try loading up http://webrtc.dennis.is in two tabs before allowing camera and microphone access.

I can notice the typeerrors are now gone but the video of the other user is empty in at least one of the two tabs. I'm using google chrome version 27.0.1453.12 dev

mudcube commented 11 years ago

Ok, I ran into that same issue about a week ago (when video did not connect because one event fired before it was expected to), but then made some modifications to the codebase, and seemed to fix the issue on my local machine. You can grab the updates on my branch, or wait for them to be applied to this repo.

sarenji commented 11 years ago

Try again, I've merged @mudcube 's changes.

kishorenc commented 11 years ago

The issue still persists after your latest merge. Here's how I reproduce it locally:

  1. Load in tab 1 without giving permission
  2. Load in tab 2 without giving permission
  3. "Allow" in tab 1
  4. "Allow" in tab 2

End result: The following errors in tab 2:

Uncaught TypeError: Cannot call method 'setRemoteDescription' of undefined webrtc.io.js:333
Uncaught TypeError: Cannot call method 'addIceCandidate' of undefined webrtc.io.js:194
kishorenc commented 11 years ago

I investigated this issue further. The problem is that rtc.createPeerConnection() is never called for Tab 2. Therefore none of the sockets have a peer connection established. So, when you click on "Allow" in Tab 1, Tab 2 tries to send an answer and fails with the peer connection undefined error.

The rtc.createPeerConnection() is called in 2 places, neither of which is applicable for Tab 2:

Fix

The quick fix for this is to add a check for peer connection and initialize if it's undefined in sendAnswer:

rtc.sendAnswer = function(socketId, sdp) {
    var pc = rtc.peerConnections[socketId];
    if(!pc) pc = rtc.createPeerConnection(socketId);   // The FIX

The question I have though is why peer connection initialization is deferred till the getUserMedia succeeds? Can we initialize it on the get_peers event like this:

rtc.on('get_peers', function(data) {
    rtc.connections = data.connections;
    rtc._me = data.you;
    rtc.createPeerConnections();   // initialize peer connections 

    // fire connections event and pass peers
    rtc.fire('connections', rtc.connections);
});

I have working patches for both the fixes which I can submit. Wanted to discuss to be sure first.

mudcube commented 11 years ago

Hi kishorenc, it looks like http://webrtc.dennis.is/ not on the latest libs, so that would still be hitting issues. It sounds like you ran them on your localserver and came up with the same issues though?

FREEZX commented 11 years ago

This is still happening from time to time and i'm running this on my local server. I also noted the 'add remote stream' event gets fired twice for the second user that allows camera access.

FREEZX commented 11 years ago

I managed to reproduce it again and got a screenshot of the stacktrace, if it can be of any use. image

asfin commented 11 years ago

I've had same problem, i've found workaround that works for me.

rtc.createStream({
    video: true,
    audio: true
}, function (stream) {
    <...>
    rtc.connect(server, room_id);
});