Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jitsi integration: use the matching WebRTC framework #1483

Closed
saghul opened this issue Aug 28, 2017 · 9 comments
Closed

jitsi integration: use the matching WebRTC framework #1483

saghul opened this issue Aug 28, 2017 · 9 comments

Comments

@saghul
Copy link

saghul commented Aug 28, 2017

Hey folks!

I'm one of the Jitsi devs. Glad to see Jitsi is getting integrated :-) I noticed you are using the WebRTC framework directly from CocoaPods. That is not the one we link with and may yield to problems. Please use this one: https://github.com/jitsi/react-native-webrtc/tree/master/ios/WebRTC.framework

We make those builds and it's guaranteed to be compatible with the SDK. In addition, and after the 57 release fallout, we include backports for issues we have run into.

@superdump
Copy link

Hey @saghul ! I think @manuroe is the best person to discuss this with for iOS. I have a suspicion that I know what the response may be - we use Jitsi for group calls, not 1:1 and we were using a WebRTC.framework for 1:1 calls previously. It could be that the choice was to use the framework we already had, which I believe was a newer version, instead of the version shipped by Jitsi.

As a note, I think @ylecollen is having similar issues on Android. Ideally we'd like to just bundle one version of the WebRTC framework / package and use that for both Jitsi and 1:1. I think on Android some crash was observed when trying to use one or the other WebRTC package but I'll have to let @ylecollen clarify.

@manuroe
Copy link
Member

manuroe commented Aug 31, 2017

Hi @saghul. Thanks for the feedback.

You are right. I am not comfortable with such mix of lib versions too. At first, I doubted this mix could even work but it works at the end.
As @superdump said, before the integration of Jitsi, we were already using a WebRTC framework provided by the pod called WebRTC (https://github.com/Anakros/WebRTC.git). We have been using it for more than one year without issue. We still want to do simple WebRTC for 1:1 rooms.

Thanks for the https://github.com/jitsi/react-native-webrtc/tree/master/ios/WebRTC.framework link. I wondered where this framework was coming from.
We would be very happy to move to your WebRTC.framework version but without a pod, it makes it complicate to use it in a project and in our build server. The notice about Apple submission (https://github.com/jitsi/react-native-webrtc/blob/master/Documentation/iOSInstallation.md#appendix-b---apple-store-submission) also means other tricks to apply on the building step.

Is it planned to make an app store compatible pod of it?
(well, I guess this is a sub-task of jitsi/jitsi-meet#1854)

@saghul
Copy link
Author

saghul commented Sep 1, 2017

(Damn, I had typed a long comment which apparently got lost grrrrr)

Hey @superdump and @manuroe!

You are right. I am not comfortable with such mix of lib versions too. At first, I doubted this mix could even work but it works at the end.

Hum, that's unexpected actually. IIRC WebRTC M60 made some not-backwards-compatible changes in handling the audio or video devices. Or maybe not :-P

We still want to do simple WebRTC for 1:1 rooms.

I see. Any specific reason not to use Jitsi for that too? We have made the 1:1 case a first class citizen not so long ago. The following will happen while there are 2 participants in a room:

  • Audio will always be P2P instead of going through the JVB (we have TURN servers in case they are needed)
  • H.264 is used since it's usually HW accelerated and no simulcast it needed

Then when a 3rd participant joins, we switch to the JVB and use VP8 with simulcast, RTX and the whole shebang.

We would be very happy to move to your WebRTC.framework version but without a pod, it makes it complicate to use it in a project and in our build server.

I guess you could embed it just like you do with the Jitsi Meet SDK?

The notice about Apple submission (https://github.com/jitsi/react-native-webrtc/blob/master/Documentation/iOSInstallation.md#appendix-b---apple-store-submission) also means other tricks to apply on the building step.

If you embed the framework just like you do the Jitsi Meet SDK you could strip the simulator architectures before committing the code to the repo and not have to worry about it at build time.

Is it planned to make an app store compatible pod of it?
(well, I guess this is a sub-task of jitsi/jitsi-meet#1854)

Yeah, but we have no ETA yet, sorry.

PS: Feel free to reach out if you have any questions: saghul at jitsi dot org.

@manuroe
Copy link
Member

manuroe commented Sep 1, 2017

Hi @saghul,

We still want to do simple WebRTC for 1:1 rooms.

I see. Any specific reason not to use Jitsi for that too?

I will let @ara4n or @AmandineLP provide an explanation for this cross-platform choice.

I guess you could embed it just like you do with the Jitsi Meet SDK?

Sure this is an option but I would like to avoid to do that again, specially with a huge lib like WebRTC. My concern is to avoid to explode our git repo size. Our Riot Android repo has some libs hardly embedded in the project and it becomes painful to git clone.
Libs dependency managers also provide a better release tagging. Honestly, I do not know what is in the Jitsi Meet SDK I embedded in the project :)

In the other hand, I understand all the benefits to use your WebRTC framework :|

@manuroe
Copy link
Member

manuroe commented Mar 5, 2019

We can now use the jitsi pod. More info at https://github.com/jitsi/jitsi-meet-ios-sdk-releases.

@saghul
Copy link
Author

saghul commented Mar 5, 2019

@manuroe Note that the Jitsi pod includes the matching WebRTC framework.

@manuroe
Copy link
Member

manuroe commented Mar 5, 2019

@manuroe Note that the Jitsi pod includes the matching WebRTC framework.

Yes, we are very interested by it too :)

@manuroe
Copy link
Member

manuroe commented Mar 6, 2019

Action plan:

  • MatrixSDK: Update the dedicated MatrixSDK subspec to use the Jitsi pod
  • MatrixSDK: Check build with pod lib lint MatrixSDK.podspec --allow-warnings & pod lib lint SwiftMatrixSDK.podspec --allow-warnings
  • Riot-iOS: Remove the build jitsi framework from the Riot project and remove its folder from the repo
  • Riot-iOS: Build & fix possible API breaks
  • Riot-iOS: Test 1:1 calls (WebRTC)
  • Riot-iOS: Test conference calls (Jitsi)

@saghul
Copy link
Author

saghul commented Apr 25, 2019

👏👏👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants