MM-54553 - Calls: Enable microphone input in background (Android) #7585

cpoile · 2023-10-04T22:35:47Z

Summary

A recent Android update seems to have changed the way apps are allowed to use the microphone from the background (or we only actually tested for a short period of time and it's always been this way).
This update fixes that by turning Mattermost into a foreground service, but only when a call is active. Otherwise there's no change to Mattermost. (See: https://developer.android.com/guide/components/foreground-services, and the react native module I used is: https://github.com/voximplant/react-native-foreground-service -- that's a helper to define a service class, register it, and start it up.)
That part doesn't affect iOS. The iOS change is slight -- add voip to the list of background modes. This is just a protection, I'm not 100% sure it does anything.
- Note: there's a little quirk with iOS. If the user backgrounds the app, and no voice tracks (in either direction) are initiated, the peer connection will close after about 50s. If a voice track is established (even for a second), the rtc session will never close no matter how long it's in the background or behind the lock screen. Not sure if it warrants an investigation and a fix, because it's a pretty rare edge case I think. Open to ideas though and we can open a new ticket for that.
Now Android and iOS will allow microphone in the background, behind the lock screen, etc. for as long as you want (my current test is 30+minutes and counting for the three devices listed below).

Test steps

Start a call, unmute, send app to background or turn off device
Before: microphone would work for 50-60s, then stop
After: microphone will work until call is closed (afaict)

Screenshots

This is what the user will see now when the app is backgrounded while on a call (and only when on a call):

tablet	phone

Ticket Link

Checklist

Added or updated unit tests (required for all new features)
Has UI changes
Includes text changes and localization file updates
Have tested against the 5 core themes to ensure consistency between them.

Device Information

Android: 13, Pixel 6
Android: 13, Galaxy Tab s7+
iOS: 16.5.1, iPhone 14

Release Note

Calls: Fixed bug where microphone will stop working when Mattermost is put in the background (Android)

streamer45 · 2023-10-04T23:27:14Z

Note: there's a little quirk with iOS. If the user backgrounds the app, and no voice tracks (in either direction) are initiated, the peer connection will close after about 50s. If a voice track is established (even for a second), the rtc session will never close no matter how long it's in the background or behind the lock screen. Not sure if it warrants an investigation and a fix, because it's a pretty rare edge case I think. Open to ideas though and we can open a new ticket for that.

That doesn't sound great. It's not that rare to be waiting a minute or so in silence before a large meeting and a mobile user may actually take the time to do other stuff. I'd definitely want to track this to investigate further.

enahum

To add to Claudio's comment, taken from iOS docs

Use background execution modes sparingly because overuse can negatively impact device performance and battery life. If an alternative to executing in the background exists, use the alternative instead. For example, apps can use the significant-change location service to receive location events as an alternative to using the Location updates background mode.

Have we measure the impact on battery and device perf?

See https://developer.apple.com/documentation/xcode/configuring-background-execution-modes

enahum · 2023-10-05T02:35:12Z

app/products/calls/connection/foreground_service.ts

