From 8d2540bba44df57362eab99f283530ecb6263bef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Tue, 25 Jun 2024 13:31:42 +0200 Subject: [PATCH 1/3] fix: camera on/off button not working in fullscreen [WPB-9815] --- .../android/di/accountScoped/CallsModule.kt | 10 ++++- .../ui/calling/SharedCallingViewModel.kt | 25 ++++------- .../ui/calling/ongoing/OngoingCallScreen.kt | 11 +++++ .../calling/ongoing/OngoingCallViewModel.kt | 29 ++++++++++++- .../ui/calling/OngoingCallViewModelTest.kt | 20 +++++++++ .../ui/calling/SharedCallingViewModelTest.kt | 43 +++---------------- kalium | 2 +- 7 files changed, 85 insertions(+), 55 deletions(-) diff --git a/app/src/main/kotlin/com/wire/android/di/accountScoped/CallsModule.kt b/app/src/main/kotlin/com/wire/android/di/accountScoped/CallsModule.kt index bdaf07cc524..572afd561e1 100644 --- a/app/src/main/kotlin/com/wire/android/di/accountScoped/CallsModule.kt +++ b/app/src/main/kotlin/com/wire/android/di/accountScoped/CallsModule.kt @@ -37,7 +37,8 @@ import com.wire.kalium.logic.feature.call.usecase.StartCallUseCase import com.wire.kalium.logic.feature.call.usecase.TurnLoudSpeakerOffUseCase import com.wire.kalium.logic.feature.call.usecase.TurnLoudSpeakerOnUseCase import com.wire.kalium.logic.feature.call.usecase.UnMuteCallUseCase -import com.wire.kalium.logic.feature.call.usecase.UpdateVideoStateUseCase +import com.wire.kalium.logic.feature.call.usecase.video.SetVideoSendStateUseCase +import com.wire.kalium.logic.feature.call.usecase.video.UpdateVideoStateUseCase import dagger.hilt.InstallIn import dagger.hilt.android.components.ViewModelComponent import dagger.hilt.android.scopes.ViewModelScoped @@ -167,6 +168,13 @@ class CallsModule { ): UpdateVideoStateUseCase = callsScope.updateVideoState + @ViewModelScoped + @Provides + fun provideSetVideoSendStateUseCase( + callsScope: CallsScope + ): SetVideoSendStateUseCase = + callsScope.setVideoSendState + @ViewModelScoped @Provides fun provideIsCallRunningUseCase(callsScope: CallsScope) = diff --git a/app/src/main/kotlin/com/wire/android/ui/calling/SharedCallingViewModel.kt b/app/src/main/kotlin/com/wire/android/ui/calling/SharedCallingViewModel.kt index 032a7004443..4e556ad0d6e 100644 --- a/app/src/main/kotlin/com/wire/android/ui/calling/SharedCallingViewModel.kt +++ b/app/src/main/kotlin/com/wire/android/ui/calling/SharedCallingViewModel.kt @@ -54,7 +54,7 @@ import com.wire.kalium.logic.feature.call.usecase.SetVideoPreviewUseCase import com.wire.kalium.logic.feature.call.usecase.TurnLoudSpeakerOffUseCase import com.wire.kalium.logic.feature.call.usecase.TurnLoudSpeakerOnUseCase import com.wire.kalium.logic.feature.call.usecase.UnMuteCallUseCase -import com.wire.kalium.logic.feature.call.usecase.UpdateVideoStateUseCase +import com.wire.kalium.logic.feature.call.usecase.video.UpdateVideoStateUseCase import com.wire.kalium.logic.feature.conversation.ObserveConversationDetailsUseCase import com.wire.kalium.logic.util.PlatformView import dagger.hilt.android.lifecycle.HiltViewModel @@ -129,8 +129,9 @@ class SharedCallingViewModel @Inject constructor( private suspend fun observeScreenState() { currentScreenManager.observeCurrentScreen(viewModelScope).collect { - if (it == CurrentScreen.InBackground) { - stopVideo() + // clear video preview when the screen is in background to avoid memory leaks + if (it == CurrentScreen.InBackground && callState.isCameraOn) { + clearVideoPreview() } } } @@ -275,6 +276,11 @@ class SharedCallingViewModel @Inject constructor( callState = callState.copy( isCameraOn = !callState.isCameraOn ) + if (callState.isCameraOn) { + updateVideoState(conversationId, VideoState.STARTED) + } else { + updateVideoState(conversationId, VideoState.STOPPED) + } } } @@ -282,7 +288,6 @@ class SharedCallingViewModel @Inject constructor( viewModelScope.launch { appLogger.i("SharedCallingViewModel: clearing video preview..") setVideoPreview(conversationId, PlatformView(null)) - updateVideoState(conversationId, VideoState.STOPPED) } } @@ -291,18 +296,6 @@ class SharedCallingViewModel @Inject constructor( appLogger.i("SharedCallingViewModel: setting video preview..") setVideoPreview(conversationId, PlatformView(null)) setVideoPreview(conversationId, PlatformView(view)) - updateVideoState(conversationId, VideoState.STARTED) - } - } - - fun stopVideo() { - viewModelScope.launch { - if (callState.isCameraOn) { - appLogger.i("SharedCallingViewModel: stopping video..") - callState = callState.copy(isCameraOn = false, isSpeakerOn = false) - clearVideoPreview() - turnLoudSpeakerOff() - } } } } diff --git a/app/src/main/kotlin/com/wire/android/ui/calling/ongoing/OngoingCallScreen.kt b/app/src/main/kotlin/com/wire/android/ui/calling/ongoing/OngoingCallScreen.kt index 0c163499973..f409331f736 100644 --- a/app/src/main/kotlin/com/wire/android/ui/calling/ongoing/OngoingCallScreen.kt +++ b/app/src/main/kotlin/com/wire/android/ui/calling/ongoing/OngoingCallScreen.kt @@ -81,6 +81,7 @@ import com.wire.android.ui.theme.wireColorScheme import com.wire.android.ui.theme.wireDimensions import com.wire.android.ui.theme.wireTypography import com.wire.android.util.ui.PreviewMultipleThemes +import com.wire.kalium.logic.data.call.CallStatus import com.wire.kalium.logic.data.id.ConversationId import java.util.Locale @@ -126,6 +127,16 @@ fun OngoingCallScreen( ) BackHandler(enabled = isCameraOn, navigator::navigateBack) } + + // Start/stop sending video feed based on the camera state when the call is established. + LaunchedEffect(sharedCallingViewModel.callState.callStatus, sharedCallingViewModel.callState.isCameraOn) { + if (sharedCallingViewModel.callState.callStatus == CallStatus.ESTABLISHED) { + when (sharedCallingViewModel.callState.isCameraOn) { + true -> ongoingCallViewModel.startSendingVideoFeed() + false -> ongoingCallViewModel.stopSendingVideoFeed() + } + } + } } @OptIn(ExperimentalMaterial3Api::class) diff --git a/app/src/main/kotlin/com/wire/android/ui/calling/ongoing/OngoingCallViewModel.kt b/app/src/main/kotlin/com/wire/android/ui/calling/ongoing/OngoingCallViewModel.kt index 7df9b0662ae..ee27e5c9eaa 100644 --- a/app/src/main/kotlin/com/wire/android/ui/calling/ongoing/OngoingCallViewModel.kt +++ b/app/src/main/kotlin/com/wire/android/ui/calling/ongoing/OngoingCallViewModel.kt @@ -35,11 +35,14 @@ import com.wire.android.ui.calling.model.UICallParticipant import com.wire.android.ui.navArgs import com.wire.android.util.CurrentScreen import com.wire.android.util.CurrentScreenManager +import com.wire.kalium.logic.data.call.Call import com.wire.kalium.logic.data.call.CallClient +import com.wire.kalium.logic.data.call.VideoState import com.wire.kalium.logic.data.id.QualifiedID import com.wire.kalium.logic.data.user.UserId import com.wire.kalium.logic.feature.call.usecase.ObserveEstablishedCallsUseCase import com.wire.kalium.logic.feature.call.usecase.RequestVideoStreamsUseCase +import com.wire.kalium.logic.feature.call.usecase.video.SetVideoSendStateUseCase import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.delay import kotlinx.coroutines.flow.distinctUntilChanged @@ -56,7 +59,8 @@ class OngoingCallViewModel @Inject constructor( private val globalDataStore: GlobalDataStore, private val establishedCalls: ObserveEstablishedCallsUseCase, private val requestVideoStreams: RequestVideoStreamsUseCase, - private val currentScreenManager: CurrentScreenManager, + private val setVideoSendState: SetVideoSendStateUseCase, + private val currentScreenManager: CurrentScreenManager ) : ViewModel() { private val ongoingCallNavArgs: CallingNavArgs = savedStateHandle.navArgs() @@ -72,6 +76,7 @@ class OngoingCallViewModel @Inject constructor( init { viewModelScope.launch { establishedCalls().first { it.isNotEmpty() }.run { + initCameraState(this) // We start observing once we have an ongoing call observeCurrentCall() } @@ -79,6 +84,28 @@ class OngoingCallViewModel @Inject constructor( showDoubleTapToast() } + private fun initCameraState(calls: List) { + val currentCall = calls.find { call -> call.conversationId == conversationId } + currentCall?.let { + if (it.isCameraOn) { + startSendingVideoFeed() + } else { + stopSendingVideoFeed() + } + } + } + + fun startSendingVideoFeed() { + viewModelScope.launch { + setVideoSendState(conversationId, VideoState.STARTED) + } + } + fun stopSendingVideoFeed() { + viewModelScope.launch { + setVideoSendState(conversationId, VideoState.STOPPED) + } + } + private suspend fun observeCurrentCall() { establishedCalls() .distinctUntilChanged() diff --git a/app/src/test/kotlin/com/wire/android/ui/calling/OngoingCallViewModelTest.kt b/app/src/test/kotlin/com/wire/android/ui/calling/OngoingCallViewModelTest.kt index a28668a90cf..8e0da849450 100644 --- a/app/src/test/kotlin/com/wire/android/ui/calling/OngoingCallViewModelTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/calling/OngoingCallViewModelTest.kt @@ -33,12 +33,14 @@ import com.wire.android.util.CurrentScreenManager import com.wire.kalium.logic.data.call.Call import com.wire.kalium.logic.data.call.CallClient import com.wire.kalium.logic.data.call.CallStatus +import com.wire.kalium.logic.data.call.VideoState import com.wire.kalium.logic.data.conversation.Conversation import com.wire.kalium.logic.data.id.ConversationId import com.wire.kalium.logic.data.id.QualifiedID import com.wire.kalium.logic.data.user.UserId import com.wire.kalium.logic.feature.call.usecase.ObserveEstablishedCallsUseCase import com.wire.kalium.logic.feature.call.usecase.RequestVideoStreamsUseCase +import com.wire.kalium.logic.feature.call.usecase.video.SetVideoSendStateUseCase import io.mockk.MockKAnnotations import io.mockk.coEvery import io.mockk.coVerify @@ -70,6 +72,9 @@ class OngoingCallViewModelTest { @MockK private lateinit var currentScreenManager: CurrentScreenManager + @MockK + private lateinit var setVideoSendState: SetVideoSendStateUseCase + @MockK private lateinit var globalDataStore: GlobalDataStore @@ -89,10 +94,25 @@ class OngoingCallViewModelTest { requestVideoStreams = requestVideoStreams, currentScreenManager = currentScreenManager, currentUserId = currentUserId, + setVideoSendState = setVideoSendState, globalDataStore = globalDataStore, ) } + @Test + fun givenAnOngoingCall_WhenTurningOnCamera_ThenSetVideoSendStateToStarted() = runTest { + ongoingCallViewModel.startSendingVideoFeed() + + coVerify(exactly = 1) { setVideoSendState.invoke(any(), VideoState.STARTED) } + } + + @Test + fun givenAnOngoingCall_WhenTurningOffCamera_ThenSetVideoSendStateToStopped() = runTest { + ongoingCallViewModel.stopSendingVideoFeed() + + coVerify { setVideoSendState.invoke(any(), VideoState.STOPPED) } + } + @Test fun givenParticipantsList_WhenRequestingVideoStream_ThenRequestItForOnlyParticipantsWithVideoEnabled() = runTest { val expectedClients = listOf( diff --git a/app/src/test/kotlin/com/wire/android/ui/calling/SharedCallingViewModelTest.kt b/app/src/test/kotlin/com/wire/android/ui/calling/SharedCallingViewModelTest.kt index 937ec100f02..d2805501904 100644 --- a/app/src/test/kotlin/com/wire/android/ui/calling/SharedCallingViewModelTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/calling/SharedCallingViewModelTest.kt @@ -44,7 +44,7 @@ import com.wire.kalium.logic.feature.call.usecase.SetVideoPreviewUseCase import com.wire.kalium.logic.feature.call.usecase.TurnLoudSpeakerOffUseCase import com.wire.kalium.logic.feature.call.usecase.TurnLoudSpeakerOnUseCase import com.wire.kalium.logic.feature.call.usecase.UnMuteCallUseCase -import com.wire.kalium.logic.feature.call.usecase.UpdateVideoStateUseCase +import com.wire.kalium.logic.feature.call.usecase.video.UpdateVideoStateUseCase import com.wire.kalium.logic.feature.conversation.ObserveConversationDetailsUseCase import io.mockk.MockKAnnotations import io.mockk.coEvery @@ -263,6 +263,7 @@ class SharedCallingViewModelTest { advanceUntilIdle() sharedCallingViewModel.callState.isCameraOn shouldBeEqualTo false + coVerify(exactly = 1) { updateVideoState(any(), VideoState.STOPPED) } } @Test @@ -274,6 +275,7 @@ class SharedCallingViewModelTest { advanceUntilIdle() sharedCallingViewModel.callState.isCameraOn shouldBeEqualTo true + coVerify(exactly = 1) { updateVideoState(any(), VideoState.STARTED) } } @Test @@ -317,7 +319,7 @@ class SharedCallingViewModelTest { } @Test - fun `given an active call, when setVideoPreview is called, then set the video preview and update video state to STARTED`() = + fun `given a call, when setVideoPreview is called, then set the video preview`() = runTest { coEvery { setVideoPreview(any(), any()) } returns Unit coEvery { updateVideoState(any(), any()) } returns Unit @@ -330,44 +332,13 @@ class SharedCallingViewModelTest { } @Test - fun `given an active call, when clearVideoPreview is called, then update video state to STOPPED`() = - runTest { - coEvery { setVideoPreview(any(), any()) } returns Unit - coEvery { updateVideoState(any(), any()) } returns Unit - - sharedCallingViewModel.clearVideoPreview() - advanceUntilIdle() - - coVerify(exactly = 1) { updateVideoState(any(), VideoState.STOPPED) } - } - - @Test - fun `given a video call, when stopping video, then clear Video Preview and turn off speaker`() = - runTest { - sharedCallingViewModel.callState = - sharedCallingViewModel.callState.copy(isCameraOn = true) - coEvery { setVideoPreview(any(), any()) } returns Unit - coEvery { updateVideoState(any(), any()) } returns Unit - coEvery { turnLoudSpeakerOff() } returns Unit - - sharedCallingViewModel.stopVideo() - advanceUntilIdle() - - coVerify(exactly = 1) { setVideoPreview(any(), any()) } - coVerify(exactly = 1) { turnLoudSpeakerOff() } - } - - @Test - fun `given an audio call, when stopVideo is invoked, then do not do anything`() = runTest { - sharedCallingViewModel.callState = sharedCallingViewModel.callState.copy(isCameraOn = false) + fun `given a call, when clearVideoPreview is called, then clear view`() = runTest { coEvery { setVideoPreview(any(), any()) } returns Unit - coEvery { turnLoudSpeakerOff() } returns Unit - sharedCallingViewModel.stopVideo() + sharedCallingViewModel.clearVideoPreview() advanceUntilIdle() - coVerify(inverse = true) { setVideoPreview(any(), any()) } - coVerify(inverse = true) { turnLoudSpeakerOff() } + coVerify(exactly = 1) { setVideoPreview(any(), any()) } } companion object { diff --git a/kalium b/kalium index 86b2becb901..0edb3a6f14b 160000 --- a/kalium +++ b/kalium @@ -1 +1 @@ -Subproject commit 86b2becb901d426451fde3aca70d3b9c522d80d8 +Subproject commit 0edb3a6f14b2df712d20729a2b90b8a9e288d97c From 365a4a2195d2ad519e5cb1cb9a00954fe1877081 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Tue, 25 Jun 2024 13:39:39 +0200 Subject: [PATCH 2/3] fix test --- .../com/wire/android/ui/calling/SharedCallingViewModelTest.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/app/src/test/kotlin/com/wire/android/ui/calling/SharedCallingViewModelTest.kt b/app/src/test/kotlin/com/wire/android/ui/calling/SharedCallingViewModelTest.kt index d2805501904..2a94fa7f879 100644 --- a/app/src/test/kotlin/com/wire/android/ui/calling/SharedCallingViewModelTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/calling/SharedCallingViewModelTest.kt @@ -328,7 +328,6 @@ class SharedCallingViewModelTest { advanceUntilIdle() coVerify(exactly = 2) { setVideoPreview(any(), any()) } - coVerify(exactly = 1) { updateVideoState(any(), VideoState.STARTED) } } @Test From 680424d1bb90a7c5ae99fd6eb40b5d8e2e71c312 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Tue, 25 Jun 2024 15:10:32 +0200 Subject: [PATCH 3/3] fix test --- .../com/wire/android/ui/calling/OngoingCallViewModelTest.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/app/src/test/kotlin/com/wire/android/ui/calling/OngoingCallViewModelTest.kt b/app/src/test/kotlin/com/wire/android/ui/calling/OngoingCallViewModelTest.kt index 8e0da849450..d89f9dd7518 100644 --- a/app/src/test/kotlin/com/wire/android/ui/calling/OngoingCallViewModelTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/calling/OngoingCallViewModelTest.kt @@ -87,6 +87,7 @@ class OngoingCallViewModelTest { coEvery { establishedCall.invoke() } returns flowOf(listOf(provideCall())) coEvery { currentScreenManager.observeCurrentScreen(any()) } returns MutableStateFlow(CurrentScreen.SomeOther) coEvery { globalDataStore.getShouldShowDoubleTapToast(any()) } returns false + coEvery { setVideoSendState.invoke(any(), any()) } returns Unit ongoingCallViewModel = OngoingCallViewModel( savedStateHandle = savedStateHandle,