From eaa7262b2f29c4268a9bca9528cbec14abd02963 Mon Sep 17 00:00:00 2001 From: Alex Hart Date: Fri, 20 May 2022 14:54:24 -0300 Subject: [PATCH] Add debug log entry for video player pool usage. --- .../mp4/GiphyMp4ProjectionPlayerHolder.java | 4 +- .../logsubmit/LogSectionExoPlayerPool.kt | 39 +++++++++++++ .../logsubmit/SubmitDebugLogRepository.java | 1 + .../VideoMediaPreviewFragment.java | 2 +- .../mediasend/VideoEditorFragment.java | 2 +- .../revealable/ViewOnceMessageActivity.java | 2 +- .../securesms/video/VideoPlayer.java | 4 +- .../video/exo/SimpleExoPlayerPool.kt | 56 ++++++++++++------- .../securesms/video/exo/ExoPlayerPoolTest.kt | 12 ++-- 9 files changed, 90 insertions(+), 32 deletions(-) create mode 100644 app/src/main/java/org/thoughtcrime/securesms/logsubmit/LogSectionExoPlayerPool.kt diff --git a/app/src/main/java/org/thoughtcrime/securesms/giph/mp4/GiphyMp4ProjectionPlayerHolder.java b/app/src/main/java/org/thoughtcrime/securesms/giph/mp4/GiphyMp4ProjectionPlayerHolder.java index 9b554ece7..9638c5d12 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/giph/mp4/GiphyMp4ProjectionPlayerHolder.java +++ b/app/src/main/java/org/thoughtcrime/securesms/giph/mp4/GiphyMp4ProjectionPlayerHolder.java @@ -54,7 +54,7 @@ public final class GiphyMp4ProjectionPlayerHolder implements Player.Listener, De this.policyEnforcer = policyEnforcer; if (player.getExoPlayer() == null) { - SimpleExoPlayer fromPool = ApplicationDependencies.getExoPlayerPool().get(); + SimpleExoPlayer fromPool = ApplicationDependencies.getExoPlayerPool().get(TAG); if (fromPool == null) { Log.i(TAG, "Could not get exoplayer from pool."); @@ -142,7 +142,7 @@ public final class GiphyMp4ProjectionPlayerHolder implements Player.Listener, De @Override public void onResume(@NonNull LifecycleOwner owner) { if (mediaItem != null) { - SimpleExoPlayer fromPool = ApplicationDependencies.getExoPlayerPool().get(); + SimpleExoPlayer fromPool = ApplicationDependencies.getExoPlayerPool().get(TAG); if (fromPool != null) { ExoPlayerKt.configureForGifPlayback(fromPool); fromPool.addListener(this); diff --git a/app/src/main/java/org/thoughtcrime/securesms/logsubmit/LogSectionExoPlayerPool.kt b/app/src/main/java/org/thoughtcrime/securesms/logsubmit/LogSectionExoPlayerPool.kt new file mode 100644 index 000000000..a6a4f953b --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/logsubmit/LogSectionExoPlayerPool.kt @@ -0,0 +1,39 @@ +package org.thoughtcrime.securesms.logsubmit + +import android.content.Context +import org.thoughtcrime.securesms.dependencies.ApplicationDependencies +import org.thoughtcrime.securesms.video.exo.ExoPlayerPool + +/** + * Prints off the current exoplayer pool stats, including ownership info. + */ +class LogSectionExoPlayerPool : LogSection { + override fun getTitle(): String = "EXOPLAYER POOL" + + override fun getContent(context: Context): CharSequence { + val poolStats = ApplicationDependencies.getExoPlayerPool().getPoolStats() + val owners: Map> = poolStats.owners.groupBy { it.tag } + val output = StringBuilder() + + output.append("Total players created: ${poolStats.created}\n") + output.append("Max allowed unreserved instances: ${poolStats.maxUnreserved}\n") + output.append("Max allowed reserved instances: ${poolStats.maxReserved}\n") + output.append("Available created unreserved instances: ${poolStats.unreservedAndAvailable}\n") + output.append("Available created reserved instances: ${poolStats.reservedAndAvailable}\n") + output.append("Total unreserved created: ${poolStats.unreserved}\n") + output.append("Total reserved created: ${poolStats.reserved}\n\n") + + output.append("Ownership Info:\n") + if (owners.isEmpty()) { + output.append(" No ownership info to display.") + } else { + owners.forEach { (ownerTag, infoList) -> + output.append(" Owner $ownerTag\n") + output.append(" reserved: ${infoList.filter { it.isReserved }.size}\n") + output.append(" unreserved: ${infoList.filterNot { it.isReserved }.size}\n") + } + } + + return output + } +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/logsubmit/SubmitDebugLogRepository.java b/app/src/main/java/org/thoughtcrime/securesms/logsubmit/SubmitDebugLogRepository.java index d98ddaeff..832fa279b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/logsubmit/SubmitDebugLogRepository.java +++ b/app/src/main/java/org/thoughtcrime/securesms/logsubmit/SubmitDebugLogRepository.java @@ -78,6 +78,7 @@ public class SubmitDebugLogRepository { } add(new LogSectionNotifications()); add(new LogSectionNotificationProfiles()); + add(new LogSectionExoPlayerPool()); add(new LogSectionKeyPreferences()); add(new LogSectionBadges()); add(new LogSectionPermissions()); diff --git a/app/src/main/java/org/thoughtcrime/securesms/mediapreview/VideoMediaPreviewFragment.java b/app/src/main/java/org/thoughtcrime/securesms/mediapreview/VideoMediaPreviewFragment.java index fce8d8b25..ef06e5738 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mediapreview/VideoMediaPreviewFragment.java +++ b/app/src/main/java/org/thoughtcrime/securesms/mediapreview/VideoMediaPreviewFragment.java @@ -49,7 +49,7 @@ public final class VideoMediaPreviewFragment extends MediaPreviewFragment { videoView = itemView.findViewById(R.id.video_player); videoView.setWindow(requireActivity().getWindow()); - videoView.setVideoSource(new VideoSlide(getContext(), uri, size, false), autoPlay); + videoView.setVideoSource(new VideoSlide(getContext(), uri, size, false), autoPlay, TAG); videoView.setPlayerPositionDiscontinuityCallback((v, r) -> { if (events.getVideoControlsDelegate() != null) { events.getVideoControlsDelegate().onPlayerPositionDiscontinuity(r); diff --git a/app/src/main/java/org/thoughtcrime/securesms/mediasend/VideoEditorFragment.java b/app/src/main/java/org/thoughtcrime/securesms/mediasend/VideoEditorFragment.java index 1844627c3..91072ce79 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mediasend/VideoEditorFragment.java +++ b/app/src/main/java/org/thoughtcrime/securesms/mediasend/VideoEditorFragment.java @@ -98,7 +98,7 @@ public class VideoEditorFragment extends Fragment implements VideoEditorHud.Even boolean autoplay = isVideoGif; player.setWindow(requireActivity().getWindow()); - player.setVideoSource(slide, autoplay); + player.setVideoSource(slide, autoplay, TAG); if (slide.isVideoGif()) { player.setPlayerCallback(new VideoPlayer.PlayerCallback() { diff --git a/app/src/main/java/org/thoughtcrime/securesms/revealable/ViewOnceMessageActivity.java b/app/src/main/java/org/thoughtcrime/securesms/revealable/ViewOnceMessageActivity.java index 441253cac..6de937205 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/revealable/ViewOnceMessageActivity.java +++ b/app/src/main/java/org/thoughtcrime/securesms/revealable/ViewOnceMessageActivity.java @@ -131,7 +131,7 @@ public class ViewOnceMessageActivity extends PassphraseRequiredActivity implemen video.setWindow(getWindow()); video.setPlayerStateCallbacks(this); - video.setVideoSource(videoSlide, true); + video.setVideoSource(videoSlide, true, TAG); video.hideControls(); video.loopForever(); diff --git a/app/src/main/java/org/thoughtcrime/securesms/video/VideoPlayer.java b/app/src/main/java/org/thoughtcrime/securesms/video/VideoPlayer.java index f669757ca..9a1ebbe72 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/video/VideoPlayer.java +++ b/app/src/main/java/org/thoughtcrime/securesms/video/VideoPlayer.java @@ -121,9 +121,9 @@ public class VideoPlayer extends FrameLayout { private MediaItem mediaItem; - public void setVideoSource(@NonNull VideoSlide videoSource, boolean autoplay) { + public void setVideoSource(@NonNull VideoSlide videoSource, boolean autoplay, String poolTag) { if (exoPlayer == null) { - exoPlayer = ApplicationDependencies.getExoPlayerPool().require(); + exoPlayer = ApplicationDependencies.getExoPlayerPool().require(poolTag); exoPlayer.addListener(exoPlayerListener); exoPlayer.addListener(playerListener); exoView.setPlayer(exoPlayer); diff --git a/app/src/main/java/org/thoughtcrime/securesms/video/exo/SimpleExoPlayerPool.kt b/app/src/main/java/org/thoughtcrime/securesms/video/exo/SimpleExoPlayerPool.kt index b04bbe1cf..f2b7b3511 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/video/exo/SimpleExoPlayerPool.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/video/exo/SimpleExoPlayerPool.kt @@ -10,6 +10,7 @@ import com.google.android.exoplayer2.source.MediaSourceFactory import com.google.android.exoplayer2.source.ProgressiveMediaSource import com.google.android.exoplayer2.upstream.DataSource import com.google.android.exoplayer2.util.MimeTypes +import org.signal.core.util.logging.Log import org.thoughtcrime.securesms.dependencies.ApplicationDependencies import org.thoughtcrime.securesms.net.ContentProxySelector import org.thoughtcrime.securesms.util.AppForegroundObserver @@ -81,6 +82,10 @@ abstract class ExoPlayerPool( private val maximumReservedPlayers: Int, ) : AppForegroundObserver.Listener { + companion object { + private val TAG = Log.tag(ExoPlayerPool::class.java) + } + private val pool: MutableMap = mutableMapOf() /** @@ -89,8 +94,8 @@ abstract class ExoPlayerPool( * @return A player if one is available, otherwise null */ @MainThread - fun get(): T? { - return get(allowReserved = false) + fun get(tag: String): T? { + return get(allowReserved = false, tag = tag) } /** @@ -100,8 +105,8 @@ abstract class ExoPlayerPool( * @throws IllegalStateException if no player is available. */ @MainThread - fun require(): T { - return checkNotNull(get(allowReserved = true)) { "Required exoPlayer could not be acquired! :: ${poolStats()}" } + fun require(tag: String): T { + return checkNotNull(get(allowReserved = true, tag = tag)) { "Required exoPlayer could not be acquired for $tag! :: ${poolStats()}" } } /** @@ -113,25 +118,26 @@ abstract class ExoPlayerPool( fun pool(exoPlayer: T) { val poolState = pool[exoPlayer] if (poolState != null) { - pool[exoPlayer] = poolState.copy(available = true) + pool[exoPlayer] = poolState.copy(available = true, tag = null) } else { throw IllegalArgumentException("Tried to return unknown ExoPlayer to pool :: ${poolStats()}") } } @MainThread - private fun get(allowReserved: Boolean): T? { + private fun get(allowReserved: Boolean, tag: String): T? { val player = findAvailablePlayer(allowReserved) return if (player == null && pool.size < getMaximumAllowed(allowReserved)) { val newPlayer = createPlayer() - val poolState = createPoolStateForNewEntry(allowReserved) + val poolState = createPoolStateForNewEntry(allowReserved, tag) pool[newPlayer] = poolState newPlayer } else if (player != null) { - val poolState = pool[player]!!.copy(available = false) + val poolState = pool[player]!!.copy(available = false, tag = tag) pool[player] = poolState player } else { + Log.d(TAG, "Failed to get an ExoPlayer instance for tag: $tag") null } } @@ -140,11 +146,11 @@ abstract class ExoPlayerPool( return if (allowReserved) getMaxSimultaneousPlayback() else getMaxSimultaneousPlayback() - maximumReservedPlayers } - private fun createPoolStateForNewEntry(allowReserved: Boolean): PoolState { + private fun createPoolStateForNewEntry(allowReserved: Boolean, tag: String?): PoolState { return if (allowReserved && pool.none { (_, v) -> v.reserved }) { - PoolState(available = false, reserved = true) + PoolState(available = false, reserved = true, tag = tag) } else { - PoolState(available = false, reserved = false) + PoolState(available = false, reserved = false, tag = tag) } } @@ -175,39 +181,51 @@ abstract class ExoPlayerPool( } private fun poolStats(): String { + return getPoolStats().toString() + } + + fun getPoolStats(): PoolStats { val poolStats = PoolStats( created = pool.size, maxUnreserved = getMaxSimultaneousPlayback() - maximumReservedPlayers, - maxReserved = maximumReservedPlayers + maxReserved = maximumReservedPlayers, + owners = emptyList() ) - pool.values.fold(poolStats) { acc, state -> + return pool.values.fold(poolStats) { acc, state -> + Log.d(TAG, "$state") acc.copy( unreservedAndAvailable = acc.unreservedAndAvailable + if (state.unreservedAndAvailable) 1 else 0, reservedAndAvailable = acc.reservedAndAvailable + if (state.reservedAndAvailable) 1 else 0, unreserved = acc.unreserved + if (!state.reserved) 1 else 0, - reserved = acc.reserved + if (state.reserved) 1 else 0 + reserved = acc.reserved + if (state.reserved) 1 else 0, + owners = if (!state.available) acc.owners + OwnershipInfo(state.tag!!, state.reserved) else acc.owners ) } - - return poolStats.toString() } protected abstract fun getMaxSimultaneousPlayback(): Int - private data class PoolStats( + data class PoolStats( val created: Int = 0, val maxUnreserved: Int = 0, val maxReserved: Int = 0, val unreservedAndAvailable: Int = 0, val reservedAndAvailable: Int = 0, val unreserved: Int = 0, - val reserved: Int = 0 + val reserved: Int = 0, + val owners: List + ) + + data class OwnershipInfo( + val tag: String, + val isReserved: Boolean ) private data class PoolState( val available: Boolean, - val reserved: Boolean + val reserved: Boolean, + val tag: String? ) { val unreservedAndAvailable = available && !reserved val reservedAndAvailable = available && reserved diff --git a/app/src/test/java/org/thoughtcrime/securesms/video/exo/ExoPlayerPoolTest.kt b/app/src/test/java/org/thoughtcrime/securesms/video/exo/ExoPlayerPoolTest.kt index 52627a366..09103dad3 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/video/exo/ExoPlayerPoolTest.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/video/exo/ExoPlayerPoolTest.kt @@ -18,7 +18,7 @@ class ExoPlayerPoolTest { val testSubject = createTestSubject(1, 1) // WHEN - val player = testSubject.require() + val player = testSubject.require("") // THEN assertNotNull(player) @@ -30,7 +30,7 @@ class ExoPlayerPoolTest { val testSubject = createTestSubject(1, 0) // WHEN - testSubject.require() + testSubject.require("") // THEN fail("Expected an IllegalStateException") @@ -42,8 +42,8 @@ class ExoPlayerPoolTest { val testSubject = createTestSubject(0, 10) // WHEN - val players = (1..10).map { testSubject.get() } - val nulls = (1..10).map { testSubject.get() } + val players = (1..10).map { testSubject.get("") } + val nulls = (1..10).map { testSubject.get("") } // THEN assertTrue(players.all { it != null }) @@ -54,11 +54,11 @@ class ExoPlayerPoolTest { fun `Given a pool that allows 10 items and has all items checked out, when I return then check them all out again, then I expect 10 non null players`() { // GIVEN val testSubject = createTestSubject(0, 10) - val players = (1..10).map { testSubject.get() } + val players = (1..10).map { testSubject.get("") } // WHEN players.filterNotNull().forEach { testSubject.pool(it) } - val morePlayers = (1..10).map { testSubject.get() } + val morePlayers = (1..10).map { testSubject.get("") } assertTrue(morePlayers.all { it != null }) }