twilio / twilio-video.js

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

Critical issue: occasionally audio/video not showing on the other participant #125

Closed theDude30 closed 7 years ago

theDude30 commented 7 years ago

Code to reproduce the issue:

The participants connected to twilio room without audio or video track. PC starts audio and video. Mobile user also starts audio and video.

Expected behavior:

The participants should have see and hear each other.

Actual behavior:

The pc and Mobile could see the preview but could not see and hear the participant(as if they were on different rooms).

Please Note: This issue happens every few times and it happens only with specific client, we were not being able to reproduce it in our office environment . I'm attaching the log from the PC only, i did not have the ability to take the logs from the phone. pc_chrome.txt

TODO

Software versions:

claudioc commented 7 years ago

We are facing the same problem. Apparently – sometimes – the trackAdded event is not fired and one of the parties only see the localTracks and not the remote one (happens almost always on Chrome mobile, rarely on Chrome desktop).

While investigating the issue, we also see that with Firefox the remoteVideo is almost never showed. We always see the TypeError: 'get peerIdentity' called on an object that does not implement interface RTCPeerConnection. (but this is might not be the TwilioVideo library, but just it catching some error from somewhere else).

rfbrazier commented 7 years ago

@theDude30 In order to address this issue, we will need the following:

  1. The Chrome browser version used on the mobile device.
  2. JavaScript console logs from the Chrome mobile device, with twilio-video.js log level set to debug.

I understand it may be difficult to get item 2. Perhaps you can use Android Remote Debugging?

In the meantime, we are going to try to reproduce this issue ourselves. Given that you are not able to reproduce it in your environment, however, getting logs from the impacted device(s) may be the best path forward.

theDude30 commented 7 years ago

i will have access to the problematic device on Tuesday. Do you have any updates? did you manage to reproduce the issue?

theDude30 commented 7 years ago

UPDATE: we could not get the logs from the phone. However, when we tested with the phone and PC outside the company's network and it worked flawlessly. Is there any network requirements that i should ask for? and is there any network test page that i can perform the tests to see that it pass or fail(and maybe provide information)?

kroft1 commented 7 years ago

We have used the following link for network tests of twilio: http://networktest.twilio.com.

theDude30 commented 7 years ago

can i develop this test page? is there a github/sources that i can use?

manjeshbhargav commented 7 years ago

@theDude30 ,

The app mentioned by @kroft1 is for testing our Voice SDK. Our QE team is trying to reproduce the issue you've experienced. We will keep you in the loop on any developments in this regard.

anna-vasilko commented 7 years ago

@theDude30 Thanks for narrowing down the issue. Indeed, it does sound like it is specific to your office network. Shortly we will follow up with possible solutions. Meantime I wanted to update you about performed testing on our end. Your scenario was tested:

anna-vasilko commented 7 years ago

@claudioc Thank you for "leading the Web"! ..based on your profile :-) Sorry to hear that you are not receiving track events.

  1. With Chrome mobile and Chrome desktop we were not able to reproduce. Suspecting the issue is related to using some restricted network. Please see my reply above. What network did you use when seeing this problem? Could you share JS console logs from the Chrome mobile with twilio-video.js log level set to debug?
  2. In regards to Firefox, it is a known issue specific to Firefox beta, described here: https://github.com/twilio/twilio-video.js/blob/master/CHANGELOG.md
claudioc commented 7 years ago

@anna-vasilko LOL, but unfortunately I am only leading the web development here :D

Today I must investigate this issue deeper and I will for sure get back to you with more details. Thanks

claudioc commented 7 years ago

@anna-vasilko @rfbrazier I managed to get the debug logs of both cases: when the remote video is showing, when the remote video is not showing. It took me 11 tries to find a failing one, reloading the page with cmd+R between the tries.

In the logs you'll see a "Room joined" message (from my code) that is fired in both cases (so the event listeners are attached). My gut feeling is the trackAdded event is missing (not fired of fired too early).

The application is a React app. The browser is Chrome 59. OS is macOS 10.12.4 Laptop: 13" MacBookPro (very low performances, might be part of the problem?). I had similar issues on a Chrome mobile (as stated earlier)

The local and remote peers are running on the same development machine (but in different browsers, just to keep them a bit "sandboxed").

remote-video-not-showing-logs.zip

This is what I do as soon as the room is joined (room type Group):

