androidkotlinthread-safetyandroid-jetpack-composesoundpool

Is there a need to use atomics or other synchronization


I'm new to Compose and Android development in general. While building a toy project with Compose, I faced a question. I need to load ~100 short sounds from assets to play them via SoundPool. SoundPool.load() loads asynchronously I suppose (otherwise why do we need a callback).

Code in viewmodel:

    private val TAG = javaClass.simpleName

    private val soundPool: SoundPool
    private val soundsInProgress = AtomicInteger(0)

    var sounds: List<Sound> by mutableStateOf(listOf())
        private set


    init {
        val audioAttributes = AudioAttributes.Builder()
            .setUsage(AudioAttributes.USAGE_MEDIA)
            .setContentType(AudioAttributes.CONTENT_TYPE_SONIFICATION)
            .build()
        soundPool = SoundPool.Builder()
            .setMaxStreams(3)
            .setAudioAttributes(audioAttributes)
            .build()
        loadSoundsFromAssets()
    }

    private fun loadSoundsFromAssets() {
        val assetManager = getApplication<MyApp>().applicationContext.assets
        val sounds = assetManager.list("sounds/")!!.map(this::parseSound)
        soundsInProgress.set(sounds.size)
        soundPool.setOnLoadCompleteListener { _, id, status ->
            if (status == 0) {
                val left = soundsInProgress.decrementAndGet()
                Log.i(TAG, "Loaded 1 sound, $left in progress")
                if (left == 0) {
                    sounds = sounds
                }
            } else {
                Log.e(TAG, "Could not load sound $id, status is $status")
            }
        }
        sounds.forEach {
            val soundId = soundPool.load(assetManager.openFd("sounds/" + it.fileName), 0)
            it.soundId = soundId
        }
    }

My aim here is to track progress of sound preloading.

The question is: Is it overkill to use atomic here or is it safe without atomic?

Also, how do I observe changes to soundsInProgress ? MutableState won't work as expected.


Solution

  • SoundPool.load() does indeed load asynchronously even if it's not documented here. I tested it empirically (calls to load return before callbacks are triggered) and the documentation would certainly mention that load needs to be called off the main thread (plus as you mention, callbacks don't make sense if it were a synchronous call).

    It can be assumed that the callbacks happen one by one since they happen on the main thread (again not documented!) so theoretically you won't need an AtomicInteger but I'll circle back to that statement once I explained my suggestion for the ui.

    For a compose UI I'd use a mutableStateOf that communicates the progress from the callback to the ui.

    Something like this:

    private data class SoundProgress(val loaded: Int, val total: Int)
    private var progress by mutableStateOf(SoundProgress(0, 0))
    
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
    
        setContent {
            Text(text = "${progress.loaded}/${progress.total}")
        }
    

    In the callback you have:

    val count = AtomicInteger()
    soundPool.setOnLoadCompleteListener { _, id, status ->
        if (status == 0) {
            progress = SoundProgress(count.incrementAndGet(), sounds.size)
        } else {
            Log.e(TAG, "Could not load sound $id, status is $status")
        }
    }
    

    Notice I used an AtomicInteger for the count variable. Since all this is running on the main thread (except the actual load operation), a simple int would most likely do. In my long years of writing asynchronous code, I've encountered unexpected behavior more often than not so while I'd say that the inc operation will always happen atomically, I use the AtomicInteger regardless just to be on the safe side. Since there's no (performance/memory) penalty to pay for using it over int and since one cannot eliminate the possibility that SoundPool uses a thread pool under the hood, I'd go with the conservative approach.

    Btw try to omit !! operators. While they are convenient, it's also an anti-pattern.

    instead of:

    val sounds = assetManager.list("sounds/")!!.map(this::parseSound)
    

    use this:

    val sounds = assetManager.list("sounds")?.map(this::parseSound) ?: listOf()
    

    A little more verbose but you won't ever get an NPE with this.