From b2b2adf0762766fae41a47728f999a9b999122a8 Mon Sep 17 00:00:00 2001 From: davotoula Date: Sun, 26 Jul 2026 06:49:00 +0200 Subject: [PATCH 1/2] fix(audio): stop the inline audio player painting over the note --- .../components/AudioPlayerBoxOverflowTest.kt | 125 ++++++++++++++++++ .../ui/components/ZoomableContentView.kt | 28 +++- .../components/UnknownMediaAspectRatioTest.kt | 63 +++++++++ .../commons/richtext/RichTextParser.kt | 11 ++ .../richtext/RichTextParserAudioUrlTest.kt | 81 ++++++++++++ 5 files changed, 307 insertions(+), 1 deletion(-) create mode 100644 amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/components/AudioPlayerBoxOverflowTest.kt create mode 100644 amethyst/src/test/java/com/vitorpamplona/amethyst/ui/components/UnknownMediaAspectRatioTest.kt create mode 100644 commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/richtext/RichTextParserAudioUrlTest.kt diff --git a/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/components/AudioPlayerBoxOverflowTest.kt b/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/components/AudioPlayerBoxOverflowTest.kt new file mode 100644 index 0000000000..9cea5fca62 --- /dev/null +++ b/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/components/AudioPlayerBoxOverflowTest.kt @@ -0,0 +1,125 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.ui.components + +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.size +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.layout.ContentScale +import androidx.compose.ui.layout.onGloballyPositioned +import androidx.compose.ui.layout.positionInRoot +import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.unit.dp +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.vitorpamplona.amethyst.service.playback.composable.audioSquare +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith + +/** + * The inline audio player sizes itself as a square so the visualizer and controls get room, which is + * taller than the 16:9 box [ZoomableContentView] builds for a video of unknown dimensions. That box + * is centre-aligned and nothing between it and NoteComposeLayout clips, so caging the square in it + * makes the player paint over the note header above and the reactions row below. + * + * These tests recreate that enclosure — the real [mediaSizingModifier] over the real + * [unknownMediaAspectRatio], holding the real [audioSquare] — and pin both halves of the contract: + * audio must not be given a ratio, and video must keep the 16:9 assumption that stops live streams + * from letterboxing on first play. + */ +@RunWith(AndroidJUnit4::class) +class AudioPlayerBoxOverflowTest { + @get:Rule val rule = createComposeRule() + + private class Bounds { + var top = 0f + var bottom = 0f + var height = 0 + + fun modifier() = + Modifier.onGloballyPositioned { + top = it.positionInRoot().y + height = it.size.height + bottom = top + height + } + } + + private fun measure( + mimeType: String?, + url: String, + square: Boolean, + ): Pair { + val box = Bounds() + val player = Bounds() + + rule.setContent { + Box(Modifier.size(width = 400.dp, height = 1200.dp)) { + Box( + modifier = mediaSizingModifier(unknownMediaAspectRatio(mimeType, url), ContentScale.Fit).then(box.modifier()), + contentAlignment = Alignment.Center, + ) { + Box((if (square) Modifier.audioSquare() else Modifier).then(player.modifier())) + } + } + } + rule.waitForIdle() + + return box to player + } + + @Test + fun squareAudioPlayerStaysInsideItsMediaBox() { + val (box, player) = measure(null, "https://haven.sdbitcoiners.com/f28a5a2e.mp3", square = true) + + assertTrue( + "player overflows above the media box by ${box.top - player.top}px", + player.top >= box.top, + ) + assertTrue( + "player overflows below the media box by ${player.bottom - box.bottom}px", + player.bottom <= box.bottom, + ) + assertEquals("the box should wrap the square", player.height, box.height) + } + + @Test + fun audioWithAnExplicitMimeAlsoStaysInside() { + val (box, player) = measure("audio/mpeg", "https://example.com/download?id=7", square = true) + + assertTrue(player.top >= box.top && player.bottom <= box.bottom) + assertEquals(player.height, box.height) + } + + @Test + fun videoOfUnknownSizeKeeps16by9() { + val (box, _) = measure(null, "https://example.com/a.mp4", square = false) + + // 400.dp wide at 16:9. Rounding lands within a pixel either way. + val expected = box.height * 16f / 9f + assertTrue( + "expected a 16:9 box, got height=${box.height} for width=$expected", + kotlin.math.abs(expected - 400 * rule.density.density) <= 1f, + ) + } +} diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentView.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentView.kt index e918d88d80..2164723756 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentView.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentView.kt @@ -87,6 +87,7 @@ import com.vitorpamplona.amethyst.commons.richtext.MediaUrlContent import com.vitorpamplona.amethyst.commons.richtext.MediaUrlImage import com.vitorpamplona.amethyst.commons.richtext.MediaUrlPdf import com.vitorpamplona.amethyst.commons.richtext.MediaUrlVideo +import com.vitorpamplona.amethyst.commons.richtext.RichTextParser import com.vitorpamplona.amethyst.commons.richtext.toCoilModel import com.vitorpamplona.amethyst.commons.ui.components.LoadingAnimation import com.vitorpamplona.amethyst.model.MediaAspectRatioCache @@ -144,6 +145,28 @@ private const val SHARED_VIDEO_CLEANUP_DELAY_MS = 120_000L // place; [MediaAspectRatioCache] then corrects anything unusual once the decoder reports its size. private const val DEFAULT_VIDEO_ASPECT_RATIO = 16f / 9f +/** + * Shape to assume for a [MediaUrlVideo] nobody has reported dimensions for — no imeta `dim` and + * nothing in [MediaAspectRatioCache]. + * + * Video gets [DEFAULT_VIDEO_ASPECT_RATIO]. Audio gets null, meaning "no ratio, wrap whatever the + * player asks for": audio has no picture, and the inline player sizes itself as a square (see + * `AudioPlayerSquare.audioSquare`) so the visualizer and controls get room. A square is taller than + * 16:9, so caging it in a 16:9 box — which is centre-aligned and unclipped all the way up through + * NoteComposeLayout — makes the player paint over the note header above it and the reactions row + * below. Audio also never reaches the cache, since it has no video track to report a size, so the + * miss is permanent rather than first-play-only. + */ +internal fun unknownMediaAspectRatio( + mimeType: String?, + url: String, +): Float? = + when { + mimeType != null -> if (mimeType.startsWith("audio/")) null else DEFAULT_VIDEO_ASPECT_RATIO + RichTextParser.isAudioUrl(url) -> null + else -> DEFAULT_VIDEO_ASPECT_RATIO + } + @Composable fun ZoomableContentView( content: BaseMediaContent, @@ -203,7 +226,10 @@ fun ZoomableContentView( } is MediaUrlVideo -> { - val ratio = content.dim?.aspectRatio() ?: MediaAspectRatioCache.get(content.url) ?: DEFAULT_VIDEO_ASPECT_RATIO + val ratio = + content.dim?.aspectRatio() + ?: MediaAspectRatioCache.get(content.url) + ?: unknownMediaAspectRatio(content.mimeType, content.url) val bridgedUrl = remember(content.url, useLocalBlossomBridge) { content.toCoilModel(useLocalBlossomBridge) diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/components/UnknownMediaAspectRatioTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/components/UnknownMediaAspectRatioTest.kt new file mode 100644 index 0000000000..26f7e230f8 --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/components/UnknownMediaAspectRatioTest.kt @@ -0,0 +1,63 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.ui.components + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +/** + * The audio player sizes itself as a square (see AudioPlayerSquare.audioSquare), which is taller than + * 16:9. Handing audio the video default therefore cages the square in a shorter, centre-aligned box + * that nothing clips, and the player — visualizer included — paints over the note header above and + * the reactions row below. Audio must stay ratio-less so the box wraps the square instead. + */ +class UnknownMediaAspectRatioTest { + @Test + fun unknownVideoAssumes16by9() { + assertEquals(16f / 9f, unknownMediaAspectRatio(null, "https://example.com/a.mp4")) + } + + @Test + fun liveStreamPlaylistKeeps16by9() { + assertEquals(16f / 9f, unknownMediaAspectRatio(null, "https://example.com/stream.m3u8")) + } + + @Test + fun bareAudioUrlHasNoRatio() { + assertNull(unknownMediaAspectRatio(null, "https://haven.sdbitcoiners.com/f28a5a2e.mp3")) + } + + @Test + fun audioMimeHasNoRatioEvenWithoutAnExtension() { + assertNull(unknownMediaAspectRatio("audio/mpeg", "https://example.com/download?id=7")) + } + + @Test + fun videoMimeWinsOverNothing() { + assertEquals(16f / 9f, unknownMediaAspectRatio("video/mp4", "https://example.com/download?id=7")) + } + + @Test + fun unknownEverythingAssumes16by9() { + assertEquals(16f / 9f, unknownMediaAspectRatio(null, "https://example.com/download?id=7")) + } +} diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/richtext/RichTextParser.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/richtext/RichTextParser.kt index 1f61884261..6e8ddcbf18 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/richtext/RichTextParser.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/richtext/RichTextParser.kt @@ -498,9 +498,15 @@ class RichTextParser { val videoExt = listOf("mp4", "avi", "wmv", "mpg", "amv", "webm", "mov", "mp3", "m3u8", "ogg", "wav", "flac", "aac", "opus", "m4a", "f4a") val pdfExt = listOf("pdf") + // The audio-only members of [videoExt] — both play through the same video pipeline, but audio + // has no picture, so anything that reasons about the shape of the media (aspect ratios, player + // sizing) has to tell them apart. `m3u8` stays out: a playlist carries either. + val audioExt = listOf("mp3", "ogg", "wav", "flac", "aac", "opus", "m4a", "f4a") + val imageExtensions = imageExt + imageExt.map { it.uppercase() } val videoExtensions = videoExt + videoExt.map { it.uppercase() } val pdfExtensions = pdfExt + pdfExt.map { it.uppercase() } + val audioExtensions = audioExt + audioExt.map { it.uppercase() } val tagIndex = Regex("\\#\\[([0-9]+)\\](.*)") val hashTagsPattern: Regex = @@ -542,6 +548,11 @@ class RichTextParser { return videoExtensions.any { removedParamsFromUrl.endsWith(it) } } + fun isAudioUrl(url: String): Boolean { + val removedParamsFromUrl = removeQueryParamsForExtensionComparison(url) + return audioExtensions.any { removedParamsFromUrl.endsWith(it) } + } + // Mirrors the canonical HLS-playlist MIME list also kept in MediaItemCache.toExoPlayerMimeType. // Called per URL during feed render — uses `equals(ignoreCase)` instead of `lowercase()` to // avoid a per-call String allocation on the common non-HLS path. diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/richtext/RichTextParserAudioUrlTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/richtext/RichTextParserAudioUrlTest.kt new file mode 100644 index 0000000000..298692a4e4 --- /dev/null +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/richtext/RichTextParserAudioUrlTest.kt @@ -0,0 +1,81 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.commons.richtext + +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class RichTextParserAudioUrlTest { + @Test + fun mp3IsAudio() { + assertTrue(RichTextParser.isAudioUrl("https://haven.sdbitcoiners.com/f28a5a2e.mp3")) + } + + @Test + fun otherAudioContainersAreAudio() { + assertTrue(RichTextParser.isAudioUrl("https://example.com/a.wav")) + assertTrue(RichTextParser.isAudioUrl("https://example.com/a.flac")) + assertTrue(RichTextParser.isAudioUrl("https://example.com/a.aac")) + assertTrue(RichTextParser.isAudioUrl("https://example.com/a.opus")) + assertTrue(RichTextParser.isAudioUrl("https://example.com/a.m4a")) + assertTrue(RichTextParser.isAudioUrl("https://example.com/a.f4a")) + assertTrue(RichTextParser.isAudioUrl("https://example.com/a.ogg")) + } + + @Test + fun uppercaseIsAudio() { + assertTrue(RichTextParser.isAudioUrl("https://example.com/A.MP3")) + } + + @Test + fun queryParamsAndFragmentsAreIgnored() { + assertTrue(RichTextParser.isAudioUrl("https://example.com/a.mp3?x=1")) + assertTrue(RichTextParser.isAudioUrl("https://example.com/a.mp3#t=10")) + } + + @Test + fun videoIsNotAudio() { + assertFalse(RichTextParser.isAudioUrl("https://example.com/a.mp4")) + assertFalse(RichTextParser.isAudioUrl("https://example.com/a.webm")) + assertFalse(RichTextParser.isAudioUrl("https://example.com/a.mov")) + } + + @Test + fun hlsPlaylistIsNotAudio() { + // A .m3u8 carries either, and a live stream is the reason the 16:9 default exists. + assertFalse(RichTextParser.isAudioUrl("https://example.com/stream.m3u8")) + } + + @Test + fun imagesAndUnknownAreNotAudio() { + assertFalse(RichTextParser.isAudioUrl("https://example.com/a.jpg")) + assertFalse(RichTextParser.isAudioUrl("https://example.com/nothing")) + } + + @Test + fun audioExtensionsAreASubsetOfVideoExtensions() { + // Audio arrives as MediaUrlVideo precisely because videoExt lumps the two together. + RichTextParser.audioExt.forEach { + assertTrue(RichTextParser.videoExt.contains(it), "videoExt must still contain $it") + } + } +} From 482be60e4a8bc82bfa6e2a9a1bd9f562df0ca0ef Mon Sep 17 00:00:00 2001 From: davotoula Date: Sun, 26 Jul 2026 08:08:00 +0200 Subject: [PATCH 2/2] Code review: - fold audioExt into videoExt and share the classifier --- .../components/AudioPlayerBoxOverflowTest.kt | 24 +++--------- .../ui/components/ZoomableContentView.kt | 15 +++---- .../commons/richtext/RichTextParser.kt | 39 ++++++++++++------- .../richtext/RichTextParserAudioUrlTest.kt | 26 ++++++------- 4 files changed, 53 insertions(+), 51 deletions(-) diff --git a/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/components/AudioPlayerBoxOverflowTest.kt b/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/components/AudioPlayerBoxOverflowTest.kt index 9cea5fca62..564715b2f9 100644 --- a/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/components/AudioPlayerBoxOverflowTest.kt +++ b/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/components/AudioPlayerBoxOverflowTest.kt @@ -55,18 +55,19 @@ class AudioPlayerBoxOverflowTest { private class Bounds { var top = 0f var bottom = 0f + var width = 0 var height = 0 fun modifier() = Modifier.onGloballyPositioned { top = it.positionInRoot().y + width = it.size.width height = it.size.height bottom = top + height } } private fun measure( - mimeType: String?, url: String, square: Boolean, ): Pair { @@ -76,7 +77,7 @@ class AudioPlayerBoxOverflowTest { rule.setContent { Box(Modifier.size(width = 400.dp, height = 1200.dp)) { Box( - modifier = mediaSizingModifier(unknownMediaAspectRatio(mimeType, url), ContentScale.Fit).then(box.modifier()), + modifier = mediaSizingModifier(unknownMediaAspectRatio(null, url), ContentScale.Fit).then(box.modifier()), contentAlignment = Alignment.Center, ) { Box((if (square) Modifier.audioSquare() else Modifier).then(player.modifier())) @@ -90,7 +91,7 @@ class AudioPlayerBoxOverflowTest { @Test fun squareAudioPlayerStaysInsideItsMediaBox() { - val (box, player) = measure(null, "https://haven.sdbitcoiners.com/f28a5a2e.mp3", square = true) + val (box, player) = measure("https://haven.sdbitcoiners.com/f28a5a2e.mp3", square = true) assertTrue( "player overflows above the media box by ${box.top - player.top}px", @@ -103,23 +104,10 @@ class AudioPlayerBoxOverflowTest { assertEquals("the box should wrap the square", player.height, box.height) } - @Test - fun audioWithAnExplicitMimeAlsoStaysInside() { - val (box, player) = measure("audio/mpeg", "https://example.com/download?id=7", square = true) - - assertTrue(player.top >= box.top && player.bottom <= box.bottom) - assertEquals(player.height, box.height) - } - @Test fun videoOfUnknownSizeKeeps16by9() { - val (box, _) = measure(null, "https://example.com/a.mp4", square = false) + val (box, _) = measure("https://example.com/a.mp4", square = false) - // 400.dp wide at 16:9. Rounding lands within a pixel either way. - val expected = box.height * 16f / 9f - assertTrue( - "expected a 16:9 box, got height=${box.height} for width=$expected", - kotlin.math.abs(expected - 400 * rule.density.density) <= 1f, - ) + assertEquals(16f / 9f, box.width.toFloat() / box.height, 0.01f) } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentView.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentView.kt index 2164723756..3e560530cc 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentView.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentView.kt @@ -160,12 +160,7 @@ private const val DEFAULT_VIDEO_ASPECT_RATIO = 16f / 9f internal fun unknownMediaAspectRatio( mimeType: String?, url: String, -): Float? = - when { - mimeType != null -> if (mimeType.startsWith("audio/")) null else DEFAULT_VIDEO_ASPECT_RATIO - RichTextParser.isAudioUrl(url) -> null - else -> DEFAULT_VIDEO_ASPECT_RATIO - } +): Float? = if (RichTextParser.isAudioContent(mimeType, url)) null else DEFAULT_VIDEO_ASPECT_RATIO @Composable fun ZoomableContentView( @@ -226,10 +221,16 @@ fun ZoomableContentView( } is MediaUrlVideo -> { + // The fallback classifies the URL string, so compute it once per content — for audio + // the cache miss is permanent and this branch re-runs on every recomposition. + val fallbackRatio = + remember(content.url, content.mimeType) { + unknownMediaAspectRatio(content.mimeType, content.url) + } val ratio = content.dim?.aspectRatio() ?: MediaAspectRatioCache.get(content.url) - ?: unknownMediaAspectRatio(content.mimeType, content.url) + ?: fallbackRatio val bridgedUrl = remember(content.url, useLocalBlossomBridge) { content.toCoilModel(useLocalBlossomBridge) diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/richtext/RichTextParser.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/richtext/RichTextParser.kt index 6e8ddcbf18..60f04f7578 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/richtext/RichTextParser.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/richtext/RichTextParser.kt @@ -495,13 +495,20 @@ class RichTextParser { ) val imageExt = listOf("png", "jpg", "gif", "bmp", "jpeg", "webp", "svg", "avif") - val videoExt = listOf("mp4", "avi", "wmv", "mpg", "amv", "webm", "mov", "mp3", "m3u8", "ogg", "wav", "flac", "aac", "opus", "m4a", "f4a") - val pdfExt = listOf("pdf") - // The audio-only members of [videoExt] — both play through the same video pipeline, but audio - // has no picture, so anything that reasons about the shape of the media (aspect ratios, player - // sizing) has to tell them apart. `m3u8` stays out: a playlist carries either. + // Audio is folded into [videoExt] because both play through the same video pipeline — but + // audio has no picture, so anything that reasons about the shape of the media (aspect + // ratios, player sizing) has to tell them apart. `m3u8` stays video-only: a playlist + // carries either. + // + // Composing [videoExt] out of [audioExt] is what keeps "every audio extension is also a + // video extension" true by construction rather than by convention — the two lists cannot + // drift apart. It is also why [audioExt] is declared first; the compiler rejects the + // reverse order outright ("Variable 'audioExt' must be initialized"), so this note is + // intent, not a guard rail. val audioExt = listOf("mp3", "ogg", "wav", "flac", "aac", "opus", "m4a", "f4a") + val videoExt = listOf("mp4", "avi", "wmv", "mpg", "amv", "webm", "mov", "m3u8") + audioExt + val pdfExt = listOf("pdf") val imageExtensions = imageExt + imageExt.map { it.uppercase() } val videoExtensions = videoExt + videoExt.map { it.uppercase() } @@ -518,14 +525,13 @@ class RichTextParser { it.uppercase() } - private fun removeQueryParamsForExtensionComparison(fullUrl: String): String = - if (fullUrl.contains("?")) { - fullUrl.split("?")[0] - } else if (fullUrl.contains("#")) { - fullUrl.split("#")[0] - } else { - fullUrl - } + private fun removeQueryParamsForExtensionComparison(fullUrl: String): String { + // Called per URL during feed render — substringBefore allocates nothing when the + // separator is absent, unlike split(). + val queryStart = fullUrl.indexOf('?') + if (queryStart >= 0) return fullUrl.substring(0, queryStart) + return fullUrl.substringBefore('#') + } fun isImageExtension(ext: String) = imageExtensions.any { it == ext } @@ -553,6 +559,13 @@ class RichTextParser { return audioExtensions.any { removedParamsFromUrl.endsWith(it) } } + // A declared MIME type is authoritative when present; the URL extension is only a fallback + // for the common bare-URL case. + fun isAudioContent( + mimeType: String?, + url: String, + ): Boolean = mimeType?.startsWith("audio/") ?: isAudioUrl(url) + // Mirrors the canonical HLS-playlist MIME list also kept in MediaItemCache.toExoPlayerMimeType. // Called per URL during feed render — uses `equals(ignoreCase)` instead of `lowercase()` to // avoid a per-call String allocation on the common non-HLS path. diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/richtext/RichTextParserAudioUrlTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/richtext/RichTextParserAudioUrlTest.kt index 298692a4e4..f8537790a1 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/richtext/RichTextParserAudioUrlTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/richtext/RichTextParserAudioUrlTest.kt @@ -31,14 +31,10 @@ class RichTextParserAudioUrlTest { } @Test - fun otherAudioContainersAreAudio() { - assertTrue(RichTextParser.isAudioUrl("https://example.com/a.wav")) - assertTrue(RichTextParser.isAudioUrl("https://example.com/a.flac")) - assertTrue(RichTextParser.isAudioUrl("https://example.com/a.aac")) - assertTrue(RichTextParser.isAudioUrl("https://example.com/a.opus")) - assertTrue(RichTextParser.isAudioUrl("https://example.com/a.m4a")) - assertTrue(RichTextParser.isAudioUrl("https://example.com/a.f4a")) - assertTrue(RichTextParser.isAudioUrl("https://example.com/a.ogg")) + fun everyAudioContainerIsAudio() { + RichTextParser.audioExt.forEach { + assertTrue(RichTextParser.isAudioUrl("https://example.com/a.$it"), it) + } } @Test @@ -72,10 +68,14 @@ class RichTextParserAudioUrlTest { } @Test - fun audioExtensionsAreASubsetOfVideoExtensions() { - // Audio arrives as MediaUrlVideo precisely because videoExt lumps the two together. - RichTextParser.audioExt.forEach { - assertTrue(RichTextParser.videoExt.contains(it), "videoExt must still contain $it") - } + fun mimeTypeIsAuthoritativeOverTheUrl() { + assertTrue(RichTextParser.isAudioContent("audio/mpeg", "https://example.com/download?id=7")) + assertFalse(RichTextParser.isAudioContent("video/mp4", "https://example.com/a.mp3")) + } + + @Test + fun urlExtensionIsTheFallbackWithoutAMimeType() { + assertTrue(RichTextParser.isAudioContent(null, "https://example.com/a.mp3")) + assertFalse(RichTextParser.isAudioContent(null, "https://example.com/a.mp4")) } }