From 961f32de5c7f7cdc91e6f1a79bc4f99b309b05cb Mon Sep 17 00:00:00 2001 From: Duhan <136324426+lostf1sh@users.noreply.github.com> Date: Sun, 4 Oct 2026 14:37:23 +0300 Subject: [PATCH 1/3] Load cloud artwork without the shared provider in-process (#138) The media session exposes cloud covers as content://.artwork/cloud/... to every controller, including the app's own, so the player, notification bitmap loader, and widgets loaded them through SharedArtworkContentProvider. The provider decodes the cover with the same Coil ImageLoader while the outer decode blocks on the pipe; Coil allows four concurrent BitmapFactory decodes, so four such loads held every slot and the inner decodes never started, leaving all artwork on its placeholder until restart. Map those URIs back to navidrome_cover:// / jellyfin_cover:// before Coil fetches them. Also stop caching non-image bodies (Subsonic reports errors with HTTP 200), evict ones cached earlier, and write downloads through a temp file so concurrent requests can't truncate a file being decoded. --- CHANGELOG.md | 1 + .../pixelplayeross/PixelPlayerApplication.kt | 2 + .../data/image/JellyfinCoilFetcher.kt | 67 ++----------- .../data/image/NavidromeCoilFetcher.kt | 60 +----------- .../data/image/RemoteArtworkCache.kt | 88 +++++++++++++++++ .../data/image/SharedCloudArtworkMappers.kt | 29 ++++++ .../data/image/RemoteArtworkCacheTest.kt | 94 +++++++++++++++++++ 7 files changed, 224 insertions(+), 117 deletions(-) create mode 100644 app/src/main/java/com/lostf1sh/pixelplayeross/data/image/RemoteArtworkCache.kt create mode 100644 app/src/main/java/com/lostf1sh/pixelplayeross/data/image/SharedCloudArtworkMappers.kt create mode 100644 app/src/test/java/com/lostf1sh/pixelplayeross/data/image/RemoteArtworkCacheTest.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index 012aa7df..ddddb958 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ All notable changes to PixelPlayerOSS will be documented in this file. ## [Unreleased] ### Fixed +- Navidrome/Subsonic and Jellyfin album artwork no longer gets stuck on the placeholder after a few songs. The player loaded cloud covers through the app's own artwork content provider, and concurrent loads could exhaust the image decoder slots that provider waited on, freezing every artwork load until the app restarted. Server error responses are also no longer cached as cover images, and concurrent cover downloads no longer corrupt each other's cache file ([#138](https://github.com/PixelPlayerHQ/PixelPlayerOSS/issues/138)). - Jellyfin playlists no longer go missing on Jellyfin 10.10 and newer, where playlists can hold mixed content and audio playlists are often reported without a media type. - Pressing back with the Now Playing screen or the Next up queue open inside a folder in the Folders tab now closes the player instead of leaving the folder ([#136](https://github.com/PixelPlayerHQ/PixelPlayerOSS/issues/136)). diff --git a/app/src/main/java/com/lostf1sh/pixelplayeross/PixelPlayerApplication.kt b/app/src/main/java/com/lostf1sh/pixelplayeross/PixelPlayerApplication.kt index 5d5c8a47..3c7b6435 100644 --- a/app/src/main/java/com/lostf1sh/pixelplayeross/PixelPlayerApplication.kt +++ b/app/src/main/java/com/lostf1sh/pixelplayeross/PixelPlayerApplication.kt @@ -155,6 +155,8 @@ class PixelPlayerApplication : Application(), ImageLoaderFactory, Configuration. override fun newImageLoader(): ImageLoader { return imageLoader.get().newBuilder() .components { + add(com.lostf1sh.pixelplayeross.data.image.SharedCloudArtworkStringMapper(packageName)) + add(com.lostf1sh.pixelplayeross.data.image.SharedCloudArtworkUriMapper(packageName)) add(localArtworkCoilFetcherFactory.get()) add(navidromeCoilFetcherFactory.get()) add(jellyfinCoilFetcherFactory.get()) diff --git a/app/src/main/java/com/lostf1sh/pixelplayeross/data/image/JellyfinCoilFetcher.kt b/app/src/main/java/com/lostf1sh/pixelplayeross/data/image/JellyfinCoilFetcher.kt index 8459b8a5..3a323f08 100644 --- a/app/src/main/java/com/lostf1sh/pixelplayeross/data/image/JellyfinCoilFetcher.kt +++ b/app/src/main/java/com/lostf1sh/pixelplayeross/data/image/JellyfinCoilFetcher.kt @@ -2,20 +2,14 @@ package com.lostf1sh.pixelplayeross.data.image import android.net.Uri import coil.ImageLoader -import coil.decode.DataSource -import coil.decode.ImageSource import coil.fetch.FetchResult import coil.fetch.Fetcher -import coil.fetch.SourceResult import coil.request.Options import com.lostf1sh.pixelplayeross.data.jellyfin.JellyfinRepository import okhttp3.OkHttpClient import okhttp3.Request -import okio.FileSystem -import okio.Path.Companion.toPath import timber.log.Timber import java.io.File -import java.io.FileOutputStream import java.util.concurrent.ConcurrentHashMap import javax.inject.Inject @@ -63,17 +57,7 @@ class JellyfinCoilFetcher( val sizeParam = uri.getQueryParameter("size")?.toIntOrNull() ?: 500 val cachedFile = File(cacheDir, "jellyfin_cover_${itemId}_$sizeParam.jpg") - if (cachedFile.exists() && cachedFile.length() > 0) { - Timber.v("$TAG: Using cached cover for $itemId") - return SourceResult( - source = ImageSource( - file = cachedFile.absolutePath.toPath(), - fileSystem = FileSystem.SYSTEM - ), - mimeType = "image/jpeg", - dataSource = DataSource.DISK - ) - } + RemoteArtworkCache.cachedResult(cachedFile)?.let { return it } val imageUrl = repository.getImageUrl(itemId, sizeParam) if (imageUrl.isNullOrBlank()) { @@ -86,7 +70,12 @@ class JellyfinCoilFetcher( val authHeader = repository.getAuthorizationHeader() return try { - downloadImage(imageUrl, cachedFile, authHeader) + val request = Request.Builder() + .url(imageUrl) + .apply { if (authHeader != null) header("Authorization", authHeader) } + .get() + .build() + RemoteArtworkCache.download(okHttpClient, request, cachedFile) } catch (e: Exception) { if (shouldLogFailure("download_$itemId")) { Timber.w(e, "$TAG: Failed to download cover art for $itemId") @@ -95,48 +84,6 @@ class JellyfinCoilFetcher( } } - private fun downloadImage(url: String, cacheFile: File, authHeader: String? = null): FetchResult? { - val request = Request.Builder() - .url(url) - .apply { if (authHeader != null) header("Authorization", authHeader) } - .get() - .build() - - return try { - okHttpClient.newCall(request).execute().use { response -> - if (!response.isSuccessful) { - Timber.w("$TAG: HTTP ${response.code} for $url") - return null - } - - val bytes = response.body.bytes() - - if (bytes.isEmpty()) { - Timber.w("$TAG: Empty response body for $url") - return null - } - - FileOutputStream(cacheFile).use { fos -> - fos.write(bytes) - } - - Timber.v("$TAG: Cached cover art (${bytes.size} bytes)") - - SourceResult( - source = ImageSource( - file = cacheFile.absolutePath.toPath(), - fileSystem = FileSystem.SYSTEM - ), - mimeType = response.header("Content-Type") ?: "image/jpeg", - dataSource = DataSource.NETWORK - ) - } - } catch (e: Exception) { - Timber.w(e, "$TAG: Failed to download image from $url") - null - } - } - class Factory @Inject constructor( private val repository: JellyfinRepository, private val okHttpClient: OkHttpClient diff --git a/app/src/main/java/com/lostf1sh/pixelplayeross/data/image/NavidromeCoilFetcher.kt b/app/src/main/java/com/lostf1sh/pixelplayeross/data/image/NavidromeCoilFetcher.kt index d3e1057c..ca9a2b9c 100644 --- a/app/src/main/java/com/lostf1sh/pixelplayeross/data/image/NavidromeCoilFetcher.kt +++ b/app/src/main/java/com/lostf1sh/pixelplayeross/data/image/NavidromeCoilFetcher.kt @@ -4,16 +4,12 @@ import android.net.Uri import coil.ImageLoader import coil.fetch.FetchResult import coil.fetch.Fetcher -import coil.fetch.SourceResult import coil.request.Options import com.lostf1sh.pixelplayeross.data.navidrome.NavidromeRepository -import com.lostf1sh.pixelplayeross.data.network.navidrome.NavidromeApiService import okhttp3.OkHttpClient import okhttp3.Request -import okio.Path.Companion.toPath import timber.log.Timber import java.io.File -import java.io.FileOutputStream import java.util.concurrent.ConcurrentHashMap import javax.inject.Inject @@ -68,17 +64,7 @@ class NavidromeCoilFetcher( val sizeParam = uri.getQueryParameter("size")?.toIntOrNull() ?: 500 val cachedFile = File(cacheDir, "navidrome_cover_${coverArtId}_$sizeParam.jpg") - if (cachedFile.exists() && cachedFile.length() > 0) { - Timber.v("$TAG: Using cached cover for $coverArtId") - return SourceResult( - source = coil.decode.ImageSource( - file = cachedFile.absolutePath.toPath(), - fileSystem = okio.FileSystem.SYSTEM - ), - mimeType = "image/jpeg", - dataSource = coil.decode.DataSource.DISK - ) - } + RemoteArtworkCache.cachedResult(cachedFile)?.let { return it } val coverArtUrl = repository.getCoverArtUrl(coverArtId, sizeParam) if (coverArtUrl.isNullOrBlank()) { @@ -89,7 +75,8 @@ class NavidromeCoilFetcher( } return try { - downloadImage(coverArtUrl, cachedFile) + val request = Request.Builder().url(coverArtUrl).get().build() + RemoteArtworkCache.download(okHttpClient, request, cachedFile) } catch (e: Exception) { if (shouldLogFailure("download_$coverArtId")) { Timber.w(e, "$TAG: Failed to download cover art for $coverArtId") @@ -98,47 +85,6 @@ class NavidromeCoilFetcher( } } - private suspend fun downloadImage(url: String, cacheFile: File): FetchResult? { - val request = Request.Builder() - .url(url) - .get() - .build() - - return try { - okHttpClient.newCall(request).execute().use { response -> - if (!response.isSuccessful) { - Timber.w("$TAG: HTTP ${response.code} for $url") - return null - } - - val bytes = response.body.bytes() - - if (bytes.isEmpty()) { - Timber.w("$TAG: Empty response body for $url") - return null - } - - FileOutputStream(cacheFile).use { fos -> - fos.write(bytes) - } - - Timber.v("$TAG: Cached cover art (${bytes.size} bytes)") - - SourceResult( - source = coil.decode.ImageSource( - file = cacheFile.absolutePath.toPath(), - fileSystem = okio.FileSystem.SYSTEM - ), - mimeType = response.header("Content-Type") ?: "image/jpeg", - dataSource = coil.decode.DataSource.NETWORK - ) - } - } catch (e: Exception) { - Timber.w(e, "$TAG: Failed to download image from $url") - null - } - } - /** * Factory for creating NavidromeCoilFetcher instances. * Registered with Coil's ImageLoader to handle navidrome_cover:// URIs. diff --git a/app/src/main/java/com/lostf1sh/pixelplayeross/data/image/RemoteArtworkCache.kt b/app/src/main/java/com/lostf1sh/pixelplayeross/data/image/RemoteArtworkCache.kt new file mode 100644 index 00000000..cedb70e8 --- /dev/null +++ b/app/src/main/java/com/lostf1sh/pixelplayeross/data/image/RemoteArtworkCache.kt @@ -0,0 +1,88 @@ +package com.lostf1sh.pixelplayeross.data.image + +import coil.decode.DataSource +import coil.decode.ImageSource +import coil.fetch.SourceResult +import okhttp3.OkHttpClient +import okhttp3.Request +import okio.FileSystem +import okio.Path.Companion.toPath +import java.io.File +import java.io.IOException + +/** + * Disk cache shared by the Navidrome and Jellyfin Coil fetchers. + * + * Only bodies that carry an image signature are cached: Subsonic servers answer failed + * `getCoverArt` calls with HTTP 200 and a JSON/XML error, and caching that body would pin a + * broken cover until the cache is cleared. Downloads land in a temp file that is renamed into + * place, so a concurrent request never decodes a file another request is still writing. + */ +internal object RemoteArtworkCache { + + private const val SIGNATURE_BYTES = 12 + + fun cachedResult(file: File): SourceResult? { + if (!file.isFile || file.length() == 0L) return null + if (!hasImageSignature(readSignature(file))) { + file.delete() + return null + } + return SourceResult( + source = ImageSource(file = file.absolutePath.toPath(), fileSystem = FileSystem.SYSTEM), + mimeType = "image/jpeg", + dataSource = DataSource.DISK + ) + } + + @Throws(IOException::class) + fun download(client: OkHttpClient, request: Request, file: File): SourceResult { + client.newCall(request).execute().use { response -> + if (!response.isSuccessful) throw IOException("HTTP ${response.code}") + val bytes = response.body.bytes() + if (!hasImageSignature(bytes)) { + throw IOException("Response is not an image (Content-Type: ${response.header("Content-Type")})") + } + + val tempFile = File.createTempFile(file.name, ".tmp", file.parentFile) + try { + tempFile.writeBytes(bytes) + if (!tempFile.renameTo(file)) throw IOException("Could not move artwork into $file") + } finally { + tempFile.delete() + } + + return SourceResult( + source = ImageSource(file = file.absolutePath.toPath(), fileSystem = FileSystem.SYSTEM), + mimeType = response.header("Content-Type") ?: "image/jpeg", + dataSource = DataSource.NETWORK + ) + } + } + + internal fun hasImageSignature(header: ByteArray): Boolean { + fun matches(offset: Int, vararg expected: Int): Boolean = + header.size >= offset + expected.size && + expected.indices.all { header[offset + it].toInt() and 0xFF == expected[it] } + + return matches(0, 0xFF, 0xD8, 0xFF) || // JPEG + matches(0, 0x89, 0x50, 0x4E, 0x47) || // PNG + matches(0, 0x47, 0x49, 0x46, 0x38) || // GIF8 + (matches(0, 0x52, 0x49, 0x46, 0x46) && matches(8, 0x57, 0x45, 0x42, 0x50)) || // RIFF....WEBP + matches(4, 0x66, 0x74, 0x79, 0x70) || // ISO-BMFF ftyp (HEIF/AVIF) + matches(0, 0x42, 0x4D) // BMP + } + + private fun readSignature(file: File): ByteArray = runCatching { + file.inputStream().use { input -> + val buffer = ByteArray(SIGNATURE_BYTES) + var read = 0 + while (read < SIGNATURE_BYTES) { + val count = input.read(buffer, read, SIGNATURE_BYTES - read) + if (count < 0) break + read += count + } + buffer.copyOf(read) + } + }.getOrDefault(ByteArray(0)) +} diff --git a/app/src/main/java/com/lostf1sh/pixelplayeross/data/image/SharedCloudArtworkMappers.kt b/app/src/main/java/com/lostf1sh/pixelplayeross/data/image/SharedCloudArtworkMappers.kt new file mode 100644 index 00000000..c6e4826e --- /dev/null +++ b/app/src/main/java/com/lostf1sh/pixelplayeross/data/image/SharedCloudArtworkMappers.kt @@ -0,0 +1,29 @@ +package com.lostf1sh.pixelplayeross.data.image + +import android.net.Uri +import coil.map.Mapper +import coil.request.Options +import com.lostf1sh.pixelplayeross.data.provider.SharedArtworkContentProvider + +/* + * The media session rewrites cloud artwork to `content://.artwork/cloud/...` for every + * controller, our own included, so the player UI, notification, and widgets receive that URI. + * Loading it in-process goes through SharedArtworkContentProvider, which decodes the cover with + * this same ImageLoader while the outer decode blocks reading the pipe. Coil allows only a few + * concurrent bitmap decodes, so a handful of such loads hold every decode slot and the inner + * decodes they wait on never start: all artwork stays stuck on its placeholder until restart. + * Mapping back to the raw cloud URI lets the Navidrome/Jellyfin fetchers serve it directly. + */ + +class SharedCloudArtworkStringMapper(private val packageName: String) : Mapper { + override fun map(data: String, options: Options): Uri? = + SharedArtworkContentProvider.parseCloudArtworkUri(data, packageName)?.let(Uri::parse) +} + +class SharedCloudArtworkUriMapper(private val packageName: String) : Mapper { + override fun map(data: Uri, options: Options): Uri? { + if (data.authority != SharedArtworkContentProvider.authority(packageName)) return null + return SharedArtworkContentProvider.parseCloudArtworkUri(data.toString(), packageName) + ?.let(Uri::parse) + } +} diff --git a/app/src/test/java/com/lostf1sh/pixelplayeross/data/image/RemoteArtworkCacheTest.kt b/app/src/test/java/com/lostf1sh/pixelplayeross/data/image/RemoteArtworkCacheTest.kt new file mode 100644 index 00000000..d7c28f67 --- /dev/null +++ b/app/src/test/java/com/lostf1sh/pixelplayeross/data/image/RemoteArtworkCacheTest.kt @@ -0,0 +1,94 @@ +package com.lostf1sh.pixelplayeross.data.image + +import com.google.common.truth.Truth.assertThat +import okhttp3.MediaType.Companion.toMediaType +import okhttp3.OkHttpClient +import okhttp3.Protocol +import okhttp3.Request +import okhttp3.Response +import okhttp3.ResponseBody.Companion.toResponseBody +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows +import org.junit.jupiter.api.io.TempDir +import java.io.File +import java.io.IOException + +class RemoteArtworkCacheTest { + + @TempDir + lateinit var cacheDir: File + + private val jpegBytes = byteArrayOf(0xFF.toByte(), 0xD8.toByte(), 0xFF.toByte(), 0xE0.toByte(), 0, 0x10) + private val subsonicError = + """{"subsonic-response":{"status":"failed","error":{"code":40,"message":"Wrong username or password"}}}""" + + @Test + fun `subsonic error answered with HTTP 200 is not cached as artwork`() { + val target = File(cacheDir, "navidrome_cover_al-1_500.jpg") + + assertThrows { + RemoteArtworkCache.download(clientReturning(subsonicError.toByteArray(), "application/json"), request(), target) + } + + assertThat(target.exists()).isFalse() + assertThat(cacheDir.listFiles()).isEmpty() + } + + @Test + fun `downloaded image replaces the cached file without leaving temp files`() { + val target = File(cacheDir, "navidrome_cover_al-1_500.jpg") + target.writeBytes(byteArrayOf(0xFF.toByte(), 0xD8.toByte(), 0xFF.toByte(), 1)) + + val result = RemoteArtworkCache.download(clientReturning(jpegBytes, "image/jpeg"), request(), target) + + assertThat(target.readBytes()).isEqualTo(jpegBytes) + assertThat(cacheDir.listFiles()!!.map { it.name }).containsExactly(target.name) + assertThat(result.mimeType).isEqualTo("image/jpeg") + } + + @Test + fun `previously cached error body is evicted instead of served`() { + val poisoned = File(cacheDir, "navidrome_cover_al-1_500.jpg").apply { writeText(subsonicError) } + + assertThat(RemoteArtworkCache.cachedResult(poisoned)).isNull() + assertThat(poisoned.exists()).isFalse() + } + + @Test + fun `cached image is served from disk`() { + val cached = File(cacheDir, "jellyfin_cover_item_500.jpg").apply { writeBytes(jpegBytes) } + + assertThat(RemoteArtworkCache.cachedResult(cached)).isNotNull() + assertThat(cached.exists()).isTrue() + } + + @Test + fun `image signatures of formats servers return are recognized`() { + val png = byteArrayOf(0x89.toByte(), 0x50, 0x4E, 0x47, 0x0D, 0x0A) + val webp = "RIFF\u0000\u0000\u0000\u0000WEBP".toByteArray(Charsets.ISO_8859_1) + val avif = "\u0000\u0000\u0000\u001CftypAVIF".toByteArray(Charsets.ISO_8859_1) + + assertThat(RemoteArtworkCache.hasImageSignature(jpegBytes)).isTrue() + assertThat(RemoteArtworkCache.hasImageSignature(png)).isTrue() + assertThat(RemoteArtworkCache.hasImageSignature(webp)).isTrue() + assertThat(RemoteArtworkCache.hasImageSignature(avif)).isTrue() + assertThat(RemoteArtworkCache.hasImageSignature(" + Response.Builder() + .request(chain.request()) + .protocol(Protocol.HTTP_1_1) + .code(200) + .message("OK") + .header("Content-Type", contentType) + .body(body.toResponseBody(contentType.toMediaType())) + .build() + } + .build() +} From abf033a7be294c1e5abcbbbad0fe5dcdc5cdd45a Mon Sep 17 00:00:00 2001 From: Duhan <136324426+lostf1sh@users.noreply.github.com> Date: Sun, 4 Oct 2026 14:37:25 +0300 Subject: [PATCH 2/3] Let System UI read shared artwork URIs System UI reads the platform session's artwork URI without connecting as a Media3 controller, so it never received the per-item grant and every open was refused. Allow callers holding MEDIA_CONTENT_CONTROL, which can already read and control every media session; other apps still need the explicit grant. --- CHANGELOG.md | 1 + .../provider/SharedArtworkContentProvider.kt | 23 +++++++++++++++---- .../SharedArtworkContentProviderTest.kt | 17 ++++++++++++++ 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ddddb958..c1390a36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ All notable changes to PixelPlayerOSS will be documented in this file. ### Fixed - Navidrome/Subsonic and Jellyfin album artwork no longer gets stuck on the placeholder after a few songs. The player loaded cloud covers through the app's own artwork content provider, and concurrent loads could exhaust the image decoder slots that provider waited on, freezing every artwork load until the app restarted. Server error responses are also no longer cached as cover images, and concurrent cover downloads no longer corrupt each other's cache file ([#138](https://github.com/PixelPlayerHQ/PixelPlayerOSS/issues/138)). +- System media controls can load album artwork through the app's artwork URI again. System UI reads that URI without connecting as a media controller, so it never received the per-track read grant and was refused on every track; privileged media surfaces (holders of `MEDIA_CONTENT_CONTROL`) are now allowed, while other apps still need the explicit grant. Stock Android fell back to the session bitmap, but System UIs without that fallback could show no artwork. - Jellyfin playlists no longer go missing on Jellyfin 10.10 and newer, where playlists can hold mixed content and audio playlists are often reported without a media type. - Pressing back with the Now Playing screen or the Next up queue open inside a folder in the Folders tab now closes the player instead of leaving the folder ([#136](https://github.com/PixelPlayerHQ/PixelPlayerOSS/issues/136)). diff --git a/app/src/main/java/com/lostf1sh/pixelplayeross/data/provider/SharedArtworkContentProvider.kt b/app/src/main/java/com/lostf1sh/pixelplayeross/data/provider/SharedArtworkContentProvider.kt index 79d3651f..b938e8e8 100644 --- a/app/src/main/java/com/lostf1sh/pixelplayeross/data/provider/SharedArtworkContentProvider.kt +++ b/app/src/main/java/com/lostf1sh/pixelplayeross/data/provider/SharedArtworkContentProvider.kt @@ -1,5 +1,6 @@ package com.lostf1sh.pixelplayeross.data.provider +import android.Manifest import android.content.ContentProvider import android.content.ContentValues import android.content.Context @@ -96,22 +97,34 @@ class SharedArtworkContentProvider : ContentProvider() { /** * Some vendor System UI implementations reject URI grants before attempting to open a * non-exported provider. The provider is therefore exported for compatibility, while every - * file open still requires either our own UID or the explicit per-item grant issued by the - * media session. + * file open still requires our own UID, the explicit per-item grant issued by the media + * session, or a privileged media surface. + * + * System UI's media controls and notification read the platform session's artwork URI + * without ever connecting as a Media3 controller, so they never receive a per-item grant. + * They hold MEDIA_CONTENT_CONTROL (signature|privileged), which already lets them read and + * control every media session, so the artwork exposes nothing new to them. */ private fun enforceArtworkReadAccess(uri: Uri, appContext: Context) { val callingUid = Binder.getCallingUid() + val callingPid = Binder.getCallingPid() val permissionResult = appContext.checkUriPermission( uri, - Binder.getCallingPid(), + callingPid, callingUid, Intent.FLAG_GRANT_READ_URI_PERMISSION, ) + val mediaControlResult = appContext.checkPermission( + Manifest.permission.MEDIA_CONTENT_CONTROL, + callingPid, + callingUid, + ) if ( !hasArtworkReadAccess( callingUid = callingUid, providerUid = appContext.applicationInfo.uid, uriPermissionResult = permissionResult, + mediaContentControlResult = mediaControlResult, ) ) { // FileNotFoundException is intentionally used here: media clients commonly treat a @@ -268,9 +281,11 @@ class SharedArtworkContentProvider : ContentProvider() { callingUid: Int, providerUid: Int, uriPermissionResult: Int, + mediaContentControlResult: Int, ): Boolean { return callingUid == providerUid || - uriPermissionResult == PackageManager.PERMISSION_GRANTED + uriPermissionResult == PackageManager.PERMISSION_GRANTED || + mediaContentControlResult == PackageManager.PERMISSION_GRANTED } } } diff --git a/app/src/test/java/com/lostf1sh/pixelplayeross/data/provider/SharedArtworkContentProviderTest.kt b/app/src/test/java/com/lostf1sh/pixelplayeross/data/provider/SharedArtworkContentProviderTest.kt index 27f4c8b5..9663d9ea 100644 --- a/app/src/test/java/com/lostf1sh/pixelplayeross/data/provider/SharedArtworkContentProviderTest.kt +++ b/app/src/test/java/com/lostf1sh/pixelplayeross/data/provider/SharedArtworkContentProviderTest.kt @@ -12,6 +12,7 @@ class SharedArtworkContentProviderTest { callingUid = 1001, providerUid = 1001, uriPermissionResult = android.content.pm.PackageManager.PERMISSION_DENIED, + mediaContentControlResult = android.content.pm.PackageManager.PERMISSION_DENIED, ) ).isTrue() } @@ -23,6 +24,7 @@ class SharedArtworkContentProviderTest { callingUid = 2001, providerUid = 1001, uriPermissionResult = android.content.pm.PackageManager.PERMISSION_GRANTED, + mediaContentControlResult = android.content.pm.PackageManager.PERMISSION_DENIED, ) ).isTrue() } @@ -34,10 +36,25 @@ class SharedArtworkContentProviderTest { callingUid = 2001, providerUid = 1001, uriPermissionResult = android.content.pm.PackageManager.PERMISSION_DENIED, + mediaContentControlResult = android.content.pm.PackageManager.PERMISSION_DENIED, ) ).isFalse() } + @Test + fun artworkReadAccess_allowsSystemMediaSurfaceWithoutGrant() { + // System UI reads the platform session's artwork URI without connecting as a + // Media3 controller, so it never receives a per-item grant. + assertThat( + SharedArtworkContentProvider.hasArtworkReadAccess( + callingUid = 10_100, + providerUid = 1001, + uriPermissionResult = android.content.pm.PackageManager.PERMISSION_DENIED, + mediaContentControlResult = android.content.pm.PackageManager.PERMISSION_GRANTED, + ) + ).isTrue() + } + @Test fun buildSongUri_usesDedicatedArtworkAuthority() { val uri = SharedArtworkContentProvider.buildSongUriString( From ae19bb47225a2c60bfc7af0c7eb5dfb7dbc2cdb8 Mon Sep 17 00:00:00 2001 From: Duhan <136324426+lostf1sh@users.noreply.github.com> Date: Sun, 4 Oct 2026 15:03:07 +0300 Subject: [PATCH 3/3] Scope System UI artwork reads to the session's current cover Holding MEDIA_CONTENT_CONTROL let a privileged caller open any /song/ artwork, not just what the session publishes. MappingPlayer now records the artwork it exposes for the current item, and privileged callers may open only those (the last three, keyed by song id or raw cloud URI so cache-bust tokens don't matter). Also stop deleting a cached cover that fails the signature check: another request may have renamed a fresh download into place after the check read the old file. Skipping it is enough, since the next successful download replaces it. --- CHANGELOG.md | 2 +- .../data/image/RemoteArtworkCache.kt | 8 +-- .../provider/SharedArtworkContentProvider.kt | 48 +++++++++++++-- .../data/service/MusicService.kt | 1 + .../data/service/player/MappingPlayer.kt | 13 ++++- .../data/image/RemoteArtworkCacheTest.kt | 6 +- .../SharedArtworkContentProviderTest.kt | 58 ++++++++++++++++++- 7 files changed, 120 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c1390a36..3aa271e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ All notable changes to PixelPlayerOSS will be documented in this file. ### Fixed - Navidrome/Subsonic and Jellyfin album artwork no longer gets stuck on the placeholder after a few songs. The player loaded cloud covers through the app's own artwork content provider, and concurrent loads could exhaust the image decoder slots that provider waited on, freezing every artwork load until the app restarted. Server error responses are also no longer cached as cover images, and concurrent cover downloads no longer corrupt each other's cache file ([#138](https://github.com/PixelPlayerHQ/PixelPlayerOSS/issues/138)). -- System media controls can load album artwork through the app's artwork URI again. System UI reads that URI without connecting as a media controller, so it never received the per-track read grant and was refused on every track; privileged media surfaces (holders of `MEDIA_CONTENT_CONTROL`) are now allowed, while other apps still need the explicit grant. Stock Android fell back to the session bitmap, but System UIs without that fallback could show no artwork. +- System media controls can load album artwork through the app's artwork URI again. System UI reads that URI without connecting as a media controller, so it never received the per-track read grant and was refused on every track; privileged media surfaces (holders of `MEDIA_CONTENT_CONTROL`) may now open the artwork the session currently shows, while every other cover, and every other app, still needs the explicit grant. Stock Android fell back to the session bitmap, but System UIs without that fallback could show no artwork. - Jellyfin playlists no longer go missing on Jellyfin 10.10 and newer, where playlists can hold mixed content and audio playlists are often reported without a media type. - Pressing back with the Now Playing screen or the Next up queue open inside a folder in the Folders tab now closes the player instead of leaving the folder ([#136](https://github.com/PixelPlayerHQ/PixelPlayerOSS/issues/136)). diff --git a/app/src/main/java/com/lostf1sh/pixelplayeross/data/image/RemoteArtworkCache.kt b/app/src/main/java/com/lostf1sh/pixelplayeross/data/image/RemoteArtworkCache.kt index cedb70e8..57a68da2 100644 --- a/app/src/main/java/com/lostf1sh/pixelplayeross/data/image/RemoteArtworkCache.kt +++ b/app/src/main/java/com/lostf1sh/pixelplayeross/data/image/RemoteArtworkCache.kt @@ -24,10 +24,10 @@ internal object RemoteArtworkCache { fun cachedResult(file: File): SourceResult? { if (!file.isFile || file.length() == 0L) return null - if (!hasImageSignature(readSignature(file))) { - file.delete() - return null - } + // A body cached before this check existed is skipped, not deleted: the next successful + // download renames over it, and deleting here could remove a fresh file another + // request renamed into place after the signature was read. + if (!hasImageSignature(readSignature(file))) return null return SourceResult( source = ImageSource(file = file.absolutePath.toPath(), fileSystem = FileSystem.SYSTEM), mimeType = "image/jpeg", diff --git a/app/src/main/java/com/lostf1sh/pixelplayeross/data/provider/SharedArtworkContentProvider.kt b/app/src/main/java/com/lostf1sh/pixelplayeross/data/provider/SharedArtworkContentProvider.kt index b938e8e8..959c1928 100644 --- a/app/src/main/java/com/lostf1sh/pixelplayeross/data/provider/SharedArtworkContentProvider.kt +++ b/app/src/main/java/com/lostf1sh/pixelplayeross/data/provider/SharedArtworkContentProvider.kt @@ -98,12 +98,12 @@ class SharedArtworkContentProvider : ContentProvider() { * Some vendor System UI implementations reject URI grants before attempting to open a * non-exported provider. The provider is therefore exported for compatibility, while every * file open still requires our own UID, the explicit per-item grant issued by the media - * session, or a privileged media surface. + * session, or a privileged media surface reading what the session currently publishes. * - * System UI's media controls and notification read the platform session's artwork URI - * without ever connecting as a Media3 controller, so they never receive a per-item grant. - * They hold MEDIA_CONTENT_CONTROL (signature|privileged), which already lets them read and - * control every media session, so the artwork exposes nothing new to them. + * System UI's media controls read the platform session's artwork URI without ever + * connecting as a Media3 controller, so they never receive a per-item grant. They hold + * MEDIA_CONTENT_CONTROL (signature|privileged) and already see the session's metadata; + * they may open only the artwork that metadata currently points at, not arbitrary songs. */ private fun enforceArtworkReadAccess(uri: Uri, appContext: Context) { val callingUid = Binder.getCallingUid() @@ -125,6 +125,7 @@ class SharedArtworkContentProvider : ContentProvider() { providerUid = appContext.applicationInfo.uid, uriPermissionResult = permissionResult, mediaContentControlResult = mediaControlResult, + isPublishedSessionArtwork = isPublishedSessionArtwork(uri.toString(), appContext.packageName), ) ) { // FileNotFoundException is intentionally used here: media clients commonly treat a @@ -282,10 +283,45 @@ class SharedArtworkContentProvider : ContentProvider() { providerUid: Int, uriPermissionResult: Int, mediaContentControlResult: Int, + isPublishedSessionArtwork: Boolean, ): Boolean { return callingUid == providerUid || uriPermissionResult == PackageManager.PERMISSION_GRANTED || - mediaContentControlResult == PackageManager.PERMISSION_GRANTED + (mediaContentControlResult == PackageManager.PERMISSION_GRANTED && isPublishedSessionArtwork) } + + /* + * Artwork the media session currently publishes to platform surfaces, keyed by what the + * URI points at (song id or raw cloud URI) so cache-bust tokens don't matter. A few recent + * entries are kept because System UI may load a cover just after the track changes. + */ + private const val MAX_PUBLISHED_SESSION_ARTWORK = 3 + private val publishedSessionArtwork = ArrayDeque(MAX_PUBLISHED_SESSION_ARTWORK) + + /** Records artwork exposed through the session's current metadata. */ + fun publishSessionArtwork(packageName: String, artworkUri: String) { + val key = artworkKey(artworkUri, packageName) ?: return + synchronized(publishedSessionArtwork) { + if (publishedSessionArtwork.lastOrNull() == key) return + publishedSessionArtwork.remove(key) + publishedSessionArtwork.addLast(key) + while (publishedSessionArtwork.size > MAX_PUBLISHED_SESSION_ARTWORK) { + publishedSessionArtwork.removeFirst() + } + } + } + + internal fun isPublishedSessionArtwork(uriString: String, packageName: String): Boolean { + val key = artworkKey(uriString, packageName) ?: return false + return synchronized(publishedSessionArtwork) { key in publishedSessionArtwork } + } + + internal fun clearPublishedSessionArtwork() { + synchronized(publishedSessionArtwork) { publishedSessionArtwork.clear() } + } + + private fun artworkKey(uriString: String, packageName: String): String? = + parseSongId(uriString, packageName)?.let { "song:$it" } + ?: parseCloudArtworkUri(uriString, packageName)?.let { "cloud:$it" } } } diff --git a/app/src/main/java/com/lostf1sh/pixelplayeross/data/service/MusicService.kt b/app/src/main/java/com/lostf1sh/pixelplayeross/data/service/MusicService.kt index fe62c1aa..ccbe5058 100644 --- a/app/src/main/java/com/lostf1sh/pixelplayeross/data/service/MusicService.kt +++ b/app/src/main/java/com/lostf1sh/pixelplayeross/data/service/MusicService.kt @@ -936,6 +936,7 @@ class MusicService : MediaSessionService() { override fun onDestroy() { PlaybackActivityTracker.setPlaybackActive(false) + com.lostf1sh.pixelplayeross.data.provider.SharedArtworkContentProvider.clearPublishedSessionArtwork() listeningStatsTracker.finalizeCurrentSession(forceSynchronousPersistence = true) reportNavidromePlayback("stopped") stopNavidromePlaybackReporting() diff --git a/app/src/main/java/com/lostf1sh/pixelplayeross/data/service/player/MappingPlayer.kt b/app/src/main/java/com/lostf1sh/pixelplayeross/data/service/player/MappingPlayer.kt index 4889fe24..492b632d 100644 --- a/app/src/main/java/com/lostf1sh/pixelplayeross/data/service/player/MappingPlayer.kt +++ b/app/src/main/java/com/lostf1sh/pixelplayeross/data/service/player/MappingPlayer.kt @@ -1,6 +1,7 @@ package com.lostf1sh.pixelplayeross.data.service.player import android.content.Context +import android.net.Uri import androidx.annotation.OptIn import androidx.media3.common.ForwardingPlayer import androidx.media3.common.MediaItem @@ -8,6 +9,7 @@ import androidx.media3.common.MediaMetadata import androidx.media3.common.Player import androidx.media3.common.Timeline import androidx.media3.common.util.UnstableApi +import com.lostf1sh.pixelplayeross.data.provider.SharedArtworkContentProvider import com.lostf1sh.pixelplayeross.utils.MediaItemBuilder /** @@ -21,13 +23,14 @@ class MappingPlayer( private val context: Context ) : ForwardingPlayer(innerPlayer) { - private fun mapMediaItem(mediaItem: MediaItem?): MediaItem? { + private fun mapMediaItem(mediaItem: MediaItem?, publishesToSession: Boolean = false): MediaItem? { if (mediaItem == null) return null val artworkUri = mediaItem.mediaMetadata.artworkUri ?: return mediaItem val exposedArtworkUri = MediaItemBuilder.externalControllerArtworkUri( context = context, rawArtworkUri = artworkUri.toString() ) ?: return mediaItem + if (publishesToSession) publish(exposedArtworkUri) if (exposedArtworkUri == artworkUri) return mediaItem val mappedMetadata = mediaItem.mediaMetadata.buildUpon() @@ -38,8 +41,13 @@ class MappingPlayer( .build() } + /** The current item's artwork reaches System UI through the platform session metadata. */ + private fun publish(exposedArtworkUri: Uri) { + SharedArtworkContentProvider.publishSessionArtwork(context.packageName, exposedArtworkUri.toString()) + } + override fun getCurrentMediaItem(): MediaItem? { - return mapMediaItem(super.getCurrentMediaItem()) + return mapMediaItem(super.getCurrentMediaItem(), publishesToSession = true) } override fun getMediaItemAt(index: Int): MediaItem { @@ -53,6 +61,7 @@ class MappingPlayer( context = context, rawArtworkUri = artworkUri.toString() ) ?: return metadata + publish(exposedArtworkUri) if (exposedArtworkUri == artworkUri) return metadata return metadata.buildUpon() diff --git a/app/src/test/java/com/lostf1sh/pixelplayeross/data/image/RemoteArtworkCacheTest.kt b/app/src/test/java/com/lostf1sh/pixelplayeross/data/image/RemoteArtworkCacheTest.kt index d7c28f67..c9362e97 100644 --- a/app/src/test/java/com/lostf1sh/pixelplayeross/data/image/RemoteArtworkCacheTest.kt +++ b/app/src/test/java/com/lostf1sh/pixelplayeross/data/image/RemoteArtworkCacheTest.kt @@ -47,11 +47,13 @@ class RemoteArtworkCacheTest { } @Test - fun `previously cached error body is evicted instead of served`() { + fun `previously cached error body is not served and the next download replaces it`() { val poisoned = File(cacheDir, "navidrome_cover_al-1_500.jpg").apply { writeText(subsonicError) } assertThat(RemoteArtworkCache.cachedResult(poisoned)).isNull() - assertThat(poisoned.exists()).isFalse() + + RemoteArtworkCache.download(clientReturning(jpegBytes, "image/jpeg"), request(), poisoned) + assertThat(RemoteArtworkCache.cachedResult(poisoned)).isNotNull() } @Test diff --git a/app/src/test/java/com/lostf1sh/pixelplayeross/data/provider/SharedArtworkContentProviderTest.kt b/app/src/test/java/com/lostf1sh/pixelplayeross/data/provider/SharedArtworkContentProviderTest.kt index 9663d9ea..2856d122 100644 --- a/app/src/test/java/com/lostf1sh/pixelplayeross/data/provider/SharedArtworkContentProviderTest.kt +++ b/app/src/test/java/com/lostf1sh/pixelplayeross/data/provider/SharedArtworkContentProviderTest.kt @@ -13,6 +13,7 @@ class SharedArtworkContentProviderTest { providerUid = 1001, uriPermissionResult = android.content.pm.PackageManager.PERMISSION_DENIED, mediaContentControlResult = android.content.pm.PackageManager.PERMISSION_DENIED, + isPublishedSessionArtwork = false, ) ).isTrue() } @@ -25,6 +26,7 @@ class SharedArtworkContentProviderTest { providerUid = 1001, uriPermissionResult = android.content.pm.PackageManager.PERMISSION_GRANTED, mediaContentControlResult = android.content.pm.PackageManager.PERMISSION_DENIED, + isPublishedSessionArtwork = false, ) ).isTrue() } @@ -37,12 +39,13 @@ class SharedArtworkContentProviderTest { providerUid = 1001, uriPermissionResult = android.content.pm.PackageManager.PERMISSION_DENIED, mediaContentControlResult = android.content.pm.PackageManager.PERMISSION_DENIED, + isPublishedSessionArtwork = true, ) ).isFalse() } @Test - fun artworkReadAccess_allowsSystemMediaSurfaceWithoutGrant() { + fun artworkReadAccess_allowsSystemMediaSurfaceForPublishedArtwork() { // System UI reads the platform session's artwork URI without connecting as a // Media3 controller, so it never receives a per-item grant. assertThat( @@ -51,10 +54,59 @@ class SharedArtworkContentProviderTest { providerUid = 1001, uriPermissionResult = android.content.pm.PackageManager.PERMISSION_DENIED, mediaContentControlResult = android.content.pm.PackageManager.PERMISSION_GRANTED, + isPublishedSessionArtwork = true, ) ).isTrue() } + @Test + fun artworkReadAccess_rejectsSystemMediaSurfaceForUnpublishedArtwork() { + assertThat( + SharedArtworkContentProvider.hasArtworkReadAccess( + callingUid = 10_100, + providerUid = 1001, + uriPermissionResult = android.content.pm.PackageManager.PERMISSION_DENIED, + mediaContentControlResult = android.content.pm.PackageManager.PERMISSION_GRANTED, + isPublishedSessionArtwork = false, + ) + ).isFalse() + } + + @Test + fun publishedSessionArtwork_matchesSongRegardlessOfCacheBustToken() { + SharedArtworkContentProvider.clearPublishedSessionArtwork() + SharedArtworkContentProvider.publishSessionArtwork( + PACKAGE, + SharedArtworkContentProvider.buildSongUriString(PACKAGE, 42L, cacheBustToken = "1") + ) + + assertThat( + SharedArtworkContentProvider.isPublishedSessionArtwork( + SharedArtworkContentProvider.buildSongUriString(PACKAGE, 42L, cacheBustToken = "2"), + PACKAGE + ) + ).isTrue() + assertThat( + SharedArtworkContentProvider.isPublishedSessionArtwork( + SharedArtworkContentProvider.buildSongUriString(PACKAGE, 43L), + PACKAGE + ) + ).isFalse() + } + + @Test + fun publishedSessionArtwork_keepsOnlyRecentTracks() { + SharedArtworkContentProvider.clearPublishedSessionArtwork() + val covers = (1..4).map { index -> + SharedArtworkContentProvider.buildCloudUriString(PACKAGE, "navidrome_cover://al-$index")!! + } + covers.forEach { SharedArtworkContentProvider.publishSessionArtwork(PACKAGE, it) } + + assertThat(SharedArtworkContentProvider.isPublishedSessionArtwork(covers.first(), PACKAGE)).isFalse() + assertThat(covers.drop(1).all { SharedArtworkContentProvider.isPublishedSessionArtwork(it, PACKAGE) }) + .isTrue() + } + @Test fun buildSongUri_usesDedicatedArtworkAuthority() { val uri = SharedArtworkContentProvider.buildSongUriString( @@ -140,4 +192,8 @@ class SharedArtworkContentProviderTest { assertThat(sharedUri).isNull() } + + private companion object { + const val PACKAGE = "com.lostf1sh.pixelplayeross" + } }