react-native-twilio-video-webrtc icon indicating copy to clipboard operation
react-native-twilio-video-webrtc copied to clipboard

interop issues between browser webRTC and iOS WebRTC with TwilioVideo 4.6

Open shaibt opened this issue 3 years ago • 3 comments

Steps to reproduce

TwilioVideo 4.6 introduces new SDP semantics that are not working 100% when connecting to P2P rooms that include browser based clients using twilio-video-js. Main issue is that iOS clients in P2P rooms are not always receiving media from browser based clients due to issues in SDP compatability. I think Twilio are aware of these issues.

This package uses this semver for podspec: s.dependency 'TwilioVideo', '~> 4.1' which causes auto "upgrade" to 4.6 if rebuilt which isn't desirable because it intro's instability in interop with browser based clients in P2P rooms.

Expected behaviour

Probably better to use full semver semantic for podspec so that only incremental updates for TwilioVideo are automatic: s.dependency 'TwilioVideo', '~> 4.1.0' (or maybe 4.4.0). Also, would be beneficial to tag new releases that introduce new TwilioVideo versions so that npm could pull previous releases of this package.

Environment

  • Node JS 14.X
  • RN 0.64.1
  • iOS 14

react-native-twilio-video-webrtc

Version: "master"

shaibt avatar Oct 05 '21 17:10 shaibt

Hi @shaibt,

A goal for the 4.6.0 release was to reduce or eliminate interoperability issues by having all SDKs use the same semantics for SDPs. Since the browsers are a constantly moving target there are occasions where we might miss the mark and need to fix things. However, I don't think that is justification to pin to a minor release forever, or automatically.

It's not my choice to make for this project, but just 2c from a developer who works on Twilio Video.

Best, Chris

ceaglest avatar Oct 06 '21 20:10 ceaglest

@ceaglest I wasn't suggesting to pin the SDK to 4.4. The problem is that this package in its latest official version (2.0) uses a dependency notation that automatically updates any Twilio Video SDK to 4.X which results in unwanted env changes in case of package install. For example 4.4 -> 4.6 causes interop issues and we were "upgraded" to 4.6 w/o any explicit change on our side. My suggestion was to pin SDK 4.4 for ver 2.0 of this package and then release a new ver of this package (lets say 2.1) that pulls the latest SDK ver. This way the dev can choose what package they'd like to use in a project.

shaibt avatar Oct 07 '21 06:10 shaibt

Hi @shaibt and project authors,

First off, the original issue reported here should be resolved with Twilio Video 4.6.1. Thank you for your patience while we worked on it. We don't intend to break customers with a minor version update, and you did the right thing by reporting the issue and following up with us for a fix.

For example 4.4 -> 4.6 causes interop issues and we were "upgraded" to 4.6 w/o any explicit change on our side.

We use semantic versioning for our SDKs, which is a statement about API and behavioural compatibility. I consider this interoperability issue a smalle scale bug that was introduced as part of a larger bug fix / enhancement targeted at Peer-to-Peer Rooms.

My suggestion was to pin SDK 4.4 for ver 2.0 of this package and then release a new ver of this package (lets say 2.1) that pulls the latest SDK ver. This way the dev can choose what package they'd like to use in a project.

I won't offer further advice on the versioning of this project as it's not really my place.

That being said, changing SDP semantics is an internal SDK decision, one that was actually made on the fly by our JavaScript 2.x SDK for several years. Also, whenever you speak to our media servers in a Group Room your communications must be converted to / from unified plan. This has been the case since 2017 when Group Rooms first launched.

We don't want you to stick with 4.4 or 4.5 which is why we released 4.6 that uses unified plan, and presents a compatible API and behavior (this interop bug was an unfortunate oversight).

Best, Chris

ceaglest avatar Oct 16 '21 04:10 ceaglest