Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,14 @@ import org.siloserver.silo.common.settings.OverlayPrefsStore
import org.siloserver.silo.common.settings.PlayerSettingsStore
import org.siloserver.silo.common.settings.ServerDrivenConfigRefresher
import org.siloserver.silo.common.settings.SeekIntervalStore
import org.siloserver.silo.common.settings.SettingsContractRevision
import org.siloserver.silo.common.settings.ServerSettingsFlusher
import org.siloserver.silo.domain.player.IntroAutoSkipController
import org.siloserver.silo.domain.settings.SeekIntervalController
import org.siloserver.silo.network.DeviceMetadataProvider
import org.siloserver.silo.network.ServerRegistry
import org.siloserver.silo.network.TokenManager
import org.siloserver.silo.network.api.SettingsApi
import org.siloserver.silo.repository.LibraryPlaybackPrefsRepository
import org.siloserver.silo.repository.ProfileRepository
import org.siloserver.silo.repository.SettingsRepository
Expand Down Expand Up @@ -89,6 +91,16 @@ val playerInfraModule = module {
}
}

// The connected server's settings manifest revision. One instance, so the
// flusher's send-time gates and the settings UI agree on what it supports.
single<SettingsContractRevision> {
val settingsApi = get<SettingsApi>()
SettingsContractRevision(
fetchCapabilities = { settingsApi.getContractCapabilities() },
getServerUrl = { get<TokenManager>().getServerUrl() },
)
}

// Long-lived application-scope flusher: debounced server writes survive
// ViewModel teardown. Uses Dispatchers.IO since flushOne does network work.
single<ServerSettingsFlusher> {
Expand All @@ -100,6 +112,7 @@ val playerInfraModule = module {
// against is still the one requests would reach.
getServerUrl = { get<TokenManager>().getServerUrl() },
getAuthScope = { get<TokenManager>().snapshotCurrentScope() },
contractRevision = get(),
)
}

