republishing Track fail using Twillio (Was Error: removeTrack() must be called with a RTCRtpSender instance as argument) #569

henritoivar · 2020-09-14T18:33:04Z

I have used Google with the error message or bug in association with the library and Cordova words to make sure the issue I'm reporting is only related to iOSRTC.
I have provided steps to reproduce (e.g. sample code or updated extra/renderer-and-libwebrtc-tests.js file).
I have provided third party library name and version, ios, Xcode and plugin version and adapter.js version if used.

Versions affected

cordova-plugin-iosrtc version 6.0.13
iOS 13.5.1
Twilio 2.6.0

Description

Trying to switch between cameras by unpublishing the previous track and then publishing a new track.

Unpublishing track returns error: removeTrack() must be called with a RTCRtpSender instance as argument

Steps to reproduce

I have adjusted the Twilio sample to implement switching between cameras:

<!DOCTYPE HTML>
<html>
<head>
    <title>
        Twilio Video Room
    </title>
    <meta name="viewport" content="viewport-fit=cover, width=device-width, initial-scale=1.0, minimum-scale=1.0, maximum-scale=1.0, user-scalable=no">
    <meta http-equiv="Content-Security-Policy"
          content="default-src 'self' data: gap: wss://* https://* 'unsafe-eval' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; media-src *">
    <script type="text/javascript" src="cordova.js"></script>
</head>
<body>
<br><br><br>
<div id="local-media" style="border: red 1px solid;"></div>
<div id="remote-media"></div>
<button onclick="toggleVideo()">Toggle video</button>
<button onclick="leaveMeeting()">Leave meeting</button>
<button onclick="startMeeting()">Start meeting</button>
<button onclick="switchCamera()">Switch camera</button>
<script type="text/javascript">

const token = '';
const roomName = 'test';
const scriptUrls = [];
let videoTrack = null;
let audioTrack = null;
let room = null;
let facing = true;

function loadScript(scriptUrl) {

  if (scriptUrls[scriptUrl]) {
    return Promise.resolve(scriptUrl);
  }

  return new Promise(function (resolve, reject) {
    // load adapter.js
    var script = document.createElement("script");
    script.type = "text/javascript";
    script.src = scriptUrl;
    script.async = false;
    document.getElementsByTagName("head")[0].appendChild(script);
    script.onload = function () {
      scriptUrls[scriptUrl] = true;
      console.debug('loadScript.loaded', script.src);
      resolve(scriptUrl);
    };
  });
}

async function startMeeting() {

  videoTrack = await getVideoTrack();
  audioTrack = await Twilio.Video.createLocalAudioTrack();
  const localMediaContainer = document.getElementById('local-media');
  localMediaContainer.appendChild(videoTrack.attach());
  localMediaContainer.appendChild(audioTrack.attach());
  console.log(videoTrack, audioTrack);

  Twilio.Video.connect(token, {
    tracks: [videoTrack, audioTrack],
    name: roomName,
    sdpSemantics: 'plan-b',
    bundlePolicy: 'max-compat'
  }).then(_room => {
    room = _room;
    console.log(`Successfully joined a Room: ${room}`);

    // Attach the Tracks of the Room's Participants.
    room.participants.forEach(function (participant) {
      console.log("Already in Room: '" + participant.identity + "'");
      participantConnected(participant);
    });

    room.on('participantConnected', participant => {
      console.log(`A remote Participant connected: ${participant}`);
      participantConnected(participant);
    });

    room.on('participantDisconnected', participant => {
      console.log(`A remote Participant connected: ${participant}`);
      participantDisconnected(participant);
    });

  }, error => {
    console.error(`Unable to connect to Room: ${error.message}`);
  });

  function participantConnected(participant) {
    console.log('Participant "%s" connected', participant.identity);
    const div = document.createElement('div');
    div.id = participant.sid;
    participant.tracks.forEach((publication) => {
      console.log('subbing to existing publication', publication);
      trackSubscribed(div, publication);
    });

    participant.on('trackPublished', (publication) => {
      trackSubscribed(div, publication)
    });
    participant.on('trackUnpublished', trackUnsubscribed);

    document.getElementById('remote-media').appendChild(div);
  }

  function participantDisconnected(participant) {
    console.log('Participant "%s" disconnected', participant.identity);

    var div = document.getElementById(participant.sid);
    if (div) {
      div.remove();
    }
  }

  function trackSubscribed(div, publication) {
    console.log('sub publication', publication);
    if(publication.track){
      attachTrack(div, publication.track);
    }
    publication.on('subscribed', track => attachTrack(div, track));
    publication.on('unsubscribed', track => detachTrack(track));
  }

  function attachTrack(div, track){
    console.log('attachTrack', track);
    div.appendChild(track.attach());
  }



  function trackUnsubscribed(publication) {
    console.log('unsub publication', publication);
    if(publication.track){
      detachTrack(publication.track);
    }
  }
}

