meetecho / janus-gateway

Janus WebRTC Server
https://janus.conf.meetecho.com
GNU General Public License v3.0
8.23k stars 2.48k forks source link

Janus JS SDK: 'hangup' method is called twice #752

Closed soulfly closed 7 years ago

soulfly commented 7 years ago

I use the following JS method to hangup:

self.videoRoomPlugin.hangup(true);

so it produces a request to Janus backend: https://github.com/meetecho/janus-gateway/blob/master/html/janus.nojquery.js#L2122

here is a log of this request: https://www.evernote.com/l/ANiopvJoyqBJp58P91qGdm51pFQcURe087sB/image.png

and then I receive the 'success' response: https://www.evernote.com/l/ANjA6dETsFBNTIP6pd-jg_7XFOlPxM7FPOYB/image.png

But then, I receive another 'hangup' event from Janus backend: https://www.evernote.com/l/ANjF9NshjftItr2O5msAZ9kBNP_MlY6qqpwB/image.png

which then produces another call of 'hangup' method here: https://github.com/meetecho/janus-gateway/blob/master/html/janus.nojquery.js#L476

So the question is do we need that 'hangup' event from server in this case when user manually calls it?

The main problem here is that I can have the following code to switch camera:

self.videoRoomPlugin.hangup(true);

var constraints = {
        audio: true,
        video: {deviceId: {exact: mediaDeviceId}}
};

navigator.mediaDevices.getUserMedia(constraints)
.then(function(stream) {
    self.videoRoomPlugin.createOffer({stream: stream});
})
.catch(function(error) {
});

which creates new PeerConnection for this handler inside 'createOffer'. But this new PeerConnection can be suddenly cleaned just right here https://github.com/meetecho/janus-gateway/blob/master/html/janus.nojquery.js#L476 because of this event from server.

So I think that Janus backend should not send this 'hangup' event in this case.

lminiero commented 7 years ago

Trying to create a new PeerConnection right after it was closed wouldn't work anyway, due to the timing associated with garbage collection for WebRTC stuff (something that should be fixed in the reference counters branch).

soulfly commented 7 years ago

Yes, I noticed the same and then did a trick, so now it works:

switchVideoinput: function(mediaDeviceId){

    self.videoRoomPlugin.hangup(true);

    var constraints = {
          audio: true,
          video: {deviceId: {exact: mediaDeviceId}}
    };

    navigator.mediaDevices.getUserMedia(constraints)
    .then(function(stream) {
        setTimeout(function(){
            self.videoRoomPlugin.createOffer({stream: stream});
        }, 1000);
    })
    .catch(function(error) {
    });
}
lminiero commented 7 years ago

Cool, closing then!