Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Listen few audios at the same time [WPB-11180] #3639

Merged
merged 9 commits into from
Nov 26, 2024
+92 −21
Merged

fix: Listen few audios at the same time [WPB-11180] #3639

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b94a1eb
fix: Listen few audios at the same time
borichellow Nov 15, 2024
588db26
Merge branch 'develop' into fix/listen_few_audios_at_the_same_time
borichellow Nov 15, 2024
c4872ab
Fix build
borichellow Nov 15, 2024
8f34688
Added singleton ConversationAudioMessagePlayerProvider to provide pla…
borichellow Nov 18, 2024
5c33a4b
Merge branch 'develop' into fix/listen_few_audios_at_the_same_time
borichellow Nov 18, 2024
8deee05
Cleared code
borichellow Nov 18, 2024
f5ef9e8
Merge branch 'develop' into fix/listen_few_audios_at_the_same_time
borichellow Nov 19, 2024
b2ff0b7
Merge branch 'develop' into fix/listen_few_audios_at_the_same_time
borichellow Nov 25, 2024
057be71
Review fixes
borichellow Nov 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 51 additions & 13 deletions app/src/main/kotlin/com/wire/android/media/audiomessage/ConversationAudioMessagePlayer.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@ import android.content.Context
import android.media.MediaPlayer
import android.media.MediaPlayer.SEEK_CLOSEST_SYNC
import android.net.Uri
import com.wire.android.di.KaliumCoreLogic
import com.wire.kalium.logic.CoreLogic
import com.wire.kalium.logic.data.id.ConversationId
import com.wire.kalium.logic.feature.asset.GetMessageAssetUseCase
import com.wire.kalium.logic.feature.asset.MessageAssetResult
import com.wire.kalium.logic.feature.session.CurrentSessionResult
import kotlinx.coroutines.channels.BufferOverflow
import kotlinx.coroutines.coroutineScope
import kotlinx.coroutines.delay
Expand All @@ -34,14 +36,44 @@ import kotlinx.coroutines.flow.filterNotNull
import kotlinx.coroutines.flow.flow
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.merge
import kotlinx.coroutines.flow.onStart
import kotlinx.coroutines.launch
import javax.inject.Inject
import javax.inject.Singleton

class ConversationAudioMessagePlayer
@Singleton
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

making it a Singleton it not really the way to go since we do not want to keep it in memory forever maybe try @reusable instead ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is potential for many resusable being used at the same time, when threads gets involved. Not sure if that matters in this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unfortunately @Reusable is useless here, as it doesn't guaranty "not re-creating" a new instance of Player. So i added a Singleton Provider, which will provide a player instance (create it if needed) and free space, when player is not used anymore

class ConversationAudioMessagePlayerProvider
@Inject constructor(
private val context: Context,
private val audioMediaPlayer: MediaPlayer,
private val getMessageAsset: GetMessageAssetUseCase
@KaliumCoreLogic private val coreLogic: CoreLogic,
) {
private var player: ConversationAudioMessagePlayer? = null
private var usageCount: Int = 0

@Synchronized
fun provide(): ConversationAudioMessagePlayer {
val player = player ?: ConversationAudioMessagePlayer(context, audioMediaPlayer, coreLogic).also { player = it }
usageCount++

return player
}

@Synchronized
fun onCleared() {
usageCount--
if (usageCount <= 0) {
player?.close()
player = null
}
}
}

class ConversationAudioMessagePlayer
internal constructor(
private val context: Context,
private val audioMediaPlayer: MediaPlayer,
@KaliumCoreLogic private val coreLogic: CoreLogic,
) {
private companion object {
const val UPDATE_POSITION_INTERVAL_IN_MS = 1000L
Expand Down Expand Up @@ -137,7 +169,7 @@ class ConversationAudioMessagePlayer
}

audioMessageStateHistory
}
}.onStart { emit(audioMessageStateHistory) }

private var currentAudioMessageId: String? = null

Expand Down Expand Up @@ -169,10 +201,10 @@ class ConversationAudioMessagePlayer
}