const roomJoined = (room: any) => {
  console.log('Room joined')
  activeRoom = room

  // Attach the Tracks of the Room's Participants.
  room.participants.forEach((participant: any) => {
    attachRemoteTracks(participant.tracks)
  })

  // When a Participant joins the Room, log the event.
  room.on('participantConnected', (participant: any) => {
    console.log("Joining: '" + participant.identity + "'")
  })

  room.on('trackAdded', function (track: any, participant: any) {
    console.log(participant.identity + ' added track: ' + track.kind)
    attachTracks([track], remoteTracksId)
  })

  room.on('trackRemoved', function (track: any, participant: any) {
    console.log(participant.identity + ' removed track: ' + track.kind)
    detachTracks([track])
  })

  // When a Participant leaves the Room, detach its Tracks.
  room.on('participantDisconnected', function (participant: any) {
    console.log("Participant '" + participant.identity + "' left the room")
    detachRemoteTracks(participant.tracks)
  })

  // Once the LocalParticipant leaves the room, detach the Tracks
  // of all Participants, including that of the LocalParticipant.
  room.on('disconnected', function () {
    detachLocalTracks()
    room.participants.forEach((participant: any) => {
      detachRemoteTracks(participant.tracks)
    })
    activeRoom = null
  })
}
claudioc commented 7 years ago

Some additional information:

I am now investigating if there is some problem using React in this scenario. I also tried waiting 10 seconds before trying to join the room, but still... no tracks :/

claudioc commented 7 years ago

I might have fixed the problem.

I was using the tracks key in the options object I am passing the Twilio.Video.connect() method (on both clients). I thought that Twilio would use those tracks (that I got at the createLocalTracks() moment) to identify which tracks to use as local.

I removed that option and now (apparently) the problem is gone.

Does it make sense to you?

rfbrazier commented 7 years ago

@claudioc I'm not sure I completely understand: Are you saying you were not previously passing the Track objects you created with createLocalTracks() into connect(), and now you are? If so, then that makes sense.

Maybe provide a quick example of your code before and after the change? That way we can confirm that what you're doing is working as expected, and your code sample can help others watching this issue avoid the same mistake in the future (and we can improve our docs to help others avoid it as well!)

Thanks so much for working with us on this, and glad to hear things are finally working well for you!

anna-vasilko commented 7 years ago

@claudioc In case when you do not pass tracks to connect, the sdk does automatically acquire an array of LocalAudioTrack and LocalVideoTrack before connecting to the Room. That explains why it works for you now after removing tracks option. Though passing array of tracks explicitly should also work. Here is an example:

Video.createLocalTracks({ audio: true }).then(function(localTracks) {
  return Video.connect('my-token', {
    name: 'my-room-name',
    tracks: localTracks
  });
});

An example of how you were acquiring tracks and passing them into connect would be very useful. You can find more examples here: https://media.twiliocdn.com/sdk/js/video/releases/1.0.0/docs/global.html#connect

claudioc commented 7 years ago

@anna-vasilko @rfbrazier , so apparently if you explicitly pass the tracks options, 50% of the time the remote video doesn't show. In my case, it's always one of the two clients that doesn't show it.

So, to recap my case (might be useful do debug or update the docs):

Now if, at the moment of the connection, I pass the tracks option, client A doesn't show the remote track and this is because... it doesn't have it. I tried waiting for 10 seconds and then inspect the room object and I see one participant with 0 tracks. In the meanwhile, client B is showing both the local and the remote one.

Final note: I am using ONE machine to run both A and B (and the middleware), so maybe this is a bit "extreme", but I have the same problem also using a mobile phone as client A.

:)

claudioc commented 7 years ago

I can share my "media-service" script without problem (this is the React client part, with the "tracks" option removed):

import { notifyParent } from '../Utils'
import { AppState } from '../AppState'
import { IAppWindow } from '../Types'

let localTracks: any
let activeRoom: any

const localTracksId = 'local-tracks'
const remoteTracksId = 'remote-tracks'

export class MediaService {
  public static showLocalMedia() {
    (window as IAppWindow).Twilio.Video.createLocalTracks()
      .then((tracks: any) => {
        localTracks = tracks
        attachLocalTracks()
      }, (error: any) => {
        notifyParent('error', JSON.stringify(error))
        notifyParent('notice', 'Unable to access local media')
      })
  }

  public static startInteraction() {
    const connectOptions = {
      name: AppState.getRoomName()
      //, logLevel: 'debug'
    }; // Semicolon needed

    (window as IAppWindow).addEventListener('beforeunload', leaveRoomIfJoined); // Semicolon needed

    (window as IAppWindow).Twilio.Video.connect(AppState.getRoomToken(), connectOptions)
      .then(roomJoined, (error: any) => {
        // @TODO better handling here
        console.log(error)
        alert('An error occurred while connecting to the Room')
        this.shutDown()
        notifyParent('error', 'Could not connect to partner: ' + error.message)
      })
  }

