From 6833d5f14348a5e43a05dc675bad0ac282afce2c Mon Sep 17 00:00:00 2001 From: Oussama Hassine Date: Mon, 25 Mar 2024 11:42:34 +0100 Subject: [PATCH] fix: Calling video not streamed when enabling camera on preview screen (WPB-7114) - cherrypick (#2808) --- .../android/di/accountScoped/CallsModule.kt | 10 ++++- .../ui/calling/SharedCallingViewModel.kt | 25 ++++------- .../ui/calling/ongoing/OngoingCallScreen.kt | 10 ++++- .../calling/ongoing/OngoingCallViewModel.kt | 29 +++++++++++- .../ui/calling/OngoingCallViewModelTest.kt | 21 +++++++++ .../ui/calling/SharedCallingViewModelTest.kt | 45 +++---------------- kalium | 2 +- 7 files changed, 83 insertions(+), 59 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 098f73210f..c0f98555a7 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 4eccb51976..26d101a46e 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 @@ -52,7 +52,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 @@ -127,8 +127,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() } } } @@ -279,6 +280,11 @@ class SharedCallingViewModel @Inject constructor( callState = callState.copy( isCameraOn = !callState.isCameraOn ) + if (callState.isCameraOn) { + updateVideoState(conversationId, VideoState.STARTED) + } else { + updateVideoState(conversationId, VideoState.STOPPED) + } } } @@ -286,7 +292,6 @@ class SharedCallingViewModel @Inject constructor( viewModelScope.launch { appLogger.i("SharedCallingViewModel: clearing video preview..") setVideoPreview(conversationId, PlatformView(null)) - updateVideoState(conversationId, VideoState.STOPPED) } } @@ -295,18 +300,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 1696dd17dc..baddd56f44 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 @@ -122,8 +122,14 @@ fun OngoingCallScreen( hangUpCall = { sharedCallingViewModel.hangUpCall(navigator::navigateBack) }, toggleVideo = sharedCallingViewModel::toggleVideo, flipCamera = sharedCallingViewModel::flipCamera, - setVideoPreview = sharedCallingViewModel::setVideoPreview, - clearVideoPreview = sharedCallingViewModel::clearVideoPreview, + setVideoPreview = { + sharedCallingViewModel.setVideoPreview(it) + ongoingCallViewModel.startSendingVideoFeed() + }, + clearVideoPreview = { + sharedCallingViewModel.clearVideoPreview() + ongoingCallViewModel.stopSendingVideoFeed() + }, navigateBack = navigator::navigateBack, requestVideoStreams = ongoingCallViewModel::requestVideoStreams, hideDoubleTapToast = ongoingCallViewModel::hideDoubleTapToast 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 10ee083f22..9b3abb360b 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 @@ -33,11 +33,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 @@ -54,7 +57,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() @@ -70,6 +74,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() } @@ -77,6 +82,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 37fa4e544a..ddb7a36e4f 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 @@ -31,12 +31,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 @@ -68,6 +70,9 @@ class OngoingCallViewModelTest { @MockK private lateinit var currentScreenManager: CurrentScreenManager + @MockK + private lateinit var setVideoSendState: SetVideoSendStateUseCase + @MockK private lateinit var globalDataStore: GlobalDataStore @@ -80,6 +85,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, @@ -87,10 +93,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 4fae4db752..82e7ffe820 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 @@ -42,7 +42,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 @@ -261,6 +261,7 @@ class SharedCallingViewModelTest { advanceUntilIdle() sharedCallingViewModel.callState.isCameraOn shouldBeEqualTo false + coVerify(exactly = 1) { updateVideoState(any(), VideoState.STOPPED) } } @Test @@ -272,6 +273,7 @@ class SharedCallingViewModelTest { advanceUntilIdle() sharedCallingViewModel.callState.isCameraOn shouldBeEqualTo true + coVerify(exactly = 1) { updateVideoState(any(), VideoState.STARTED) } } @Test @@ -315,57 +317,24 @@ 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 sharedCallingViewModel.setVideoPreview(view) advanceUntilIdle() coVerify(exactly = 2) { setVideoPreview(any(), any()) } - coVerify(exactly = 1) { updateVideoState(any(), VideoState.STARTED) } } @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 e9c9ef1a9d..e19050e052 160000 --- a/kalium +++ b/kalium @@ -1 +1 @@ -Subproject commit e9c9ef1a9d68ce317a73dd040d6f505fba21a932 +Subproject commit e19050e052f0ea341b3e313fada4c07bf77f122b