+};
+
+export const foregroundServiceStart = async () => {
+    await VIForegroundService.getInstance().createNotificationChannel(channelConfig);


Why create the notification channel every time it starts? Creating the channel once should be enough

Calling it again with the same params is a noop, but just to be clean I separated it into a setup call. 👍

cpoile · 2023-10-05T14:55:02Z

That doesn't sound great. It's not that rare to be waiting a minute or so in silence before a large meeting and a mobile user may actually take the time to do other stuff. I'd definitely want to track this to investigate further.

I've created https://mattermost.atlassian.net/browse/MM-54765 . I still think this would be rare, but I'll look to see if there's a simple way to keep the connection alive.

cpoile · 2023-10-05T14:58:21Z

To add to Claudio's comment, taken from iOS docs

Use background execution modes sparingly because overuse can negatively impact device performance and battery life. If an alternative to executing in the background exists, use the alternative instead. For example, apps can use the significant-change location service to receive location events as an alternative to using the Location updates background mode.

Have we measure the impact on battery and device perf?

See https://developer.apple.com/documentation/xcode/configuring-background-execution-modes

I removed iOS changes from this PR, to keep it focused on fixing the Android issue. I'll tackle that in https://mattermost.atlassian.net/browse/MM-54765

@enahum What method have we used in the past to measure the impact of PRs on battery and device perf? (so that I can use that in the next PR)

enahum · 2023-10-05T15:58:45Z

To add to Claudio's comment, taken from iOS docs

Use background execution modes sparingly because overuse can negatively impact device performance and battery life. If an alternative to executing in the background exists, use the alternative instead. For example, apps can use the significant-change location service to receive location events as an alternative to using the Location updates background mode.

Have we measure the impact on battery and device perf?

See https://developer.apple.com/documentation/xcode/configuring-background-execution-modes

I removed iOS changes from this PR, to keep it focused on fixing the Android issue. I'll tackle that in https://mattermost.atlassian.net/browse/MM-54765

@enahum What method have we used in the past to measure the impact of PRs on battery and device perf? (so that I can use that in the next PR)

I normally check the battery app report, also you can use xcode instrumentation

streamer45

Great fix! 💯

I am only slightly concerned about the added dependency which doesn't seem the most frequently used nor up to date one but I guess we may always provide a custom implementation in the future if necessary.

streamer45 · 2023-10-05T16:20:22Z

app/products/calls/connection/foreground_service.ts

+export const foregroundServiceStart = async () => {
+    const notificationConfig = {
+        channelId: 'calls_channel',
+        id: 345678,


Should there be some logic behind the id value? Should it be a constant somewhere?

It's completely arbitrary (just has to be unique for the channel), and since we're not using this outside the file, or need to change it, 1/5 I don't think we need a constant.

mattermost-build · 2023-10-05T16:26:48Z

Building app in separate branch.

DHaussermann · 2023-10-06T17:57:20Z

@cpoile This is working for me now.
I tested this on a Samsung S22 running Android 13 with the stock One UI 5.1 Launcher

I tested for over 15 minutes and could still actively use calls
Both listening on a call as well as well as using the microphone to speak continued working
Tested both cases of pushing the app to the background in the OS as well as well as locking the screen
Also briefly regression tested iOS build as a precaution since it is out of scope

There's only 1 thing to follow-up up on with the screens above ☝️

I see the icon in the top indicating that the app is active
I see the notification in the stack

I do not see this green badge thing

My badge just shows up as a red 1 as if I had any other type of mention. (I made sure there were no mention badges before getting on the call.)
I'm not sure I have this screen on my Samsung launcher. Did you just pull down from settings? I don't know if I get a count of my running apps.

cpoile · 2023-10-10T15:27:56Z

@DHaussermann

My badge just shows up as a red 1 as if I had any other type of mention. (I made sure there were no mention badges before getting on the call.)

That's what I get on my Samsung tablet too, so it might be a samsung thing.

I'm not sure I have this screen on my Samsung launcher. Did you just pull down from settings? I don't know if I get a count of my running apps.

It comes from pulling down the top of the screen (to get the Internet, bluetooth, flashlight, dnd buttons). Then pull those buttons down again to expand it, and you might see the running apps bar on the bottom of that expanded screen. (That's on my pixel.) On my Samsung tablet I don't have that, I have the slot in the notifications list (after pulling down the top of the screen once) -- see the second row first column in the screenshots above.

DHaussermann · 2023-10-10T18:45:14Z

@cpoile I'm not sure if there's much more needed here.

Not getting the green badge on Samsung launcher is a bit unfortunate. Given the relatively small importance compared to the rest of the PR, would it seem reasonable to you if I opened a separate Jira issue for this and it can then be prioritized as appropriate rather then preventing this PR from merging?

DHaussermann

Tested and passed based on testing above ☝️

@cpoile I have created a separate Jira here https://mattermost.atlassian.net/browse/MM-54935 for the badge issue so this can logged and investigated based on whatever priority we decide we want to assign.

This PR can now be merged 👍

…ttermost#7585) * Add foreground service for Android; add voip UIBackgroundMode for iOS * remove iOS change for this PR * create foreground notification channel once at startup

Add foreground service for Android; add voip UIBackgroundMode for iOS

39f19df

cpoile added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Oct 4, 2023

cpoile requested review from streamer45, enahum and DHaussermann October 4, 2023 22:35

mm-cloud-bot added release-note labels Oct 4, 2023

enahum requested changes Oct 5, 2023

View reviewed changes

cpoile added 2 commits October 5, 2023 09:18

remove iOS change for this PR

2ecbdf5

create foreground notification channel once at startup

f7c2b85

cpoile requested a review from enahum October 5, 2023 14:58

enahum approved these changes Oct 5, 2023

View reviewed changes

streamer45 approved these changes Oct 5, 2023

View reviewed changes

DHaussermann added the Build Apps for PR Build the mobile app for iOS and Android to test label Oct 5, 2023

mattermost-build removed the Build Apps for PR Build the mobile app for iOS and Android to test label Oct 5, 2023

DHaussermann approved these changes Oct 13, 2023

View reviewed changes

DHaussermann added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Oct 13, 2023

cpoile merged commit 1fb01df into main Oct 13, 2023
7 checks passed

cpoile deleted the MM-54446-calls-disconnecting-in-background branch October 13, 2023 18:45

amyblais added this to the v2.10.0 milestone Oct 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MM-54553 - Calls: Enable microphone input in background (Android) #7585

MM-54553 - Calls: Enable microphone input in background (Android) #7585

cpoile commented Oct 4, 2023

streamer45 commented Oct 4, 2023

enahum left a comment

enahum Oct 5, 2023

cpoile Oct 5, 2023

cpoile commented Oct 5, 2023

cpoile commented Oct 5, 2023

enahum commented Oct 5, 2023

streamer45 left a comment

streamer45 Oct 5, 2023

cpoile Oct 5, 2023

mattermost-build commented Oct 5, 2023

DHaussermann commented Oct 6, 2023 •

edited

Loading

cpoile commented Oct 10, 2023

DHaussermann commented Oct 10, 2023

DHaussermann left a comment

MM-54553 - Calls: Enable microphone input in background (Android) #7585

MM-54553 - Calls: Enable microphone input in background (Android) #7585

Conversation

cpoile commented Oct 4, 2023

Summary

Test steps

Screenshots

Ticket Link

Checklist

Device Information

Release Note

streamer45 commented Oct 4, 2023

enahum left a comment

Choose a reason for hiding this comment

enahum Oct 5, 2023

Choose a reason for hiding this comment

cpoile Oct 5, 2023

Choose a reason for hiding this comment

cpoile commented Oct 5, 2023

cpoile commented Oct 5, 2023

enahum commented Oct 5, 2023

streamer45 left a comment

Choose a reason for hiding this comment

streamer45 Oct 5, 2023

Choose a reason for hiding this comment

cpoile Oct 5, 2023

Choose a reason for hiding this comment

mattermost-build commented Oct 5, 2023

DHaussermann commented Oct 6, 2023 • edited Loading

cpoile commented Oct 10, 2023

DHaussermann commented Oct 10, 2023

DHaussermann left a comment

Choose a reason for hiding this comment

DHaussermann commented Oct 6, 2023 •

edited

Loading