twilio / twilio-video.js

Twilio’s Programmable Video JavaScript SDK
https://www.twilio.com/docs/video/javascript
Other
571 stars 217 forks source link

trackSubscribed sometimes doesn’t fire after upgrading SDK to 2.0.0 #857

Closed patricktemple closed 4 years ago

patricktemple commented 4 years ago

Hi,

Our app has a JS client (Chrome 79) connecting to an iOS client (SDK version 2.10) in P2P rooms. The iOS clients sends two video tracks to web (phone camera and screenshare, essentially), and web sends back one video track (webcam). Both sides also send an audio track and data track. All calls are initiated from iOS, meaning that when web connects, the tracks have already been created and attached to the room by the remote participant.

I just upgraded the JS SDK to 2.0.0 after receiving the email about the breakage coming with an upcoming Chrome update. That’s caused a problem: in roughly a third of calls, one of the video feeds from iOS doesn’t come through to the web.

From the web’s perspective, when the remote participant first connects, all the publications exist as they should in the participant.tracks list. They all have track=null, but that’s normally fine, because those tracks become available in the trackSubscribed event soon after. But sometimes, one of the video tracks never fires a trackSubscribed event, and so we never get handle to the track object, so that video stays blank on screen.

This problem first starts when we go from version 2.0.0-beta14 to beta15. I saw that’s when you switch over to Unified Plan, so I figure it’s related to that.

Once a call begins and shows this problem, I can try refreshing the web page, to no avail. The video that was blank will stay blank when the page reloads, even though all the other tracks work fine. The only fix at this point is to end the call and start a new one from iOS. (With one exception: I can downgrade my web Twilio version to <2.0.0-beta15, and then it can connect to the same call that was having problems before.)

That last bit suggests that two things are going on:

Here’s a room SID where we had the problem: RM183492e162e5d6ab91957fb76c80cb7f

As usual that call started on iOS, and then web connected and one of the video feeds never fired trackSubscribed. I refreshed a few times seeing the same problem, then ended the call.

And here's a room SID of a call where the problem did NOT occur: RM2abc3fc68211b6e77ee0248b080f8faa

Code of how we construct the room, starting after we've created the room object:

if (room.participants.size > 0) {
  this.setupParticipant(room.participants.values().next().value);
}
room.on('participantConnected', setupParticipant);

function setupParticipant(participant: RemoteParticipant) {
    participant.tracks.forEach(publication => {
      if (publication.track) {
        this.setupRemoteTrack(publication.track); // this attaches the tracks to DOM etc
      }
    });

    participant.on('trackSubscribed', track => {
      // This normally fires for all remote tracks, but sometimes a video track is missing
      this.setupRemoteTrack(track);
    });
  };
}

Thanks for your help.

gritbranch commented 4 years ago

I am having similar issue. I use "twilio-video": "2.0.0" but with Chrome (version 79) and MacOS Safari (version 13.0.4).

I am testing the code from https://www.twilio.com/blog/video-chat-app-asp-net-core-3-angular-8-twilio, please see comment below when running in Safari and Chrome.

private registerParticipantEvents(participant: RemoteParticipant) {
    if (participant) {
        participant.tracks.forEach(publication => {
            console.log(publication);
            // For Safari
            // RemoteVideoTrackPublication.track is null
            // RemoteAudioTrackPublication.track is null
            //
            // For Chrome
            // track: RemoteAudioTrack
            // isStarted: (...)
            // isEnabled: (...)
            // isSwitchedOff: (...)
            // _isStarted: (...)
            // kind: "audio"
            // ...
            // track: RemoteVideoTrack
            // isStarted: (...)
            // isEnabled: (...)
            // isSwitchedOff: (...)
            // _isStarted: (...)
            // kind: "video"
            // ...
            this.subscribe(publication);
        });
        participant.on('trackPublished', publication => this.subscribe(publication));
        participant.on('trackUnpublished',
            publication => {
                if (publication && publication.track) {
                    this.detachRemoteTrack(publication.track);
                }
            });
    }
}

I have tried twilio-video 2.0.0 and 2.0.0-beta14 and I basically get the same result.

Appreciate any help regarding this.

Thank you!

patricktemple commented 4 years ago

Just debugged this again, and it turns out what I said about refreshing isn't entirely true. Sometimes refreshing the page on web does fix the issue, and sometimes when the session starts fine, refreshing it causes the issue to arise when the page reloads. Not sure why it didn't appear that way before... the only thing that's changed is that we just upgraded our iOS SDK to 3.0.1. Still seeing the issue overall. I tried and reproduced it on 3.1.0 as well.

makarandp0 commented 4 years ago

@patricktemple and @ryansalvador, Thank you for reporting and debugging this issue. I will look into this today, and let you know our findings. It does appear to be issue between unified/non-unified plan interop.

gritbranch commented 4 years ago

Thanks for looking into this. We are able to work around the issue by using a promise based approach to make sure that the array of local tracks created is not null before we connect to the room. In our case, This is what appears to be causing the other participant not receiving the event fired by Safari.

Our setup now looks similar to this:

private localTracksPromise: Promise<LocalTrack[]>;

this.localTracksPromise = createLocalTracks({audio: true, video: true});

this.localTracksPromise.then(localTracks => {
    // For Safari
    // This ensures that localTracks will have an array of LocalTracks at this point
    const connectOptions = {
      'my-room',
      localTracks
    };

    connect('my-token', connectOptions as ConnectOptions).then(connectedRoom => {
        ...
    }
})

// Other participant can now register and listen to these tracks...
private registerParticipantEvents(participant: RemoteParticipant) {
    if (participant) {
        participant.tracks.forEach(publication => {
            // For Safari / Chrome / Firefox
            // track: RemoteAudioTrack
            // isStarted: (...)
            // isEnabled: (...)
            // isSwitchedOff: (...)
            // _isStarted: (...)
            // kind: "audio"
            // ...
            // track: RemoteVideoTrack
            // isStarted: (...)
            // isEnabled: (...)
            // isSwitchedOff: (...)
            // _isStarted: (...)
            // kind: "video"
            // ...
        });
    }
}

This approach also handles device permission very well as it waits for the localTracks promise to be resolved before connect().

Hope this helps anyone who encounters the same issue.

makarandp0 commented 4 years ago

Thanks for the note @ryansalvador. Looks like the issue you had was different from what @patricktemple had. Glad to know that you are unblocked.

@patricktemple - We are actively looking at the planB->unified issue that you are seeing. So far have identified a case where multiple video tracks published from planB client does not get translated correctly to unified plan, causing it to not subscribe one of them. We are looking for a fix, and will update soon.

patricktemple commented 4 years ago

Great, thank you!

anna-vasilko commented 4 years ago

Hi @patricktemple The server-side fix is rolled out as of yesterday night. Please let us know if it fixes it for you!

patricktemple commented 4 years ago

So far so good! I upgraded again and things have worked well. I'll let you know if anything changes.

By the way we've only supported Chrome on our project so far, and when we tried Safari and Firefox in the past we saw intermittently blank videos like what I described in this bug. Unlike in Chrome, this issue predated the upgrade to 2.0.0. But now after your fix, the problem appears solved in those browsers too.

manjeshbhargav commented 4 years ago

@patricktemple ,

We're really glad that your issue seems to have been resolved. I will close this issue for now. Feel free to re-open it if you see the same behavior in the future.

Thanks,

Manjesh Malavalli JSDK Team