function detachTrack(track) {
  console.log('detachTrack', track);
  track.detach().forEach(element => element.remove());
}

function toggleVideo() {
  console.log(videoTrack, room);
  if (videoTrack.isEnabled) {
    videoTrack.disable();
    // room.localParticipant.unpublishTrack(videoTrack);
    console.log('disable');
  } else {
    videoTrack.enable();
    // room.localParticipant.publishTrack(videoTrack);
    console.log('enable');
  }
}

async function switchCamera(){
  facing = !facing;
  // Get new track
  const newTrack = await getVideoTrack();

  // Stop old track
  videoTrack.stop();
  detachTrack(videoTrack);

  // Unpublish previous track
  room.localParticipant.unpublishTrack(videoTrack);
  
  // Publish new track
  room.localParticipant.publishTrack(newTrack);
}

function getVideoTrack(){
  return Twilio.Video.createLocalVideoTrack({facingMode: facing ? 'user' : {exact: 'environment'}});
}

function leaveMeeting() {
  room.disconnect();
  videoTrack.stop();
  audioTrack.stop();
  detachTrack(videoTrack);
  detachTrack(audioTrack);
}

async function ready() {
  // Note: This allow this sample to run on any Browser
  var cordova = window.cordova;
  if (cordova && cordova.plugins && cordova.plugins.iosrtc) {

    // Expose WebRTC and GetUserMedia SHIM as Globals (Optional)
    // Alternatively WebRTC API will be inside cordova.plugins.iosrtc namespace
    cordova.plugins.iosrtc.registerGlobals();

    // Enable iosrtc debug (Optional)
    cordova.plugins.iosrtc.debug.enable('*', true);
  }
  console.log('loading Twilio');

  await loadScript('https://media.twiliocdn.com/sdk/js/video/releases/2.6.0/twilio-video.js');
  console.log('loaded');
}

if(!window.cordova){
  ready();
}

document.addEventListener('deviceready', ready, false);
</script>
</body>
</html>

Expected results

Should switch to other camera.

Actual results

Error: removeTrack() must be called with a RTCRtpSender instance as argument

The text was updated successfully, but these errors were encountered:

hthetiot · 2020-09-15T09:09:41Z

@henritoivar thank you, will look into it.

hthetiot · 2020-09-15T09:16:49Z

@henritoivar, this is expected error if removeTrack is not getting an iosRTC RTCRtpSender instance.

It's probably an issue witht the way twilio-video.js manage sender here:

https://github.com/twilio/twilio-video.js/blob/9d9d7a63434f28cb4090861d96250e1ef7103a0a/lib/signaling/localparticipant.js#L59

Thank to your full example, I should be able to reproduce and handle the issue.

hthetiot · 2020-09-15T09:24:09Z

Note: updated example with missing <script type="text/javascript" src="cordova.js"></script>

hthetiot · 2020-09-15T14:44:19Z

I think this is related #571
@henritoivar You may test this PR (see instruction in description) - #572

henritoivar · 2020-09-16T09:52:44Z

@hthetiot Thanks a lot. This seems to be a part of the solution.

The issue i have now is that unpublishing works, but trying to publish again does not.

I have it set up so that i have the example running on the browser with one token and on iOS with another token.
When i unpublish my video track from iOS then it also dissappears on the browser - this is good.
When i republish my video track from iOS then it does not reappear on the browser - but it should.

When i have the example running both on browsers, then it works.

I'll try to dig a bit more to get more details on this.

