image: make bitmap provider use generation system

Make BitmapProvider leverage the generation system used in Indexer.

This helps reduce the surface for instruction-by-instruction race
conditions.
This commit is contained in:
OxygenCobalt 2022-06-04 13:40:16 -06:00
parent 9597dc4dd2
commit b2ad54eef3
No known key found for this signature in database
GPG key ID: 37DBE3621FE9AD47
5 changed files with 29 additions and 9 deletions

View file

@ -2,7 +2,11 @@
## dev [v2.3.2, v2.4.0, or v3.0.0] ## dev [v2.3.2, v2.4.0, or v3.0.0]
#### What's Improved
- Genre parsing now handles multiple integer values and cover/remix indicators
#### Dev/Meta #### Dev/Meta
- New translations [Fjuro -> Czech]
- Moved music loading to a foreground service - Moved music loading to a foreground service
## v2.3.1 ## v2.3.1

View file

@ -25,6 +25,7 @@ import coil.request.Disposable
import coil.request.ImageRequest import coil.request.ImageRequest
import coil.size.Size import coil.size.Size
import org.oxycblt.auxio.music.Song import org.oxycblt.auxio.music.Song
import org.oxycblt.auxio.util.logD
/** /**
* A utility to provide bitmaps in a manner less prone to race conditions. * A utility to provide bitmaps in a manner less prone to race conditions.
@ -33,15 +34,15 @@ import org.oxycblt.auxio.music.Song
* request with some target callbacks could result in overlapping requests causing unrelated * request with some target callbacks could result in overlapping requests causing unrelated
* updates. This class (to an extent) resolves this by keeping track of the current request and * updates. This class (to an extent) resolves this by keeping track of the current request and
* disposing of it every time a new request is created. This greatly reduces the surface for race * disposing of it every time a new request is created. This greatly reduces the surface for race
* conditions save the case of instruction-by-instruction data races, which are effectively * conditions.
* impossible to solve.
* *
* @author OxygenCobalt * @author OxygenCobalt
*/ */
class BitmapProvider(private val context: Context) { class BitmapProvider(private val context: Context) {
private var currentRequest: Request? = null private var currentRequest: Request? = null
private var currentGeneration = 0L
/* If this provider is currently attempting to load something. */ /** If this provider is currently attempting to load something. */
val isBusy: Boolean val isBusy: Boolean
get() = currentRequest?.run { !disposable.isDisposed } ?: false get() = currentRequest?.run { !disposable.isDisposed } ?: false
@ -50,6 +51,10 @@ class BitmapProvider(private val context: Context) {
* callback. * callback.
*/ */
fun load(song: Song, target: Target) { fun load(song: Song, target: Target) {
// Increment the generation value so that previous requests are invalidated.
// This is a second safeguard to mitigate instruction-by-instruction race conditions.
val generation = synchronized(this) { ++currentGeneration }
currentRequest?.run { disposable.dispose() } currentRequest?.run { disposable.dispose() }
currentRequest = null currentRequest = null
@ -59,8 +64,16 @@ class BitmapProvider(private val context: Context) {
.data(song) .data(song)
.size(Size.ORIGINAL) .size(Size.ORIGINAL)
.target( .target(
onSuccess = { target.onCompleted(it.toBitmap()) }, onSuccess = {
onError = { target.onCompleted(null) }) if (generation == currentGeneration) {
target.onCompleted(it.toBitmap())
}
},
onError = {
if (generation == currentGeneration) {
target.onCompleted(null)
}
})
.transformations(SquareFrameTransform.INSTANCE)) .transformations(SquareFrameTransform.INSTANCE))
currentRequest = Request(context.imageLoader.enqueue(request.build()), target) currentRequest = Request(context.imageLoader.enqueue(request.build()), target)

View file

@ -57,7 +57,7 @@ class Indexer {
private var lastResponse: Response? = null private var lastResponse: Response? = null
private var loadingState: Loading? = null private var loadingState: Loading? = null
private var currentGeneration: Long = 0 private var currentGeneration = 0L
private val callbacks = mutableListOf<Callback>() private val callbacks = mutableListOf<Callback>()
fun addCallback(callback: Callback) { fun addCallback(callback: Callback) {

View file

@ -88,10 +88,10 @@ class IndexerService : Service(), Indexer.Callback {
override fun onIndexerStateChanged(state: Indexer.State?) { override fun onIndexerStateChanged(state: Indexer.State?) {
when (state) { when (state) {
is Indexer.State.Complete -> { is Indexer.State.Complete -> {
if (state.response is Indexer.Response.Ok && musicStore.library == null) { if (state.response is Indexer.Response.Ok &&
state.response.library != musicStore.library) {
// Load was completed successfully, so apply the new library if we // Load was completed successfully, so apply the new library if we
// have not already. // have not already.
// TODO: Change null check for equality check [automatic rescanning]
musicStore.library = state.response.library musicStore.library = state.response.library
} }

View file

@ -108,6 +108,8 @@ private fun String.parseId3v1Genre(): String? =
// CR and RX are not technically ID3v1, but are formatted similarly to a plain number. // CR and RX are not technically ID3v1, but are formatted similarly to a plain number.
this == "CR" -> "Cover" this == "CR" -> "Cover"
this == "RX" -> "Remix" this == "RX" -> "Remix"
// Current name is fine.
else -> null else -> null
} }
@ -119,7 +121,7 @@ private fun String.parseId3v2Genre(): String? {
// You can read the spec for it here: https://id3.org/id3v2.3.0#TCON // You can read the spec for it here: https://id3.org/id3v2.3.0#TCON
// This implementation in particular is based off Mutagen's genre parser. // This implementation in particular is based off Mutagen's genre parser.
// Case 1: Genre IDs in the format (DIGITS|RX|CR). If these exist, parse them as // Case 1: Genre IDs in the format (INT|RX|CR). If these exist, parse them as
// ID3v1 tags. // ID3v1 tags.
val genreIds = groups[1] val genreIds = groups[1]
if (genreIds != null && genreIds.value.isNotEmpty()) { if (genreIds != null && genreIds.value.isNotEmpty()) {
@ -143,6 +145,7 @@ private fun String.parseId3v2Genre(): String? {
return genres.distinctBy { it }.joinToString(separator = ", ").ifEmpty { null } return genres.distinctBy { it }.joinToString(separator = ", ").ifEmpty { null }
} }
/** Regex that implements matching for ID3v2's genre format. */
private val GENRE_RE = Regex("((?:\\(([0-9]+|RX|CR)\\))*)(.+)?") private val GENRE_RE = Regex("((?:\\(([0-9]+|RX|CR)\\))*)(.+)?")
/** /**