Expand Down Expand Up @@ -132,6 +145,7 @@ val playerInfraModule = module {
// server so settings scope stays in lockstep with what the
// server records as `device_id` for each override.
getDeviceId = { get<DeviceMetadataProvider>().current()?.id },
contractRevision = get(),
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,8 @@ class SubtitleManager(
}

private fun buildCaptionStyle(appearance: SubtitleAppearance): CaptionStyleCompat {
val foreground = parseHexColor(appearance.fontColor)
val foregroundAlpha = appearance.textOpacity.coerceIn(1, 100) * 255 / 100
val foreground = parseHexColor(appearance.fontColor, foregroundAlpha)
val backgroundAlpha = if (appearance.backgroundStyle == SubtitleBackgroundStylePreset.Box) {
(appearance.backgroundOpacity.coerceIn(0, 100) * 255 / 100)
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ class AndroidPlayerSettingsStore(
private val getDeviceId: suspend () -> String? = { null },
private val serverChangeSignal: Flow<Unit> = flowOf(Unit),
private val getAuthScope: suspend () -> org.siloserver.silo.network.AuthScopeSnapshot? = { null },
/** Null in tests that never reach a server; text opacity then counts as supported. */
private val contractRevision: SettingsContractRevision? = null,
private val dataStoreFactory: (profileId: String) -> DataStore<Preferences> = { profileId ->
PreferenceDataStoreFactory.create(
produceFile = { context.preferencesDataStoreFile(fileNameFor(profileId)) },
Expand Down Expand Up @@ -400,6 +402,17 @@ class AndroidPlayerSettingsStore(
if (matchDevice) deviceCaptioningAppearance(context, appearance) else appearance
}

override val subtitleTextOpacitySupportedFlow: Flow<Boolean> =
if (contractRevision == null) {
flowOf(true)
} else {
combine(currentScopeFlow, contractRevision.known) { scope, known ->
val revision = known?.takeIf { scope != null && it.serverUrl == scope.serverUrl }
?.manifestRevision
revision == null || SubtitleAppearance.supportsTextOpacity(revision)
}.distinctUntilChanged()
}

override suspend fun setSubtitleMatchesDevice(enabled: Boolean) =
writeBoolLocal(PlaybackSettingsKeys.SubtitleMatchesDevice, enabled)

Expand Down Expand Up @@ -532,10 +545,27 @@ class AndroidPlayerSettingsStore(
writeString(PlaybackSettingsKeys.OrientationMode, value)

override suspend fun setSubtitleAppearance(value: SubtitleAppearance) {
val sanitized = value.sanitized()
val json = sanitized.toJsonString()
updateSubtitleAppearance { value }
}

// Reads the current appearance from `prefs` inside the same `edit`
// transaction that writes the transformed result. DataStore serializes
// `edit` calls against each other (each transform lambda runs to
// completion holding the store's internal lock before the next one
// starts), so this is the actual fix for two callers racing on a
// separately-read "current" value — not just a smaller window.
//
// The enqueue happens inside the transaction for the same reason: the
// flusher keeps the last value enqueued per key, so enqueueing after
// `edit` returns would let two concurrent edits enqueue in the opposite
// order to the one they committed in, and the server would keep the
// older composite.
override suspend fun updateSubtitleAppearance(transform: (SubtitleAppearance) -> SubtitleAppearance) {
withScope { scope, store ->
store.edit { prefs ->
val current = prefs.projectedAppearance(scope)
val sanitized = transform(current).sanitized()
val json = sanitized.toJsonString()
prefs[stringPreferencesKey(scope.keyPrefix + PlaybackSettingsKeys.SubtitleAppearance)] = json
prefs[stringPreferencesKey(scope.keyPrefix + SAVED_CUSTOM_SUBTITLE_APPEARANCE)] = json
// The granular slots are rewritten from the composite rather
Expand All @@ -545,8 +575,8 @@ class AndroidPlayerSettingsStore(
// Setting an explicit appearance implicitly enables the
// device override (matches iOS `setSubtitleAppearance`).
prefs[booleanPreferencesKey(scope.keyPrefix + PlaybackSettingsKeys.SubtitleUsesDeviceOverride)] = true
serverSettingsFlusher.enqueue(scope.profileId, PlaybackSettingsKeys.SubtitleAppearance, json, scope.serverUrl, scope.authority)
}
serverSettingsFlusher.enqueue(scope.profileId, PlaybackSettingsKeys.SubtitleAppearance, json, scope.serverUrl, scope.authority)
}
}

Expand All @@ -561,15 +591,16 @@ class AndroidPlayerSettingsStore(
*/
override suspend fun flushProjectedSubtitleAppearance() {
withScope { scope, store ->
val snapshot = store.data.first()
val projected = snapshot.projectedAppearance(scope)
val json = projected.toJsonString()
if (snapshot.stringFor(scope, PlaybackSettingsKeys.SubtitleAppearance, "") == json) return@withScope
// Projected and enqueued inside the transaction, like
// [updateSubtitleAppearance], so a concurrent edit cannot commit
// between the read and the write or enqueue out of commit order.
store.edit { prefs ->
val json = prefs.projectedAppearance(scope).toJsonString()
if (prefs.stringFor(scope, PlaybackSettingsKeys.SubtitleAppearance, "") == json) return@edit
prefs[stringPreferencesKey(scope.keyPrefix + PlaybackSettingsKeys.SubtitleAppearance)] = json
prefs[stringPreferencesKey(scope.keyPrefix + SAVED_CUSTOM_SUBTITLE_APPEARANCE)] = json
serverSettingsFlusher.enqueue(scope.profileId, PlaybackSettingsKeys.SubtitleAppearance, json, scope.serverUrl, scope.authority)
}
serverSettingsFlusher.enqueue(scope.profileId, PlaybackSettingsKeys.SubtitleAppearance, json, scope.serverUrl, scope.authority)
}
}

Expand All @@ -594,6 +625,13 @@ class AndroidPlayerSettingsStore(
// restarting the session in place reverted the toggle every time.
// Draining first makes the pull observe the write. Offline, both
// fail and the local value stands.
//
// Re-read the server's settings revision first, so the push below
// gates revision-dependent fields on a current answer and a server
// upgraded while the app ran is noticed.
contractRevision?.let { revision ->
getServerUrl()?.takeIf { it.isNotBlank() }?.let { url -> runCatching { revision.refresh(url) } }
}
runCatching { serverSettingsFlusher.flushNow() }
withScope { scope, store ->
// Batched canonical resolution: one request answers every
Expand Down Expand Up @@ -638,8 +676,9 @@ class AndroidPlayerSettingsStore(
// otherwise the fields left by whatever resolved while the
// override was off win right back over it.
writeGranularAppearance(it, scope, sanitized)
// In the transaction, so it enqueues in commit order.
serverSettingsFlusher.enqueue(scope.profileId, PlaybackSettingsKeys.SubtitleAppearance, json, scope.serverUrl, scope.authority)
}
serverSettingsFlusher.enqueue(scope.profileId, PlaybackSettingsKeys.SubtitleAppearance, json, scope.serverUrl, scope.authority)
serverSettingsFlusher.flushNow()
} else {
store.edit {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ fun deviceCaptioningAppearance(context: Context, base: SubtitleAppearance): Subt
val edgeType = style.takeIf { it.hasEdgeType() }?.edgeType
val edgeColor = style.takeIf { it.hasEdgeColor() }?.edgeColor

val foregroundAlpha = fontColor?.let { AndroidColor.alpha(it) }
val backgroundAlpha = backgroundColor?.let { AndroidColor.alpha(it) }
val backgroundStyle = when {
backgroundAlpha == null -> SubtitleBackgroundStylePreset.Box
Expand All @@ -34,6 +35,7 @@ fun deviceCaptioningAppearance(context: Context, base: SubtitleAppearance): Subt
fontSize = fontSizePresetFor(manager.fontScale),
fontFamily = base.fontFamily,
fontColor = fontColor?.let(::rgbHex) ?: base.fontColor,
textOpacity = foregroundAlpha?.let { (it * 100 / 255).coerceIn(1, 100) } ?: base.textOpacity,
backgroundColor = backgroundColor?.let(::rgbHex) ?: base.backgroundColor,
backgroundStyle = backgroundStyle,
backgroundOpacity = backgroundAlpha?.let { (it * 100) / 255 } ?: base.backgroundOpacity,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,15 @@ interface PlayerSettingsStore {
/** [subtitleAppearanceFlow] with the match-device override applied. */
val effectiveSubtitleAppearanceFlow: Flow<org.siloserver.silo.model.settings.SubtitleAppearance>

/**
* False only when the active server is known to run a settings manifest
* older than [SubtitleAppearance.TEXT_OPACITY_MIN_MANIFEST_REVISION], so it
* would discard [SubtitleAppearance.textOpacity]. An unknown revision
* counts as supported: the flusher holds the value until it is known.
*/
val subtitleTextOpacitySupportedFlow: Flow<Boolean>
get() = flowOf(true)

// Setters
suspend fun setIntroSkipMode(value: IntroSkipMode)

Expand Down Expand Up @@ -179,6 +188,23 @@ interface PlayerSettingsStore {

suspend fun setSubtitleAppearance(value: SubtitleAppearance)

/**
* Applies [transform] atomically against the current stored appearance,
* read inside the same DataStore transaction that writes the result.
* Unlike a caller reading [subtitleAppearanceFlow] and then calling
* [setSubtitleAppearance] separately, no write from another caller can
* land in the gap between the read and the write — two edits committing
* around the same time (e.g. two fields as a sheet dismisses) each see
* the other's result instead of racing on a shared pre-transaction read.
*
* The default falls back to the non-atomic read-then-write for fakes
* that only need to capture the resulting value; [AndroidPlayerSettingsStore]
* overrides this with the real atomic transaction.
*/
suspend fun updateSubtitleAppearance(transform: (SubtitleAppearance) -> SubtitleAppearance) {
setSubtitleAppearance(transform(subtitleAppearanceFlow.first()))
}

/**
* Project the granular, client-local `subtitle.*` fields into the
* composite `playback.subtitle_appearance` and enqueue it.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import org.siloserver.silo.common.diagnostics.SiloLog
import org.siloserver.silo.model.diagnostics.DiagnosticsLogCategory
import org.siloserver.silo.model.settings.SettingKeys
import org.siloserver.silo.model.settings.SettingScopeIdentity
import org.siloserver.silo.model.settings.SubtitleAppearance
import org.siloserver.silo.network.AuthScopeSnapshot
import org.siloserver.silo.network.ApiResult
import org.siloserver.silo.network.api.SettingsApi
Expand Down Expand Up @@ -95,6 +96,13 @@ class DefaultServerSettingsFlusher(
*/
private val getServerUrl: suspend () -> String? = { null },
private val getAuthScope: (suspend () -> AuthScopeSnapshot?)? = null,
/**
* The connected server's settings manifest revision, for writes whose
* accepted shape depends on it. Share the app's instance so the settings
* UI and the flusher agree on what the server supports.
*/
private val contractRevision: SettingsContractRevision =
SettingsContractRevision({ settingsApi.getContractCapabilities() }, getServerUrl),
) : ServerSettingsFlusher {

private val lock = Any()
Expand Down Expand Up @@ -275,11 +283,13 @@ class DefaultServerSettingsFlusher(
SiloLog.w(CATEGORY, TAG, "dropping $key: ${op.value} does not encode as the contract type")
return false
}
val wire = gateForServerRevision(key, encoded, op.serverUrl)
?: return failed("put", key, "settings manifest revision unknown", retry = true)
return when (
val result = settingsApi.putValue(
key = key,
scope = SettingScopeIdentity.profileDevice(),
value = encoded,
value = wire,
profileId = profileId,
authority = op.authority,
)
Expand All @@ -294,6 +304,26 @@ class DefaultServerSettingsFlusher(
}
}

/**
* [encoded] in the shape the server at [serverUrl] accepts, or null when
* that depends on a manifest revision not known yet.
*
* Decided here, at send time, rather than at enqueue: a value queued
* before the revision was known has to reach a revision-14 server whole,
* because a PUT replaces the stored object and a stripped one would reset
* the textOpacity stored there. While the revision is unknown the write is
* held — kept queued and retried like a transient failure — because both
* alternatives lose data: sent whole, a server below 14 rejects it as a
* contract error, the op is dropped and the next refresh reverts every
* field in it; sent stripped, it can reset a revision-14 server's value.
*/
private suspend fun gateForServerRevision(key: String, encoded: JsonElement, serverUrl: String): JsonElement? {
if (key != SettingKeys.PLAYBACK_SUBTITLE_APPEARANCE) return encoded
if (encoded !is JsonObject || SubtitleAppearance.TEXT_OPACITY_FIELD !in encoded) return encoded
val revision = contractRevision.revisionFor(serverUrl) ?: return null
return SubtitleAppearance.wireObjectForRevision(encoded, revision)
}

private suspend fun flushDelete(profileId: String, key: String, op: PendingOp.Delete): Boolean {
return when (
val result = settingsApi.deleteValue(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package org.siloserver.silo.common.settings

import org.siloserver.silo.model.settings.SettingsContractCapabilities
import org.siloserver.silo.network.ApiResult
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock

/**
* The settings manifest revision of the connected server, read from the
* contract capabilities endpoint and cached per server URL.
*
* The settings flusher asks for it before sending a write whose shape depends
* on the revision, and the settings UI hides controls the server would
* discard. Both share one instance so they agree on what the server supports.
*/
class SettingsContractRevision(
private val fetchCapabilities: suspend () -> ApiResult<SettingsContractCapabilities>,
/** The server requests currently address; null when it cannot be read. */
private val getServerUrl: suspend () -> String? = { null },
) {
data class Known(val serverUrl: String, val manifestRevision: Int)

private val mutex = Mutex()
private val _known = MutableStateFlow<Known?>(null)

/** The last revision a probe answered, with the server that answered it. */
val known: StateFlow<Known?> = _known.asStateFlow()

/**
* The cached revision for [serverUrl], probing when none is cached.
* Null means unknown: the probe failed, or [serverUrl] is not the server
* requests would reach, so any answer would describe a different server.
*/
suspend fun revisionFor(serverUrl: String): Int? {
cachedFor(serverUrl)?.let { return it }
return mutex.withLock {
cachedFor(serverUrl) ?: probe(serverUrl)
}
}

/**
* Re-read the revision for [serverUrl] even when one is cached, so a
* server upgraded while the app runs is noticed. A failed probe keeps the
* cached value.
*/
suspend fun refresh(serverUrl: String): Int? = mutex.withLock {
probe(serverUrl) ?: cachedFor(serverUrl)
}

private fun cachedFor(serverUrl: String): Int? =
_known.value?.takeIf { it.serverUrl == serverUrl }?.manifestRevision

private suspend fun probe(serverUrl: String): Int? {
if (!isActive(serverUrl)) return null
val revision = when (val result = fetchCapabilities()) {
is ApiResult.Success -> result.data.manifestRevision
// No capabilities route: the server predates every revision gate.
is ApiResult.Error -> if (result.code == 404) 0 else null
is ApiResult.NetworkError -> null
} ?: return null
// The request goes to whichever server is active when it is sent, so a
// switch during the fetch means the answer describes another server.
if (!isActive(serverUrl)) return null
_known.value = Known(serverUrl, revision)
return revision
Comment thread
Quick104 marked this conversation as resolved.
}

private suspend fun isActive(serverUrl: String): Boolean {
val active = runCatching { getServerUrl() }.getOrNull()
return active == null || active == serverUrl
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,17 @@ class SubtitleManagerAppearanceTest {
assertEquals(0xFF000000.toInt(), style.edgeColor)
}

@Test
fun textOpacityScalesOnlyTheForegroundAlpha() {
val half = captionStyleFor(SubtitleAppearance.DEFAULT.copy(fontColor = "#ff0000", textOpacity = 50))
// 50% of 255 truncates to 0x7F; the RGB and the edge color are untouched.
assertEquals(0x7FFF0000, half.foregroundColor)
assertEquals(0xFF000000.toInt(), half.edgeColor)

val faintest = captionStyleFor(SubtitleAppearance.DEFAULT.copy(textOpacity = 1))
assertEquals(0x02FFFFFF, faintest.foregroundColor)
}

@Test
fun bottomSubtitlesUseTheReferenceSafeMargin() {
val method = SubtitleManager::class.java.getDeclaredMethod(
Expand Down
Loading
Loading