From 2dab90fc2e4ca1f74d196b9f41a07699492fd340 Mon Sep 17 00:00:00 2001 From: davotoula Date: Tue, 14 Apr 2026 21:21:58 +0200 Subject: [PATCH] fix(video-quality): label by short side so portrait videos show 360p/540p etc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit VideoQualityButton and VideoQualityChoices were labelling each rendition by format.height. That matches the streaming convention "360p = 360 pixel short side" only for landscape content. For a portrait upload (9:16) the renditions encode as 360x640, 540x960, 720x1280, 1080x1920, 2160x3840 — so format.height is the long side, and the picker rendered "640p / 960p / 1280p / 1920p / 3840p" instead of the expected "360p / 540p / 720p / 1080p / 4K". Switch to minOf(format.width, format.height) (the short side) for both the ladder rung labels and the currently-playing indicator. Rename QualityChoice.height -> QualityChoice.shortSide and getCurrentPlayingHeight() -> getCurrentPlayingShortSide() so the fields match the thing they now represent, and add a one-line comment explaining the convention. Co-Authored-By: Claude Opus 4.5 --- .../controls/VideoQualityAvailability.kt | 11 ++++++++-- .../composable/controls/VideoQualityButton.kt | 22 +++++++++++-------- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/VideoQualityAvailability.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/VideoQualityAvailability.kt index ff9be88bd1..25581490cf 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/VideoQualityAvailability.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/VideoQualityAvailability.kt @@ -25,10 +25,17 @@ import androidx.media3.common.Tracks fun getVideoTrackGroup(tracks: Tracks): Tracks.Group? = tracks.groups.firstOrNull { it.type == C.TRACK_TYPE_VIDEO && it.length > 0 } -fun getCurrentPlayingHeight(tracks: Tracks): Int? { +// Returns the "Xp" value for the currently selected video track. Uses min(width, height) so +// that a portrait video's renditions get the same "360p / 540p / 720p" labels as a landscape +// source — the streaming convention is to label by the short side, not format.height which is +// the long side for portrait content. +fun getCurrentPlayingShortSide(tracks: Tracks): Int? { val group = getVideoTrackGroup(tracks) ?: return null for (i in 0 until group.length) { - if (group.isTrackSelected(i)) return group.getTrackFormat(i).height + if (group.isTrackSelected(i)) { + val format = group.getTrackFormat(i) + return minOf(format.width, format.height).takeIf { it > 0 } + } } return null } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/VideoQualityButton.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/VideoQualityButton.kt index 3d59c00705..99637bfbcd 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/VideoQualityButton.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/VideoQualityButton.kt @@ -124,7 +124,7 @@ fun VideoQualityButton( ) { VideoQualityChoices( videoGroup = videoGroup, - currentHeight = getCurrentPlayingHeight(tracks), + currentShortSide = getCurrentPlayingShortSide(tracks), isAuto = !hasVideoOverride(player), onSelectAuto = { clearVideoOverride(player) @@ -142,7 +142,7 @@ fun VideoQualityButton( @Composable private fun VideoQualityChoices( videoGroup: Tracks.Group, - currentHeight: Int?, + currentShortSide: Int?, isAuto: Boolean, onSelectAuto: () -> Unit, onSelectTrack: (Int) -> Unit, @@ -162,7 +162,7 @@ private fun VideoQualityChoices( horizontalAlignment = Alignment.CenterHorizontally, ) { TextButton(colors = colors, onClick = onSelectAuto) { - val suffix = currentHeight?.let { " (${it}p)" } ?: "" + val suffix = currentShortSide?.let { " (${it}p)" } ?: "" Text( stringRes(R.string.video_quality_auto) + suffix, fontWeight = if (isAuto) FontWeight(1000) else FontWeight(400), @@ -172,17 +172,20 @@ private fun VideoQualityChoices( choices.forEach { choice -> TextButton(colors = colors, onClick = { onSelectTrack(choice.trackIndex) }) { Text( - "${choice.height}p ${formatBitrate(choice.bitrate)}", - fontWeight = if (!isAuto && currentHeight == choice.height) FontWeight(1000) else FontWeight(400), + "${choice.shortSide}p ${formatBitrate(choice.bitrate)}", + fontWeight = if (!isAuto && currentShortSide == choice.shortSide) FontWeight(1000) else FontWeight(400), ) } } } } +// shortSide = min(width, height). Matches the streaming convention that "360p" means +// 360 pixels on the short side regardless of orientation, so portrait videos get sensible +// labels instead of "640p / 960p / 1280p" for the same ladder rungs. private data class QualityChoice( val trackIndex: Int, - val height: Int, + val shortSide: Int, val bitrate: Int, ) @@ -190,11 +193,12 @@ private fun buildQualityChoices(group: Tracks.Group): ImmutableList() for (i in 0 until group.length) { val format = group.getTrackFormat(i) - if (format.height > 0) { - choices.add(QualityChoice(i, format.height, format.bitrate)) + val shortSide = minOf(format.width, format.height) + if (shortSide > 0) { + choices.add(QualityChoice(i, shortSide, format.bitrate)) } } - return choices.sortedByDescending { it.height }.toImmutableList() + return choices.sortedByDescending { it.shortSide }.toImmutableList() } private fun formatBitrate(bitrate: Int): String =