fix: Listen few audios at the same time [WPB-11180] #3639
Conversation
borichellow
requested review from
m-zagorski,
Garzas,
saleniuk and
damian-kaczmarek
echoes-hq
bot
added
the
echoes: unplanned
|
|
||
| @Singleton |
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3639 +/- ##
===========================================
- Coverage 46.04% 46.03% -0.02%
===========================================
Files 472 472
Lines 16108 16127 +19
Branches 2666 2670 +4
===========================================
+ Hits 7417 7424 +7
- Misses 7917 7928 +11
- Partials 774 775 +1
Continue to review full report in Codecov by Sentry.
|
…yer and free space when it's needed
app/src/main/kotlin/com/wire/android/media/audiomessage/ConversationAudioMessagePlayer.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/com/wire/android/media/audiomessage/ConversationAudioMessagePlayer.kt
Outdated
Show resolved
Hide resolved
|
Built wire-android-staging-compat-pr-3639.apk is available for download |
|
Built wire-android-dev-debug-pr-3639.apk is available for download |
|
Built wire-android-staging-compat-pr-3639.apk is available for download |
|
Built wire-android-dev-debug-pr-3639.apk is available for download |
| } | ||
| usageCount++ | ||
|
|
||
| return player!! |
There was a problem hiding this comment.
[Just a thought] Im not a big fan of the !! operator. so maybe something like:
val player = player ?: ConversationAudioMessagePlayer(context, audioMediaPlayer, coreLogic).also { player = it }
usageCount++
return player
|
|
Built wire-android-staging-compat-pr-3639.apk is available for download |
|
Built wire-android-dev-debug-pr-3639.apk is available for download |
What's new in this PR?
Issues
STR:
Result:
Both audio messages play at the same time. The first one continues uninterrupted and the second message starts from the beginning
Causes (Optional)
Both
ConversationandConversationsMediascreens useConversationMessagesViewModelwhich usesConversationAudioMessagePlayer. As far as separate instance of ViewModel is created for each screen, separate player are used and each of it has no control over other.Solutions
Added singleton
ConversationAudioMessagePlayerProvider, which can provide player and counts usages of it.So when new ViewModel needs player it will refer to the same instance of player and can pause previously played audio if needed. On the other hand: manager counts usages of player and when it's not needed anymore - remove it from memory.