Is there a need to use atomics or other synchronization

Question

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.

Answer 1

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.

Answer 2

It depends on if this method soundPool.load() is a blocking or a non-blocking call.

val soundId = soundPool.load(assetManager.openFd("sounds/" + it.fileName), 0)

if this is a blocking call, then you don't need the atomic integer else you'll need the atomic integer.

I looked up the source for SoundPool and seems it has a native (C/C++) implementation, probably a blocking call. So you probably do not need the atomic integer. - make sure to verify this first.

You could verify this by printing the sound file name in the setOnLoadCompleteListener, and checking if it prints in order as it was in the sounds list when loaded from assetmanager.

Answer 3

You don't need an AtomicInteger here. The variable will be incremented on load no matter what and since the only operation being performed is an increment, the whole point of AtomicInteger goes in flames, doesn't it? You'll use that stuff when you need to observe a particular variable for precise measurements regarding its increment-decrement, when a mainstream process inherently depends on its value.

In your case, all you have to do is log/display how many files have been successfully loaded. It does not have an impact on anything whatsoever. Regardless, I don't see any reason a standard int would not update correctly in a scenario, but again, AtomicInteger doesn't cost you anything, so embrace your paranoia all you want.

As far as the observing part is concerned, one of the answers encourage you to use MutableStateFlow. I discourage that, since the simplest approach is almost always the best one.

I am assuming, from a call to init, that the logic you provided about the loading assets part is placed conveniently inside your viewModel. Now, just create a Mutablestate<T> object inside the model. Then, update it like so

var progress by mutableStateOf(0f)

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--.toFloat()
            progress = left / sounds.size
            if (left == 0) {
                sounds = sounds // WHAT DOES THIS EVEN MEAN?
            }
        } 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
    }
}

Then, observe this inside a Composable Scope. I don't see why this won't work.

{
  Text("${viewModel.progress}")
}