private suspend fun stopCurrentlyPlayingAudioMessage() {
if (currentAudioMessageId != null) {
val currentAudioState = audioMessageStateHistory[currentAudioMessageId]
currentAudioMessageId?.let {
val currentAudioState = audioMessageStateHistory[it]
if (currentAudioState?.audioMediaPlayingState != AudioMediaPlayingState.Fetching) {
stop(currentAudioMessageId!!)
stop(it)
}
}
}
Expand All @@ -194,14 +226,22 @@ class ConversationAudioMessagePlayer

coroutineScope {
launch {
val currentAccountResult = coreLogic.getGlobalScope().session.currentSession()
if (currentAccountResult is CurrentSessionResult.Failure) return@launch

audioMessageStateUpdate.emit(
AudioMediaPlayerStateUpdate.AudioMediaPlayingStateUpdate(
messageId,
AudioMediaPlayingState.Fetching
)
)

when (val result = getMessageAsset(conversationId, messageId).await()) {
val assetMessage = coreLogic
.getSessionScope((currentAccountResult as CurrentSessionResult.Success).accountInfo.userId)
.messages
.getAssetMessage(conversationId, messageId)

when (val result = assetMessage.await()) {
is MessageAssetResult.Success -> {
audioMessageStateUpdate.emit(
AudioMediaPlayerStateUpdate.AudioMediaPlayingStateUpdate(
Expand All @@ -219,9 +259,7 @@ class ConversationAudioMessagePlayer
)
audioMediaPlayer.prepare()

if (position != null) {
audioMediaPlayer.seekTo(position)
}
if (position != null) audioMediaPlayer.seekTo(position)

audioMediaPlayer.start()

Expand Down Expand Up @@ -292,7 +330,7 @@ class ConversationAudioMessagePlayer
)
}

fun close() {
audioMediaPlayer.release()
internal fun close() {
audioMediaPlayer.reset()
}
}
7 changes: 4 additions & 3 deletions ...n/kotlin/com/wire/android/ui/home/conversations/messages/ConversationMessagesViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import androidx.lifecycle.SavedStateHandle
import androidx.lifecycle.viewModelScope
import com.wire.android.R
import com.wire.android.appLogger
import com.wire.android.media.audiomessage.ConversationAudioMessagePlayer
import com.wire.android.media.audiomessage.ConversationAudioMessagePlayerProvider
import com.wire.android.model.SnackBarMessage
import com.wire.android.navigation.SavedStateViewModel
import com.wire.android.ui.home.conversations.ConversationNavArgs
Expand Down Expand Up @@ -98,7 +98,7 @@ class ConversationMessagesViewModel @Inject constructor(
private val getMessageForConversation: GetMessagesForConversationUseCase,
private val toggleReaction: ToggleReactionUseCase,
private val resetSession: ResetSessionUseCase,
private val conversationAudioMessagePlayer: ConversationAudioMessagePlayer,
private val conversationAudioMessagePlayerProvider: ConversationAudioMessagePlayerProvider,
private val getConversationUnreadEventsCount: GetConversationUnreadEventsCountUseCase,
private val clearUsersTypingEvents: ClearUsersTypingEventsUseCase,
private val getSearchedConversationMessagePosition: GetSearchedConversationMessagePositionUseCase,
Expand All @@ -108,6 +108,7 @@ class ConversationMessagesViewModel @Inject constructor(
private val conversationNavArgs: ConversationNavArgs = savedStateHandle.navArgs()
val conversationId: QualifiedID = conversationNavArgs.conversationId
private val searchedMessageIdNavArgs: String? = conversationNavArgs.searchedMessageId
private val conversationAudioMessagePlayer = conversationAudioMessagePlayerProvider.provide()

var conversationViewState by mutableStateOf(
ConversationMessagesViewState(
Expand Down Expand Up @@ -436,7 +437,7 @@ class ConversationMessagesViewModel @Inject constructor(

override fun onCleared() {
super.onCleared()
conversationAudioMessagePlayer.close()
conversationAudioMessagePlayerProvider.onCleared()
}

private companion object {
Expand Down
34 changes: 30 additions & 4 deletions app/src/test/kotlin/com/wire/android/media/ConversationAudioMessagePlayerTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,12 @@ import com.wire.android.framework.FakeKaliumFileSystem
import com.wire.android.media.audiomessage.AudioMediaPlayingState
import com.wire.android.media.audiomessage.AudioState
import com.wire.android.media.audiomessage.ConversationAudioMessagePlayer
import com.wire.kalium.logic.CoreLogic
import com.wire.kalium.logic.data.auth.AccountInfo
import com.wire.kalium.logic.data.id.ConversationId
import com.wire.kalium.logic.feature.asset.GetMessageAssetUseCase
import com.wire.kalium.logic.data.user.UserId
import com.wire.kalium.logic.feature.asset.MessageAssetResult
import com.wire.kalium.logic.feature.session.CurrentSessionResult
import io.mockk.MockKAnnotations
import io.mockk.coEvery
import io.mockk.every
Expand All @@ -45,11 +48,14 @@ class ConversationAudioMessagePlayerTest {
val (arrangement, conversationAudioMessagePlayer) = Arrangement()
.withAudioMediaPlayerReturningTotalTime(1000)
.withSuccessFullAssetFetch()
.withCurrentSession()
.arrange()

val testAudioMessageId = "some-dummy-message-id"

conversationAudioMessagePlayer.observableAudioMessagesState.test {
// skip first emit from onStart
awaitItem()
conversationAudioMessagePlayer.playAudio(
ConversationId("some-dummy-value", "some.dummy.domain"),
testAudioMessageId
Expand Down Expand Up @@ -95,13 +101,16 @@ class ConversationAudioMessagePlayerTest {
fun givenTheSuccessFullAssetFetch_whenPlayingTheSameMessageIdTwiceSequentially_thenEmitStatesAsExpected() = runTest {
val (arrangement, conversationAudioMessagePlayer) = Arrangement()
.withSuccessFullAssetFetch()
.withCurrentSession()
.withAudioMediaPlayerReturningTotalTime(1000)
.withMediaPlayerPlaying()
.arrange()

val testAudioMessageId = "some-dummy-message-id"

conversationAudioMessagePlayer.observableAudioMessagesState.test {
// skip first emit from onStart
awaitItem()
// playing first time
conversationAudioMessagePlayer.playAudio(
ConversationId("some-dummy-value", "some.dummy.domain"),
Expand Down Expand Up @@ -161,13 +170,16 @@ class ConversationAudioMessagePlayerTest {
runTest {
val (arrangement, conversationAudioMessagePlayer) = Arrangement()
.withSuccessFullAssetFetch()
.withCurrentSession()
.withAudioMediaPlayerReturningTotalTime(1000)
.arrange()

val firstAudioMessageId = "some-dummy-message-id1"
val secondAudioMessageId = "some-dummy-message-id2"

conversationAudioMessagePlayer.observableAudioMessagesState.test {
// skip first emit from onStart
awaitItem()
// playing first audio message
conversationAudioMessagePlayer.playAudio(
ConversationId("some-dummy-value", "some.dummy.domain"),
Expand Down Expand Up @@ -242,13 +254,16 @@ class ConversationAudioMessagePlayerTest {
runTest {
val (arrangement, conversationAudioMessagePlayer) = Arrangement()
.withSuccessFullAssetFetch()
.withCurrentSession()
.withAudioMediaPlayerReturningTotalTime(1000)
.arrange()

val firstAudioMessageId = "some-dummy-message-id1"
val secondAudioMessageId = "some-dummy-message-id2"

conversationAudioMessagePlayer.observableAudioMessagesState.test {
// skip first emit from onStart
awaitItem()
// playing first audio message
conversationAudioMessagePlayer.playAudio(
ConversationId("some-dummy-value", "some.dummy.domain"),
Expand Down Expand Up @@ -366,13 +381,16 @@ class ConversationAudioMessagePlayerTest {
runTest {
val (arrangement, conversationAudioMessagePlayer) = Arrangement()
.withSuccessFullAssetFetch()
.withCurrentSession()
.withAudioMediaPlayerReturningTotalTime(1000)
.withMediaPlayerPlaying()
.arrange()

val testAudioMessageId = "some-dummy-message-id"

conversationAudioMessagePlayer.observableAudioMessagesState.test {
// skip first emit from onStart
awaitItem()
// playing first time
conversationAudioMessagePlayer.playAudio(
ConversationId("some-dummy-value", "some.dummy.domain"),
Expand Down Expand Up @@ -454,7 +472,7 @@ class Arrangement {
lateinit var context: Context

@MockK
lateinit var getMessageAssetUseCase: GetMessageAssetUseCase
lateinit var coreLogic: CoreLogic

@MockK
lateinit var mediaPlayer: MediaPlayer
Expand All @@ -463,16 +481,24 @@ class Arrangement {
ConversationAudioMessagePlayer(
context,
mediaPlayer,
getMessageAssetUseCase,
coreLogic,
)
}

init {
MockKAnnotations.init(this, relaxed = true)
}

fun withCurrentSession() = apply {
coEvery { coreLogic.getGlobalScope().session.currentSession.invoke() } returns CurrentSessionResult.Success(
AccountInfo.Valid(UserId("some-user-value", "some.user.domain"))
)
}

fun withSuccessFullAssetFetch() = apply {
coEvery { getMessageAssetUseCase.invoke(any(), any()) } returns CompletableDeferred(
coEvery {
coreLogic.getSessionScope(any()).messages.getAssetMessage.invoke(any<ConversationId>(), any<String>())
} returns CompletableDeferred(
MessageAssetResult.Success(
decodedAssetPath = FakeKaliumFileSystem().selfUserAvatarPath(),
assetSize = 0,
Expand Down
8 changes: 7 additions & 1 deletion ...m/wire/android/ui/home/conversations/messages/ConversationMessagesViewModelArrangement.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import com.wire.android.config.TestDispatcherProvider
import com.wire.android.config.mockUri
import com.wire.android.media.audiomessage.AudioState
import com.wire.android.media.audiomessage.ConversationAudioMessagePlayer
import com.wire.android.media.audiomessage.ConversationAudioMessagePlayerProvider
import com.wire.android.ui.home.conversations.ConversationNavArgs
import com.wire.android.ui.home.conversations.model.AssetBundle
import com.wire.android.ui.home.conversations.model.UIMessage
Expand Down Expand Up @@ -96,6 +97,9 @@ class ConversationMessagesViewModelArrangement {
@MockK
lateinit var conversationAudioMessagePlayer: ConversationAudioMessagePlayer

@MockK
lateinit var conversationAudioMessagePlayerProvider: ConversationAudioMessagePlayerProvider

@MockK
lateinit var getConversationUnreadEventsCount: GetConversationUnreadEventsCountUseCase

Expand Down Expand Up @@ -124,7 +128,7 @@ class ConversationMessagesViewModelArrangement {
getMessagesForConversationUseCase,
toggleReaction,
resetSession,
conversationAudioMessagePlayer,
conversationAudioMessagePlayerProvider,
getConversationUnreadEventsCount,
clearUsersTypingEvents,
getSearchedConversationMessagePosition,
Expand All @@ -143,6 +147,8 @@ class ConversationMessagesViewModelArrangement {
coEvery { getConversationUnreadEventsCount(any()) } returns GetConversationUnreadEventsCountUseCase.Result.Success(0L)
coEvery { updateAssetMessageDownloadStatus(any(), any(), any()) } returns UpdateTransferStatusResult.Success
coEvery { clearUsersTypingEvents() } returns Unit
every { conversationAudioMessagePlayerProvider.provide() } returns conversationAudioMessagePlayer
every { conversationAudioMessagePlayerProvider.onCleared() } returns Unit
coEvery {
getSearchedConversationMessagePosition(any(), any())
} returns GetSearchedConversationMessagePositionUseCase.Result.Success(position = 0)
Expand Down
Loading