henritoivar · 2020-09-16T11:07:17Z

The following issue with black remote video is also present: #541

hthetiot · 2020-09-16T16:16:53Z

The following issue with black remote video is also present: #541

arf, can you confirm @sboudouk ?

hthetiot · 2020-09-16T16:19:17Z

The issue i have now is that unpublishing works, but trying to publish again does not.

replaceTrack require renegotiate, RTCRtpSender is a SHAM for now not a shim see
#544

you may try #544 that now include #572

sboudouk · 2020-09-16T16:24:03Z

The following issue with black remote video is also present: #541

arf, can you confirm @sboudouk ?

I did not get into this issue, probably because I am using twilio 2.4.

What version of cordova ios are you running @henritoivar ? Can you try downgrading your twilio version to 2.4.X to see if it fix the issues ?

I have no access to my workstation for now, will test it asap.

henritoivar · 2020-09-16T17:53:54Z

@hthetiot I'm still having the same issue with unpublishing and republishing ( video not reappearing on the other side / or black)

But it seems i can now work around and avoid using unpublish/publish as i can now use replaceTrack (available in twilio using track.restart) to switch between front/rear camera. Also it seems i can implement muting the video/audio track by using enable/disable track without unpublishing/publishing.

@sboudouk I did try using twilio 2.4 and had the same issues. Actually i'm using Capacitor. I will also test this on cordova and get back with the results.

hthetiot · 2020-09-16T19:01:11Z

But it seems i can now work around and avoid using unpublish/publish as i can now use replaceTrack (available in twilio using track.restart) to switch between front/rear camera. Also it seems i can implement muting the video/audio track by using enable/disable track without unpublishing/publishing.

good to know @henritoivar

I have no access to my workstation for now, will test it asap.

if you test its all on master @sboudouk

henritoivar · 2020-09-18T12:05:43Z

@hthetiot @sboudouk

I also tested using cordova 10.0.0, cordova-ios 6.1.1. The following issues persist:

Video stream not visible on browser after unpublishing and then republishing video on iOS device (using toggleVideo)
Video stream hangs on browser after switching camera on iOS using track.restart

You can test it out aswell by cloning this repository: https://github.com/henritoivar/test-iosrtc

It's possible to work around restarting the track, but it seems there is some kind of memory leak happening. Probably a result of how the streams are terminated. Something i noticed is that the cordova.plugins.iosrtc.mediaStreams object will contain quite a few stream objects after the call has been closed.

I managed to get the video stream showing again on the browser after restarting the track on iOS by using the following hack:

async function switchCameraWorking() {
  facing = !facing;
  videoTrack.restart({ facingMode: facing ? 'user' : { exact: 'environment' } });
  setTimeout(() => {
    videoTrack.disable();
    room.localParticipant.unpublishTrack(videoTrack);
    setTimeout(() => {
      videoTrack.enable();
      room.localParticipant.publishTrack(videoTrack);
    }, 1000);
  }, 1000);
}

The problem with this hack is that if you switch the camera multiple times, then the app seems to get slower and will finally hangs. Probably a result of the beforementioned memory leak from unpublishing track.

hthetiot · 2020-09-23T10:27:26Z

@henritoivar I wonder what unpublishTrack and publishTrack does, we may need to shim better MediaTrack to fix this issue and prevent your workarround.

Notice that the issue with track destruction is fixed on master via cc5bafc and will be released as part of 6.0.14.

To test master:

cordova plugin remove cordova-plugin-iosrtc --verbose
cordova plugin add https://github.com/cordova-rtc/cordova-plugin-iosrtc#master --verbose
cordova platform remove ios --no-save
cordova platform add ios --no-save

hthetiot · 2020-09-23T10:30:29Z

related #449

henritoivar · 2020-09-23T10:50:16Z

@hthetiot Thanks, i'll give it a try.

Also something i came upon. Might be unrelated, but maybe something worth taking note of:

I was trying to track down why unified plan doesn't work with Twilio. Twilio sets a isUnifiedPlan flag to implement unified-plan specific track management: https://github.com/twilio/twilio-video.js/blob/c86e0001d3706e2c13b1914cffb2f08faae2285c/lib/signaling/v2/peerconnection.js

