Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Voice and video 1:1 chat? #14

Open
gerroon opened this issue Sep 12, 2018 · 15 comments
Open

Voice and video 1:1 chat? #14

gerroon opened this issue Sep 12, 2018 · 15 comments
Assignees

Comments

@gerroon
Copy link

gerroon commented Sep 12, 2018

Hi

Can you please enable the voice chat stuff, I do not mean Jitsi, I mean webrtc.

thanks

@hrj
Copy link

hrj commented Sep 16, 2018

I didn't realize that 1:1 calls use webrtc, and that they don't require Jitsi.

Will try to fix this soon.

@hrj hrj self-assigned this Sep 16, 2018
@hrj hrj changed the title Voice chat? Voice and video 1:1 chat? Sep 21, 2018
@hrj
Copy link

hrj commented Sep 21, 2018

Enabling 1:1 voice and video chat requires including the "react-native-webrtc" dependency. This increases the application size from 15M to 22M.

It also requires two new permissions: Record Audio and Modify audio settings. Record audio is a permission that user can control in recent versions of Android.

With these changes, the feature works fine, and I think these changes are an acceptable tradeoff.

Poll: Please add 👍 reaction if you agree that the tradeoff is acceptable, else 👎

@MurzNN
Copy link

MurzNN commented Sep 21, 2018

Can we load "react-native-webrtc" library on demand only? The application size is not the main problem for users, main problems are memory and cpu usage, app load time, app performance.
So maybe add checkbox "Enable webrtc calls" in app settings, that will load library only when enabled? Or load it only when first call is required.

@MurzNN
Copy link

MurzNN commented Sep 21, 2018

Another solution is provide different versions of app, like Open GApps variants: https://opengapps.org/ - pico, nano, micro, mini... If we can configure automatic building different versions - this will be good solution for users.

@hrj
Copy link

hrj commented Sep 21, 2018

Can we load "react-native-webrtc" library on demand only?

That's a good idea; I will spend some more time in exploring this option.

Another solution is provide different versions of app, like Open GApps variants: https://opengapps.org/ - pico, nano, micro, mini... If we can configure automatic building different versions - this will be good solution for users.

That's a solution, but before that, I want to know whether adding "react-native-webrtc" makes miniVector almost the same as riot-android. If yes, then we can think of alt versions, because it will take a lot of maintenance effort.

@ghost
Copy link

ghost commented Sep 21, 2018

That's too bad. I thought the basic 1:1 functionality worked without react native.

If you have to add back react native, it's almost the same as the original Riot, and there's not much justification left for maintaining this fork.

@gerroon
Copy link
Author

gerroon commented Sep 21, 2018

@Matrixcoffee I personally do not agree with that, removing analytics and jitsi integration are definetely very positive and makes it a distinct branch in my view.

@hrj
Copy link

hrj commented Sep 27, 2018

Good points by everyone; I am still not decided one way or the other.

I think lazy loading will be minimum requirement for this change to qualilfy for miniVector. I have raised an issue upstream element-hq#2620 as it should help everybody, not just this fork.

When that gets implemented (or rejected), we can revisit this.

@hrj
Copy link

hrj commented Oct 13, 2018

The current progress on this can be tried out by compiling #18

@ghost
Copy link

ghost commented Oct 16, 2018

IIRC Riot-Android previously used libjingle for its WebRTC.

@gerroon
Copy link
Author

gerroon commented Oct 16, 2018

@Matrixcoffee What does it use now?

@ghost
Copy link

ghost commented Oct 19, 2018

Looks like it's using Jitsi or one of its dependencies, written in react native. Jitsi also communicates over WebRTC.

Including two different libraries to implement the same protocol makes little sense, so I understand why they dropped libjingle.

@gerroon
Copy link
Author

gerroon commented Oct 19, 2018

@Matrixcoffee

I wonder if that explains why the audio quality dropped since recent versions :( I felt like I was getting much better voice audio quality before 0.16

@ghost
Copy link

ghost commented Oct 23, 2018

The commit is cf7766f which was over a year ago, and confirms that jingle was in fact dropped.

Maybe @hrj can back that commit out, instead of removing Jitsi the way it is done now.

@hrj
Copy link

hrj commented Oct 24, 2018

My half-educated guess is that:

  1. libjingle was a build of WebRTC maintained by pristine.io
  2. The support for libjingle was discontinued by pristine. See for example, this
  3. Support for video conf calls in react-android was added through Jitsi.
  4. Jitsi needed react-native.
  5. The webrtc support provided by react-native was also sufficient for the voice calls. Hence libjingle was removed and react-native-webrtc was used instead.

Maybe @hrj can back that commit out, instead of removing Jitsi the way it is done now.

That's a good idea, but a tad late to do that now. Think merge conflicts.

However, I can refer to that commit to decide which binary files to prune from the repo. There are many font files there which might not be used outside of the Jitsi.

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

No branches or pull requests

3 participants