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..564715b2f9 --- /dev/null +++ b/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/components/AudioPlayerBoxOverflowTest.kt @@ -0,0 +1,113 @@ +/* + * 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 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( + 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(null, 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("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 videoOfUnknownSizeKeeps16by9() { + val (box, _) = measure("https://example.com/a.mp4", square = false) + + 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 e918d88d80..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 @@ -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,23 @@ 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? = if (RichTextParser.isAudioContent(mimeType, url)) null else DEFAULT_VIDEO_ASPECT_RATIO + @Composable fun ZoomableContentView( content: BaseMediaContent, @@ -203,7 +221,16 @@ fun ZoomableContentView( } is MediaUrlVideo -> { - val ratio = content.dim?.aspectRatio() ?: MediaAspectRatioCache.get(content.url) ?: DEFAULT_VIDEO_ASPECT_RATIO + // 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) + ?: fallbackRatio 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..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,12 +495,25 @@ 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") + + // 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() } val pdfExtensions = pdfExt + pdfExt.map { it.uppercase() } + val audioExtensions = audioExt + audioExt.map { it.uppercase() } val tagIndex = Regex("\\#\\[([0-9]+)\\](.*)") val hashTagsPattern: Regex = @@ -512,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 } @@ -542,6 +554,18 @@ class RichTextParser { return videoExtensions.any { removedParamsFromUrl.endsWith(it) } } + fun isAudioUrl(url: String): Boolean { + val removedParamsFromUrl = removeQueryParamsForExtensionComparison(url) + 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 new file mode 100644 index 0000000000..f8537790a1 --- /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 everyAudioContainerIsAudio() { + RichTextParser.audioExt.forEach { + assertTrue(RichTextParser.isAudioUrl("https://example.com/a.$it"), it) + } + } + + @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 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")) + } +}