const sdpFormat = getSdpFormat(configuration.sdpSemantics);
const isUnifiedPlan = sdpFormat === 'unified';

The problem here is that getSdpFormat depends on browser detection and in WKWebView it will always return null, thus isUnifiedPlan will always be false. In this case RTCPeerConnection still initializes with unified plan (default), but Twilio will use plan-b specific track management. Thus we get errors like: setRemoteDescription() failed: Failed to set remote answer sdp: The order of m-lines in answer doesn't match order in offer

I tried hardcoding Twilio to use unified plan, but it seems in that case Twilio is using RTCPeerConnection.addTransceiver which is not yet supported in iosrtc. See the following lines:
It sets localMediaStream in case of plan-b.

const localMediaStream = isUnifiedPlan ? null : new options.MediaStream();

Which is later referenced to whether to use addTransceiver (in case of unified-plan) or addTrack (in case of plan-b).

 addMediaTrackSender(mediaTrackSender) {
    if (this._peerConnection.signalingState === 'closed' || this._rtpSenders.has(mediaTrackSender)) {
      return;
    }
    let sender;
    if (this._localMediaStream) {
      this._localMediaStream.addTrack(mediaTrackSender.track);
      sender = this._peerConnection.addTrack(mediaTrackSender.track, this._localMediaStream);
    } else {
      const transceiver = this._addOrUpdateTransceiver(mediaTrackSender.track);
      sender = transceiver.sender;
    }
    mediaTrackSender.addSender(sender);
    this._rtpSenders.set(mediaTrackSender, sender);
  }

So it seems to get unified-plan working with Twilio we would need Twilio to implement a way of setting isUnifiedPlan from configuration and in iosrtc we would need addTransceiver support.

henritoivar · 2020-09-24T10:18:17Z

Regarding unpublishing and then republishing track:

I can see a WebSocket message going out that the track is published:
When doing this flow on Chrome:

When doing the same flow on iOS:

In Chrome after this message I also see a message coming in that the track should be published:

But in iOS in the incoming message this track has disappeared for some reason:

Something to note of also is that the track id(this is the TrackSender/MediaStreamTrack id) and name on Chrome are different, but on iOS name and id of the track are equal.
Also on Chrome when unpublishing and then republishing the track id (TrackSender/MediaStreamTrack id) changes, but the name stays the same . On iOS both the name and id stay the same.

I'm doing some more digging to see why this is happening.

@hthetiot Technically the initial error message in this issue is fixed, but there are still issues with republishing a track. Should i create a new issue for the republishing problem, or keep it here?

hthetiot · 2020-09-24T10:28:11Z

Technically the initial error message in this issue is fixed, but there are still issues with republishing a track. Should i create a new issue for the republishing problem, or keep it here?

That fine Let keep it about republishing a track, still i will release original fix on 6.0.14 in soon.

hthetiot · 2020-09-24T10:35:23Z

@hthetiot after disconnecting there is still a large amount of MediaStream objects in cordova.plugins.iosrtc.mediaStreams.

see new separate issue #575
cc @henritoivar @samgabriel

henritoivar · 2020-09-25T07:48:34Z

@hthetiot Thanks a lot. Let me know if i can help anyhow.

henritoivar · 2020-10-09T08:21:42Z

@hthetiot Have you managed to look into this?

I'm considering having a go at fixing this. Any pointers are welcome.

hthetiot · 2020-10-16T20:18:35Z

@hthetiot Have you managed to look into this?

Not yet, we focusing on #576 for 6.0.14 for now, however, i did technicaly fixed the "removeTrack() must be called with a RTCRtpSender instance as argument" as part of 6.0.14 and master.

hthetiot · 2020-10-17T01:25:39Z

@hthetiot Have you managed to look into this?

I'm considering having a go at fixing this. Any pointers are welcome.

Feel free to give it a shoot, it's good to know or understand the source code you depend or use anyway. Notice that if it was that simple I would have fix it already (this issue is going in so many directions it's not even clear for me anymore).

hthetiot · 2020-10-17T01:39:55Z

Please create separate issue @henritoivar there is one for clone already.

hthetiot · 2020-10-17T23:05:14Z

