search

LiMium / mini-vector-android

A simpler Matrix client for Android, with fewer permissions and dependencies
Apache License 2.0
44 stars 4 forks source link

Voice and video 1:1 chat? #14

Open gerroon opened 6 years ago

gerroon commented 6 years ago

Hi

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

thanks

hrj commented 6 years ago

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

Will try to fix this soon.

hrj commented 6 years ago

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 :+1: reaction if you agree that the tradeoff is acceptable, else :-1:

MurzNN commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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 https://github.com/vector-im/riot-android/issues/2620 as it should help everybody, not just this fork.

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

hrj commented 6 years ago

The current progress on this can be tried out by compiling https://github.com/LiMium/mini-vector-android/pull/18

ghost commented 6 years ago

IIRC Riot-Android previously used libjingle for its WebRTC.

gerroon commented 6 years ago

@Matrixcoffee What does it use now?

ghost commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

The commit is cf7766faf79b99e326c78645a2d35c705734c1d1 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 commented 6 years ago

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.