twilio / twilio-video.js

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

Participants cannot see each other's tracks #77

Closed katranci closed 7 years ago

katranci commented 7 years ago

Hello,

I'm using v1.0.0-beta5. When two participants are connected to the same room with default local tracks (video & audio), querying each other's tracks returns an empty Map. They can see each other fine (room.participants.size returns 1). They can see their own tracks fine as well (room.localParticipant.tracks.size is 2). What could be wrong?

Thank you

jskrzypek commented 7 years ago

I am also seeing this issue. What framework are you using for your app?

MilllerTime commented 7 years ago

Using v1.0.0-beta5, I'm experiencing this in Chrome 57 while Firefox works as expected.

I can confirm that room.participants does show a remote participant in all browsers. However in Chrome, the remote participant has no tracks (empty Map), while the local participant has two tracks, both audio and video. This is consistent with your observations.

In Firefox, all participants have expected tracks and everything renders just fine.

If a connection is established between Chrome and Firefox, FF shows the remote tracks from Chrome, but Chrome doesn't show anything received from FF. Chrome is sending audio/video tracks over the wire, it just doesn't populate remote participant instances with tracks received.

I'm experiencing this using both Windows and Mac OS X. Edit: Same deal with Chrome and FF on Android as well.

geoffreyabdallah commented 7 years ago

Are you adding handlers for 'trackAdded' and 'trackRemoved' on each participant as well as having handlers for 'participantConnected' on room? I found with the upgrade having the 'trackAdded' and 'trackRemoved' were now necessary due to the async/event based nature of the changes from beta4 to beta5.

If you can add a code snippet I'd be happy to help take a look.

MilllerTime commented 7 years ago

@geoffreyabdallah Once connected and a Room instance is obtained, I am registering for the "participantConnected" event on the Room. Whenever a new participant connects, I am registering both "trackAdded" and "trackRemoved" on the new Participant instance.

The problem is not just that the tracks aren't being displayed in the browser. Inspecting a live Participant instance in Chrome reveals that participant.tracks.size === 0. FF reports a size of 2, all other factors the same. Given these observations, I don't believe the issue is as simple as forgetting to add an event listener.

jskrzypek commented 7 years ago

@MillerTime i am seeing the same behavior

rfbrazier commented 7 years ago

@katranci is this issue reproducible 100% of the time, or only in some cases? Is behavior browser-dependent, or do Chrome and Firefox behave in the same way?

@MilllerTime same question--are you seeing this 100% of the time?

For either of you, if the issue is intermittent, can you provide an estimate of frequency? Also, can you provide DEBUG logs for both sender and receiver when the issue occurs?

We're looking into this on our side as well, but so far have not been able to reproduce in -beta5 or earlier releases. Thanks for any additional information that will help us track this down.

MilllerTime commented 7 years ago

@rfbrazier Yes. I've only been testing video connections this week, but my stated observations have been 100% consistent across all platforms I've used (Windows 10, OS X, and Android 7) for this entire week.

I'll have to follow up with logs tomorrow, but thanks for looking into this issue!

katranci commented 7 years ago

@rfbrazier It's reproducible 100% of the time for me as well. I'm using macOS 10.12.4 and Chrome 57.0.2987.133 (64-bit).

katranci commented 7 years ago

@rfbrazier I've forked TwilioDevEd/video-quickstart-node and applied the minimal changes required that will allow you to reproduce the issue:

https://github.com/TwilioDevEd/video-quickstart-node/compare/master...katranci:issue-77?expand=1

geoffreyabdallah commented 7 years ago

@katranci I'm still failing to see how this is unexpected behavior. As mentioned in my comment, when room.participants is iterated over after joining room, participants.tracks is currently empty, but since the object is reused, once the tracks are present, the size will be updated correctly within the same map. This is what I was trying to get at with the necessity of trackAdded to then let you know when said updates on the map happen (which is commented out in your example.) The other possibility would be to wrap what you want to do in a saga/promise, wait x number of ms until there is a track present, and then continue execution after yielding/resolving. I'm attaching screenshots below (tested in firefox and chrome) so you can see the delayed evaluation within the same log statement console.log("Number of " + participant.identity + " tracks: " + participant.tracks.size, participant.tracks):

Firefox:

screen shot 2017-04-07 at 9 41 15 am screen shot 2017-04-07 at 9 41 22 am