@henritoivar Please try this PR see testing instructions: #587

hthetiot · 2020-11-08T16:51:46Z

@henritoivar can you check #592

hthetiot · 2020-11-19T11:51:26Z

The last know Twillio issue should have been fixed on #605 that has landed on master target is 6.0.16

Can you test @sboudouk @henritoivar @dgreif we kept Janus fix in place also.

henritoivar changed the title ~~Error: removeTrack() msut be called with a RTCRtpSender instance as argument~~ Error: removeTrack() must be called with a RTCRtpSender instance as argument Sep 14, 2020

hthetiot self-assigned this Sep 15, 2020

hthetiot added this to the 6.0.14 milestone Sep 15, 2020

hthetiot added the bug label Sep 15, 2020

This comment has been minimized.

Sign in to view

hthetiot changed the title ~~Error: removeTrack() must be called with a RTCRtpSender instance as argument~~ republishing Track fail using Twillio (Was Error: removeTrack() must be called with a RTCRtpSender instance as argument) Sep 24, 2020

hthetiot modified the milestones: 6.0.14, 6.0.x Sep 24, 2020

hthetiot mentioned this issue Sep 24, 2020

after disconnecting there is still a large amount of MediaStream objects in cordova.plugins.iosrtc.mediaStreams. #575

Closed

3 tasks

This comment has been minimized.

Sign in to view

hthetiot modified the milestones: 6.0.x, 6.0.15 Oct 16, 2020

hthetiot added the third party library support label Oct 16, 2020

This comment has been minimized.

Sign in to view

hthetiot closed this as completed Oct 17, 2020

hthetiot modified the milestones: 6.0.15, 6.0.14 Oct 17, 2020

This comment has been minimized.

Sign in to view

cordova-rtc locked as too heated and limited conversation to collaborators Oct 17, 2020

hthetiot modified the milestones: 6.0.14, 6.0.17 Nov 19, 2020

hthetiot reopened this Nov 19, 2020

hthetiot closed this as completed Nov 19, 2020

hthetiot modified the milestones: 6.1.1, 6.0.16 Jan 20, 2021

republishing Track fail using Twillio (Was Error: removeTrack() must be called with a RTCRtpSender instance as argument) #569

republishing Track fail using Twillio (Was Error: removeTrack() must be called with a RTCRtpSender instance as argument) #569

Comments

henritoivar commented Sep 14, 2020 • edited by hthetiot Loading

Versions affected

Description

Steps to reproduce

Expected results

Actual results

hthetiot commented Sep 15, 2020

hthetiot commented Sep 15, 2020 • edited Loading

hthetiot commented Sep 15, 2020

hthetiot commented Sep 15, 2020

henritoivar commented Sep 16, 2020

henritoivar commented Sep 16, 2020

hthetiot commented Sep 16, 2020

hthetiot commented Sep 16, 2020

sboudouk commented Sep 16, 2020

henritoivar commented Sep 16, 2020

hthetiot commented Sep 16, 2020

henritoivar commented Sep 18, 2020

hthetiot commented Sep 23, 2020

hthetiot commented Sep 23, 2020

henritoivar commented Sep 23, 2020

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

henritoivar commented Sep 24, 2020 • edited Loading

hthetiot commented Sep 24, 2020

hthetiot commented Sep 24, 2020 • edited Loading

henritoivar commented Sep 25, 2020

henritoivar commented Oct 9, 2020

This comment has been minimized.

This comment has been minimized.

hthetiot commented Oct 16, 2020

This comment has been minimized.

hthetiot commented Oct 17, 2020 • edited Loading

hthetiot commented Oct 17, 2020

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

hthetiot commented Oct 17, 2020 • edited Loading

hthetiot commented Nov 8, 2020

hthetiot commented Nov 19, 2020

henritoivar commented Sep 14, 2020 •

edited by hthetiot

Loading

hthetiot commented Sep 15, 2020 •

edited

Loading

henritoivar commented Sep 24, 2020 •

edited

Loading

hthetiot commented Sep 24, 2020 •

edited

Loading

hthetiot commented Oct 17, 2020 •

edited

Loading

hthetiot commented Oct 17, 2020 •

edited

Loading