  public static shutDown() {
    leaveRoomIfJoined()
    detachLocalTracks()
  }
}

const roomJoined = (room: any) => {
  activeRoom = room

  // Attach the Tracks of the Room's Participants.
  room.participants.forEach((participant: any) => {
    console.log("Already in Room: '" + participant.identity + "'")
    attachRemoteTracks(participant.tracks)
  })

  // When a Participant joins the Room, log the event.
  room.on('participantConnected', (participant: any) => {
    console.log("Joining: '" + participant.identity + "'")
  })

  room.on('trackAdded', function (track: any, participant: any) {
    console.log(participant.identity + ' added track: ' + track.kind)
    attachTracks([track], remoteTracksId)
  })

  room.on('trackRemoved', function (track: any, participant: any) {
    console.log(participant.identity + ' removed track: ' + track.kind)
    detachTracks([track])
  })

  // When a Participant leaves the Room, detach its Tracks.
  room.on('participantDisconnected', function (participant: any) {
    console.log("Participant '" + participant.identity + "' left the room")
    detachRemoteTracks(participant.tracks)
  })

  // Once the LocalParticipant leaves the room, detach the Tracks
  // of all Participants, including that of the LocalParticipant.
  room.on('disconnected', function () {
    detachLocalTracks()
    room.participants.forEach((participant: any) => {
      detachRemoteTracks(participant.tracks)
    })
    activeRoom = null
  })
}

const leaveRoomIfJoined = (): void => {
  (window as Window).removeEventListener('beforeunload', leaveRoomIfJoined)
  if (activeRoom) {
    activeRoom.disconnect()
  }
}

// Attaches local tracks to the <Video> container
// This is not obviously the way to interact with another component
const attachLocalTracks = (): void => {
  attachTracks(localTracks, localTracksId)
}

const attachRemoteTracks = (tracks: any): void => {
  const remoteTracks = Array.from(tracks.values())
  attachTracks(remoteTracks, remoteTracksId)
}

const attachTracks = (tracks: Array<any>, containerId: string) => {
  const container = document.getElementById(containerId)
  if (container && !container.querySelector('video')) {
    tracks.forEach((track: any) => {
      container.appendChild(track.attach())
    })
  }
}

const detachLocalTracks = (): void => {
  if (!localTracks) {
    return
  }
  detachTracks(localTracks)
}

const detachRemoteTracks = (tracks: any): void => {
  // This apparently has some bad side-effects (freezes on chrome android)
  //const remoteTracks = Array.from(tracks.values())
  //detachTracks(remoteTracks)
}

const detachTracks = (tracks: Array<any>): void => {
  tracks.forEach((track: any) => {
    track.mediaStreamTrack.stop()
    track.detach().forEach((detachedElement: any) => {
      detachedElement.remove()
    })
  })
}
markandrus commented 7 years ago

@claudioc thanks for sharing your code. I believe you are seeing these errors because, although you set event listeners for "trackAdded", you do not check the tracks on the Participant once "participantConnected" fires. You must handle both

If you don't do this, you may miss Tracks in your application. I've created a small JSFiddle demonstrating this behavior: it has two Participants connect to the same Room, and then deliberately ignores "trackAdded" events, checking only tracks. The test runs in a loop: sometimes the Tracks will be emitted later via "trackAdded", but sometimes they are present "synchronously" in the tracks Map. See here, substituting your own Access Tokens (I tested against Group Rooms).

Can you try updating your code here:

  // When a Participant joins the Room, log the event.
  room.on('participantConnected', (participant: any) => {
    console.log("Joining: '" + participant.identity + "'")
    // NOTE(mroberts): Add the following?
    attachRemoteTracks(participant.tracks)
  })
markandrus commented 7 years ago

@claudioc reading up on @theDude30's issue here, and checking the logs he's posted, I believe his issue is distinct from yours. AFAICT, @theDude30's logs indicate that remote AudioTracks and VideoTracks are being received (and that "trackAdded" events are being fired), but there may be a network issue preventing audio and video from being received. Since your issue relates specifically to whether or not the "trackAdded" events are being fired, perhaps you could try my suggestion above and we could followup with your issue on a separate GitHub issue?

Thanks, Mark

claudioc commented 7 years ago

@markandrus OK I guess it makes sense to try this way (and I'll re-add the tracks option to the connect()).

"Unfortunately" I am in vacation now and I cannot try this on my home laptop, so I guess we'll have to wait until I will be back (1 week). Sorry about that :/

Thank you very much for the effort!

rfbrazier commented 7 years ago

@theDude30 per our troubleshooting and resolution of the trackAdded issue earlier this week, we are going to close this issue.

For any watching this issue, the root cause was a restrictive firewall in the end user's environment.