From 3b75f2209e3d5bcc7805004f551b41bccf64fffc Mon Sep 17 00:00:00 2001 From: SomeTroglodyte <63000004+SomeTroglodyte@users.noreply.github.com> Date: Wed, 30 Jun 2021 16:55:11 +0200 Subject: [PATCH] Fix mod sounds from local, Android SoundPool issues, more commenting (#4310) --- core/src/com/unciv/UncivGame.kt | 8 +- core/src/com/unciv/ui/utils/Sounds.kt | 124 ++++++++++++++++++++++---- 2 files changed, 108 insertions(+), 24 deletions(-) diff --git a/core/src/com/unciv/UncivGame.kt b/core/src/com/unciv/UncivGame.kt index e75bc09f75..b9a97bb8af 100644 --- a/core/src/com/unciv/UncivGame.kt +++ b/core/src/com/unciv/UncivGame.kt @@ -15,10 +15,7 @@ import com.unciv.models.ruleset.RulesetCache import com.unciv.models.tilesets.TileSetCache import com.unciv.models.translations.Translations import com.unciv.ui.LanguagePickerScreen -import com.unciv.ui.utils.CameraStageBaseScreen -import com.unciv.ui.utils.CrashController -import com.unciv.ui.utils.ImageGetter -import com.unciv.ui.utils.center +import com.unciv.ui.utils.* import com.unciv.ui.worldscreen.PlayerReadyScreen import com.unciv.ui.worldscreen.WorldScreen import java.util.* @@ -180,8 +177,9 @@ class UncivGame(parameters: UncivGameParameters) : Game() { override fun dispose() { cancelDiscordEvent?.invoke() + Sounds.clearCache() - // Log still running threads (should be only this one and "DestroyJavaVM") + // Log still running threads (on desktop that should be only this one and "DestroyJavaVM") val numThreads = Thread.activeCount() val threadList = Array(numThreads) { _ -> Thread() } Thread.enumerate(threadList) diff --git a/core/src/com/unciv/ui/utils/Sounds.kt b/core/src/com/unciv/ui/utils/Sounds.kt index af337b9b61..80b7608756 100644 --- a/core/src/com/unciv/ui/utils/Sounds.kt +++ b/core/src/com/unciv/ui/utils/Sounds.kt @@ -1,22 +1,51 @@ package com.unciv.ui.utils +import com.badlogic.gdx.Application import com.badlogic.gdx.Gdx import com.badlogic.gdx.audio.Sound import com.badlogic.gdx.files.FileHandle import com.unciv.UncivGame import com.unciv.models.UncivSound import java.io.File +import kotlin.concurrent.thread + +/* + * Problems on Android + * + * Essentially the freshly created Gdx Sound object from newSound() is not immediately usable, it + * needs some preparation time - buffering, decoding, whatever. Calling play() immediately will result + * in no sound, a logcat warning (not ready), and nothing else - specifically no exceptions. Also, + * keeping a Sound object for longer than necessary to play it once will trigger CloseGuard warnings + * (resource failed to clean up). Also, Gdx will attempt fast track, which will cause logcat entries, + * and these will be warnings if the sound file's sample rate (not bitrate) does not match the device's + * hardware preferred bitrate. On a Xiaomi Mi8 that is 48kHz, not 44.1kHz. Channel count also must match. + * + * @see "https://github.com/libgdx/libgdx/issues/1775" + * logcat entry "W/System: A resource failed to call end.": + * unavoidable as long as we cache Gdx Sound objects loaded from assets + * logcat entry "W/SoundPool: sample X not READY": + * could be avoided by preloading the 'cache' or otherwise ensuring a minimum delay between + * newSound() and play() - there's no test function that does not trigger logcat warnings. + * + * Current approach: Cache on demand as before, catch stream not ready and retry. This maximizes + * logcat messages but user experience is acceptable. Empiric delay needed was measured a 40ms + * so that is the minimum wait before attempting play when we know the sound is freshly cached + * and the system is Android. + */ /** * Generates Gdx [Sound] objects from [UncivSound] ones on demand, only once per key * (two UncivSound custom instances with the same filename are considered equal). - * + * * Gdx asks Sound usage to respect the Disposable contract, but since we're only caching * a handful of them in memory we should be able to get away with keeping them alive for the - * app lifetime. + * app lifetime - and we do dispose them when the app is disposed. */ object Sounds { - private enum class SupportedExtensions { mp3, ogg, wav } // Gdx won't do aac/m4a + private const val debugMessages = true + + @Suppress("EnumEntryName") + private enum class SupportedExtensions { mp3, ogg, wav } // Per Gdx docs, no aac/m4a private val soundMap = HashMap() @@ -29,16 +58,25 @@ object Sounds { if (!UncivGame.isCurrentInitialized()) return val game = UncivGame.Current - // Get a hash covering all mods - quickly, so don't map cast or copy the Set types + // Get a hash covering all mods - quickly, so don't map, cast or copy the Set types val hash1 = if (game.isGameInfoInitialized()) game.gameInfo.ruleSet.mods.hashCode() else 0 val newHash = hash1.xor(game.settings.visualMods.hashCode()) + // If hash the same, leave the cache as is if (modListHash != Int.MIN_VALUE && modListHash == newHash) return // Seems the mod list has changed - clear the cache + clearCache() + modListHash = newHash + if (debugMessages) println("Sound cache cleared") + } + + /** Release cached Sound resources */ + // Called from UncivGame.dispose() to honor Gdx docs + fun clearCache() { for (sound in soundMap.values) sound?.dispose() soundMap.clear() - modListHash = newHash + modListHash = Int.MIN_VALUE } /** Build list of folders to look for sounds */ @@ -50,40 +88,88 @@ object Sounds { val game = UncivGame.Current // Allow mod sounds - preferentially so they can override built-in sounds - // audiovisual mods first, these are already available when game.gameInfo is not - val modList: MutableSet = game.settings.visualMods + // audiovisual mods after game mods but before built-in sounds + // (these can already be available when game.gameInfo is not) + val modList: MutableSet = mutableSetOf() if (game.isGameInfoInitialized()) modList.addAll(game.gameInfo.ruleSet.mods) // Sounds from game mods + modList.addAll(game.settings.visualMods) + // Translate the basic mod list into relative folder names so only sounds/name.ext needs + // to be added. Thus the empty string, added last, represents the builtin sounds folder. return modList.asSequence() .map { "mods$separator$it$separator" } + - sequenceOf("") // represents builtin sounds folder + sequenceOf("") } - fun get(sound: UncivSound): Sound? { + /** Holds a Gdx Sound and a flag indicating the sound is freshly loaded and not from cache */ + private data class GetSoundResult(val resource: Sound, val isFresh: Boolean) + + /** Retrieve (if not cached create from resources) a Gdx Sound from an UncivSound + * @param sound The sound to fetch + * @return `null` if file cannot be found, a [GetSoundResult] otherwise + */ + private fun get(sound: UncivSound): GetSoundResult? { checkCache() - if (sound in soundMap) return soundMap[sound] + // Look for cached sound + if (sound in soundMap) + return if(soundMap[sound] == null) null + else GetSoundResult(soundMap[sound]!!, false) + + // Not cached - try loading it val fileName = sound.value var file: FileHandle? = null - for ( (modFolder, extension) in getFolders().flatMap { + for ( (modFolder, extension) in getFolders().flatMap { + // This is essentially a cross join. To operate on all combinations, we pack both lambda + // parameters into a Pair (using `to`) and unwrap that in the loop using automatic data + // class deconstruction `(,)`. All this avoids a double break when a match is found. folder -> SupportedExtensions.values().asSequence().map { folder to it } } ) { val path = "${modFolder}sounds$separator$fileName.${extension.name}" + file = Gdx.files.local(path) + if (file.exists()) break file = Gdx.files.internal(path) if (file.exists()) break } - val newSound = - if (file == null || !file.exists()) null - else Gdx.audio.newSound(file) - // Store Sound for reuse or remember that the actual file is missing - soundMap[sound] = newSound - return newSound + @Suppress("LiftReturnOrAssignment") + if (file == null || !file.exists()) { + if (debugMessages) println("Sound ${sound.value} not found!") + // remember that the actual file is missing + soundMap[sound] = null + return null + } else { + if (debugMessages) println("Sound ${sound.value} loaded from ${file.path()}") + val newSound = Gdx.audio.newSound(file) + // Store Sound for reuse + soundMap[sound] = newSound + return GetSoundResult(newSound, true) + } } + /** Find, cache and play a Sound + * + * Sources are mods from a loaded game, then mods marked as permanent audiovisual, + * and lastly Unciv's own assets/sounds. Will fail silently if the sound file cannot be found. + * + * This will wait for the Stream to become ready (Android issue) if necessary, and do so on a + * separate thread. No new thread is created if the sound can be played immediately. + * + * @param sound The sound to play + */ fun play(sound: UncivSound) { val volume = UncivGame.Current.settings.soundEffectsVolume if (sound == UncivSound.Silent || volume < 0.01) return - get(sound)?.play(volume) + val (resource, isFresh) = get(sound) ?: return + val initialDelay = if (isFresh && Gdx.app.type == Application.ApplicationType.Android) 40 else 0 + + if (initialDelay > 0 || resource.play(volume) == -1L) { + thread (name = "DelayedSound") { + Thread.sleep(initialDelay.toLong()) + while (resource.play(volume) == -1L) { + Thread.sleep(20L) + } + } + } } -} \ No newline at end of file +}