Chrome (notice if you hover over the i it says: "value below was evaluated just now"):

screen shot 2017-04-07 at 9 44 14 am screen shot 2017-04-07 at 9 44 20 am screen shot 2017-04-07 at 9 44 25 am
jskrzypek commented 7 years ago

@geoffreyabdallah what version of Chrome are you running? I do listen on trackAdded and the event never fires in Chrome 58. In Firefox my code works and uses the trackAdded handler

geoffreyabdallah commented 7 years ago

@jskrzypek Chrome 57, I'll download 58 and report back using both my company's internal app as well as the starter app (if there are issues on the starter app).

geoffreyabdallah commented 7 years ago

@jskrzypek No issue on my end w/ 58 - any chance you'd be willing to share your code or adding a snippet and I may be able to take a deeper dive and help out?

jskrzypek commented 7 years ago

@geoffreyabdallah so I'm on Chrome 58.0.3029.54 beta and if I run the TwilioDevEd video-quickstart-node I get this error when I try to connect to a room, which I mentioned in #79:

[connect #1]: Error while connecting to a Room: TypeError: this._peerConnection.addStream is not a function
    at PeerConnectionV2.addMediaStream (twilio-video.js:4052)
    at PeerConnectionManager._getOrCreate (twilio-video.js:4309)
    at getConfigurationSucceeded (twilio-video.js:4338)
    at <anonymous>
jskrzypek commented 7 years ago

@geoffreyabdallah which OS are you on? could this perhaps also be a mac vs windows thing? I don't think that makes sense...

jskrzypek commented 7 years ago

Ok I've added a workaround for the error I mentioned in my branch here, and there's no tracks on the participants when they get connected: https://github.com/jskrzypek/video-quickstart-node/tree/chrome-workaroung

MilllerTime commented 7 years ago

@jskrzypek I see the same problem on Mac, Windows, and Android. I don't think it's platform specific. I'm also not seeing the TypeError you've mentioned, but perhaps that is specific to the TwilioDevEd video-quickstart-node app you're running?

@geoffreyabdallah I'm using Chrome 57 as well (64 bit, no extensions enabled) and this is what a remote Participant instance looks like to me:

participant

I'm logging the Participant when connected, and I'm aware it may not immediately have tracks available. However, the tracks.size property is computed on the fly, and I waited a good 10-15 seconds before accessing it. As you can see, it was still 0. Further, I'm logging anytime the "trackAdded" event fires on a Participant, and it never fires in Chrome for remote participants.

Doing the same thing in Firefox, accessing tracks.size via the console yields 2 and I also see the "trackAdded" event firing twice. So all of my code is working as expected - Chrome just isn't getting any tracks.

I assume you're running the same Twilio Video script as I am? https://media.twiliocdn.com/sdk/js/video/releases/1.0.0-beta5/twilio-video.min.js

@rfbrazier I looked around the Twilio console for debug logs, and did find a Debugger page. However, it shows no errors or warnings. I assume that means everything is working on Twilio's backend? If there are any other logs you'd like to see, let me know. As far as I can tell, this must just be some quirk when the JS SDK is used with Chrome. It's also a totally silent failure - I'm not getting any errors in Chrome either and I've made sure Promises aren't swallowing errors. I always append a .catch() and rethrow in a new call stack using setTimeout. Everything appears to work except for the part where a remote Participant receives tracks.

Edit: I just tested with the latest Chrome Canary build (59.0.3065.0) and I'm still experiencing the exact same issue.

markandrus commented 7 years ago

Hi @katranci,

I'm sorry to hear you are having this issue. @geoffreyabdallah, thanks a bunch for taking the time to step through the issue, too. In general, any async-updated collection including

may initially be empty, and an application should register callbacks in order to track updates to the collection. This is because Participants can come and go at any time, Tracks can be added and removed at any time, etc. These callbacks include listeners for the "participantConnected", "participantDisconnected", "trackAdded", and "trackRemoved" events.

That being said, assuming these callbacks are in place, there may still be an issue—either client- or server-side—preventing these updates from firing. @katranci, I'd be very interested to know: if you use the stock video-quickstart-node application without the event handlers commented out, does the application work correctly, or are you still seeing that Tracks are missing? If it works, maybe you could share your application's code? If it does not work, could you please share the logs from both Participants in the Room, ensuring that connect is called with logLevel set to "debug", e.g.

connect(token, { logLevel: 'debug' })

Thanks! Mark

markandrus commented 7 years ago

Hi @MilllerTime,

Thanks for the additional information you have provided.

I'm logging anytime the "trackAdded" event fires on a Participant, and it never fires in Chrome for remote participants.

Can you please share the code where you attach the event listeners? I am wondering if the listeners are being attached after the events may have fired in Chrome. If the listeners are attached as soon as the Promise returned by connect resolves, it ought to not be a problem; however, if they are attached any later I fear an initial round of events may be missed. Although that would not explain why you see the computed size property set to 0...

Could you please share full logs from each Participant in the Room, ensuring that connect is called with logLevel set to "debug", e.g.

connect(token, { logLevel: 'debug' })

Thanks! Mark

MilllerTime commented 7 years ago

Hey @markandrus, I didn't realize the connect method accepted a logLevel option. That's good to know, thanks! Attached are logs from a participant using Chrome and a participant using Firefox, both connecting to the same room.

logs.zip

As for the code, I am indeed attaching the track listeners on the next tick after a remote participant joins, whether they were already in the room upon connection or joined later. It's not happening as soon as the Promise returned from connect resolves. However, I'm making up for any missed track events by first iterating the tracks map on the Participant, then attaching the listeners to monitor future changes. Given a variable participant that points to a remote Participant instance from room.participants, I am executing the following code. (FWIW, I'm doing this in componentDidMount of a custom React component, but that shouldn't matter)

// add any tracks that might already exist
participant.tracks.forEach(this.addTrack);
// monitor future track changes
participant.on('trackAdded', this.addTrack);
participant.on('trackRemoved', this.removeTrack);

The addTrack and removeTrack methods are defined as:

constructor() {
  this.addTrack = this.addTrack.bind(this);
}

addTrack(track) {
  this.mediaEl.appendChild(track.attach());
}

removeTrack(track) {
  track.stop();
  track.detach().forEach(el => el.remove());
}
markandrus commented 7 years ago

@MilllerTime these logs are very useful, thank you. Looking at Firefox's logs, we can see that the expected AudioTrack and a VideoTrack are raised for the Chrome Participant:

2017-04-08 05:56:44.149Z | INFO in [Participant #2: PA5d7d4bae0d669535720af504203a86b1]: Created a new Participant: 38636C30-223D-427E-B68D-372C2DCC1CDB
…
2017-04-08 05:56:44.251Z | DEBUG in [AudioTrack #3: 3b482127-28d8-4272-8794-ace0ac09e594]: Initializing
2017-04-08 05:56:44.254Z | INFO in [Participant #2: PA5d7d4bae0d669535720af504203a86b1]: Added a new AudioTrack: 3b482127-28d8-4272-8794-ace0ac09e594
…
2017-04-08 05:56:44.262Z | DEBUG in [VideoTrack #4: b15686d8-1a16-4762-95eb-b2db776ed3d6]: Initializing
2017-04-08 05:56:44.264Z | INFO in [Participant #2: PA5d7d4bae0d669535720af504203a86b1]: Added a new VideoTrack: b15686d8-1a16-4762-95eb-b2db776ed3d6

However, if we look at Chrome's logs, we never see such events:

twilio-video.min.js:92 2017-04-08 05:57:48.416Z | INFO in [Participant #2: PA28caa01899ea8b5d3eb8dcf88dba93a4]: Created a new Participant: B9453AE3-52C8-4840-A5E7-4EC95105F612
…

In Chrome, we should be receiving two "trackAdded" events for a remote AudioTrack with MediaStreamTrack ID {69cc74ff-b796-4cf8-a014-f66ee11132ac} and a remote VideoTrack with MediaStreamTrack ID {269c253a-efd0-40eb-87e2-9ef52df6639b}. I was beginning to think an SDP failure may have occurred, so I wrote a JSFiddle that applies the same initial SDP answer to an RTCPeerConnection; however, the SDP was fine and both MediaStreamTracks were present.

There are basically two events that must occur for a new AudioTrack or VideoTrack to be constructed and raised in a "trackAdded" event:

So I'm still not sure what is wrong. Can you confirm whether or not the video-quickstart-node project works for you? If not, perhaps there is something unique to your code. Could you perhaps share more of it or a smaller repro example? (feel free to email me, too).

As for the code, I am indeed attaching the track listeners on the next tick after a remote participant joins, whether they were already in the room upon connection or joined later. It's not happening as soon as the Promise returned from connect resolves. However, I'm making up for any missed track events by first iterating the tracks map on the Participant, then attaching the listeners to monitor future changes.

Great, this should be fine 👍

Thanks, Mark

MilllerTime commented 7 years ago

@markandrus The log lines you've copied certainly correspond to the behavior I'm seeing.

I setup the video-quickstart-node project locally and that is working for me - the "trackAdded" events come through and I can see remote video. I even replaced the local Node generated accessToken with an accessToken generated via our in-house API and everything was still great. Since that works, I looked closely at the quickstart source code and there is really nothing special about what I am doing in comparison. I refactored my event handling to mirror that of the quickstart just as a test, and still couldn't get video to render in Chrome on my project (or have track events fire).

As a further test, I wanted to see if Chrome might raise track events if I dynamically remove/added tracks on a remote participant. The results didn't directly solve the problem, but were interesting nonetheless:

I established a connection between Chrome and Firefox as I've been doing, and the result was exactly what I've previously documented here. I also stored some global variables to tinker with:

On the Firefox client, I obtained a reference to the local video track:

var videoTrack = localParticipant.videoTracks.values().next().value;

I then removed the track:

localParticipant.removeTrack(videoTrack);

This had no effect on Chrome. So, I added the track back:

localParticipant.addTrack(videoTrack);

This raised three events on Chrome!

Participant "Firefox" added a video track.
Participant "Firefox" removed a video track.
Participant "Firefox" added a video track.

Is it possible that the first "trackAdded" was the original track, and somehow it got stuck and was "flushed" when I added another?

Debugging the participant instance from Chrome, I could indeed see there was now one video track. However, I couldn't actually see any video (I'm sure this was because the track was stopped when I removed it originally). Still, if I enabled/disabled the stopped track from Firefox, Chrome would fire the associated "trackEnabled" and "trackDisabled" events. I felt like I was halfway there, and decided to try adding a track that wasn't stopped. First, I removed the existing track:

localParticipant.removeTrack(videoTrack);

This time, a "trackRemoved" event fired in Chrome immediately. Then I added a new track:

Video.createLocalVideoTrack().then(track => {
  localParticipant.addTrack(track);
});

This raised a "trackAdded" event locally in Firefox, and I could see live local video once again. However, Chrome got nothing. No "trackAdded" event and no video.

So I once again queried the local video, and removed / added it back:

// Same thing as before
videoTrack = localParticipant.videoTracks.values().next().value;
localParticipant.removeTrack(videoTrack);
localParticipant.addTrack(videoTrack);

After running the last addTrack statement, Chrome once again logged three track events: "trackAdded", "trackRemoved", "trackAdded".

That seems to further confirm that the tracks/events are getting stuck somehow. Does this shed any light on the issue?

At this point, my code for logging track events is as simple as this. I can't imagine this being the source of the problem, but the fact that the quickstart works is perplexing!

// Assume accessToken and roomName were already obtained.
Video.connect(accessToken, { name: roomName }).then(room => {
  room.on('trackAdded', (track, participant) => {
    this.log(`Participant "${participant.identity}" added a ${track.kind} track.`);
  });
  room.on('trackRemoved', (track, participant) => {
    this.log(`Participant "${participant.identity}" removed a ${track.kind} track.`);
  });
});
MilllerTime commented 7 years ago

I can confirm the issue is specific to my project, but not because of the way I'm using Twilio Video.

I started with a clean slate on CodePen, and added Twilio Video and the ability to connect to a room. That worked fine in Chrome and FF. I then introduced React, and recreated the use case from my project using Twilio and a special React component. Everything still worked in Chrome - I could see tracks on remote participant objects.

Since that worked, I copy/pasted the code I wrote on CodePen into a new screen in my local project, and Chrome would no longer receive any tracks, just empty participant objects.

Given that 100% identical code works on CodePen but not my project definitely points to a problem with my project. However like I previously stated, Chrome shows no errors so there is not an obvious conflict. Also the fact everything works seamlessly in Firefox makes things even more confusing.

@markandrus Have you ever run into any conflicts with other libraries before? I'm using Webpack to bundle my code, so nothing should be leaking into global space to interfere with Twilio. Still, something is happening...

@jskrzypek What framework are you using, where you are experiencing the same results as I am?

MilllerTime commented 7 years ago

@markandrus Got it. Turns out the WebRTC's "adapter.js" doesn't play well with Twilio.

I'm using this file: https://webrtc.github.io/adapter/adapter-latest.js from this repo: https://github.com/webrtc/adapter.

I added it to my project half a year ago and forgot about it, but I use the browser's native WebRTC APIs for some features. I rely on the adapter to "insulate myself from spec changes and prefix differences" as the repo claims, and I am using the latest release. It is maintained by the WebRTC project, and my understanding was that anything compliant with the WebRTC spec would just work with it. However that doesn't seem to be the case, and I apologize it took this long to discover.

@katranci @jskrzypek Is there any chance either of you are using the WebRTC adapter? If so, does removing it solve this issue? If not, maybe this should be a separate issue.

I'm not entirely sure what adapter.js is doing, but it is definitely the culprit.

jskrzypek commented 7 years ago

@MilllerTime, I'm using angular (we're on ^4.x). While trying to fix my issue #79 I know I added adapter.js, and I might not have removed it after I solved it.

Similar to your case, after solving my issue with the extension, the quickstart works but I've still been having trouble with the participant track events in my app in chrome.

If removing adapter.js does solve the issue it looks like the real issue is that the RTCPeerConnection adapter/polyfill that twilio-video.js is using doesn't have a good way itself to detect when the RTCPeerConnection constructor has already been wrapped by something. That was exactly what was happening with the extension I was using that caused my other issue.

jskrzypek commented 7 years ago

@MilllerTime yup the remaining issue on my side was adapter.js (webrtc-adapter from npm). As soon as I removed that the whole thing works now.

markandrus commented 7 years ago

@MilllerTime

Have you ever run into any conflicts with other libraries before?

Ah, actually, yes. We have seen issues with Angular's Zone.js, which monkey-patches various asynchronous interfaces (now fixed), and we've also seen issues with adapter.js, which monkey-patches various WebRTC APIs. It sounds like you hit the same issue here: https://github.com/twilio/twilio-video.js/issues/25

twilio-video.js needs to know certain things about the WebRTC APIs it is operating with in order to workaround known issues in Chrome and Firefox. I believe adapter.js may be masking—or at least changing—these interfaces. We will have to schedule some time in the future to assess whether or not we can work alongside adapter.js, but for the time being it is unsupported, but—in the short term—we can do a better job identifying common issues in this repo.

katranci commented 7 years ago

Same here. I was using getscreenmedia npm module which depends on webrtc-adapter package. Removing that resolved the issue.

MilllerTime commented 7 years ago

@katranci Awesome! Good to hear. Since it seems everyone here has resolved things, perhaps this issue can be closed?

@jskrzypek Glad you fully resolved your issue as well! 🎉I saw on your other issue that you spent a good week figuring everything out. It was a frustrating week for me as well - it's great this is behind us.

@markandrus Thanks for the info, this makes sense. Issue 25 is exactly what I was experiencing. It just didn't hit my radar, because I didn't know to search for anything related to adapter.js until I solved the problem. Thankfully, it seems the WebRTC APIs have stabilized enough that I no longer need adapter.js for my non-twilio WebRTC features. However, I 100% agree these compat issues should be documented for other's ease of discovery. A note in the README would be great, as https://media.twiliocdn.com/sdk/js/video/releases/1.0.0-beta5/docs/ is one of the first things I read when I was getting started. My first exposure though (and many other's I'm sure) was the getting started guide and the SDK download section. Maybe those are candidates for a compat notice as well?

Thanks for everyone's time working to figure this one out. I also appreciate that Twilio is willing to assess the possibility of working with adapter.js in the meantime!

markandrus commented 7 years ago

@MilllerTime thanks for the feedback. We've added a COMMON_ISSUES.md, linked to it from the README.md, and called out to it from our new ISSUE_TEMPLATE.md. I hope this helps future users when they encounter issues or file bugs. I'll close this issue now in favor of tracking on #25

markandrus commented 7 years ago

This is now fixed in twilio-video@1.1.0 as well as recent versions of adapter.js.