mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-06 11:48:24 +00:00
feat(video): cap the rendition viewport on metered connections; fix a PiP race
Follow-up to @davotoula's review on #4028. Sizing the ladder to the player is right on wifi, but it removed the app's only bandwidth lever and put nothing back: on mobile data a full-width card would pull most of the ladder, with only the ConnectivityType autoplay gate — which decides whether to play, not how much to pull — standing between a scroll and the data bill. Clamp the pushed viewport to a 480px short side while isMobileOrMeteredConnection is true, preserving aspect so the viewport still describes the player's shape. It stays one lever at the single setViewportSize call rather than a policy per call site, and it keeps the "quality proportional to the player" behaviour on wifi. Connectivity changes do not relayout, so a LaunchedEffect re-pushes with the last measured size when the ceiling flips; before the first measurement the existing zero-size guard makes that a no-op. Also from the same review: processIntentForPiP calls enterPictureInPictureMode from composition (PiPFromIntents), so the first layout pass can measure the activity at full screen before the window shrinks, handing the selector a full-screen viewport for the opening seconds of a PiP that is a few inches wide. RenderPipVideo now withholds the push until isInPictureInPictureMode is true; the shrink relayouts and pushes the real size. The pre-T makeBasic() path still wants an on-device look. clampViewportShortSide rounds up so a rounding artifact can never ask for 479 and drop a rung that sits exactly at the cap. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SQs7TP2WeXNR8SwUNLgmUC
This commit is contained in:
+20
-1
@@ -28,6 +28,7 @@ import androidx.compose.foundation.layout.Box
|
||||
import androidx.compose.foundation.layout.fillMaxSize
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.DisposableEffect
|
||||
import androidx.compose.runtime.LaunchedEffect
|
||||
import androidx.compose.runtime.MutableState
|
||||
import androidx.compose.runtime.getValue
|
||||
import androidx.compose.runtime.mutableStateOf
|
||||
@@ -48,11 +49,13 @@ import com.vitorpamplona.amethyst.service.playback.composable.controls.BottomGra
|
||||
import com.vitorpamplona.amethyst.service.playback.composable.controls.FullscreenSwipeControlsState
|
||||
import com.vitorpamplona.amethyst.service.playback.composable.controls.FullscreenSwipeLevelIndicator
|
||||
import com.vitorpamplona.amethyst.service.playback.composable.controls.LogVideoQualitySelection
|
||||
import com.vitorpamplona.amethyst.service.playback.composable.controls.METERED_MAX_SHORT_SIDE_PX
|
||||
import com.vitorpamplona.amethyst.service.playback.composable.controls.RenderAnimatedBottomInfo
|
||||
import com.vitorpamplona.amethyst.service.playback.composable.controls.RenderCenterButtons
|
||||
import com.vitorpamplona.amethyst.service.playback.composable.controls.RenderTopButtons
|
||||
import com.vitorpamplona.amethyst.service.playback.composable.controls.TopGradientOverlay
|
||||
import com.vitorpamplona.amethyst.service.playback.composable.controls.applyViewportConstraint
|
||||
import com.vitorpamplona.amethyst.service.playback.composable.controls.clampViewportShortSide
|
||||
import com.vitorpamplona.amethyst.service.playback.composable.controls.fullscreenSwipeControls
|
||||
import com.vitorpamplona.amethyst.service.playback.composable.mediaitem.LoadedMediaItem
|
||||
import com.vitorpamplona.amethyst.service.playback.composable.mediaitem.isHlsMedia
|
||||
@@ -107,6 +110,12 @@ fun RenderVideoPlayer(
|
||||
// unnecessary recomposition of the whole player tree just to update a value that is only
|
||||
// ever read inside the onDoubleTap callback below.
|
||||
val containerWidth = remember { intArrayOf(0) }
|
||||
|
||||
// Last measured player size, kept out of snapshot state for the same reason as containerWidth:
|
||||
// it exists only so a connectivity flip can re-push the viewport without a layout pass.
|
||||
val lastMeasured = remember { intArrayOf(0, 0) }
|
||||
val isMetered by accountViewModel.settings.isMobileOrMeteredConnection.collectAsStateWithLifecycle()
|
||||
val viewportCeiling = if (isMetered) METERED_MAX_SHORT_SIDE_PX else 0
|
||||
val isLive = remember(mediaItem.src.videoUri, mediaItem.src.mimeType) { isHlsMedia(mediaItem.src.videoUri, mediaItem.src.mimeType) }
|
||||
|
||||
val swipeState = remember { FullscreenSwipeControlsState() }
|
||||
@@ -135,6 +144,13 @@ fun RenderVideoPlayer(
|
||||
WatchPlaybackErrors(controllerState)
|
||||
LogVideoQualitySelection(controllerState.controller)
|
||||
|
||||
// Moving on or off mobile data does not relayout, so the new ceiling has to be pushed by hand.
|
||||
// Before the first measurement lastMeasured is still zero and applyViewportConstraint no-ops.
|
||||
LaunchedEffect(viewportCeiling, controllerState.controller) {
|
||||
val (w, h) = clampViewportShortSide(lastMeasured[0], lastMeasured[1], viewportCeiling)
|
||||
applyViewportConstraint(controllerState.controller, w, h)
|
||||
}
|
||||
|
||||
// Audio files have no video dimensions, so without this the player collapses to a thin strip and
|
||||
// the controls get crammed. Size it square (capped) so the visualizer and controls get room.
|
||||
// Voice notes keep their seek-bar strip; the full-screen dialog fills the screen.
|
||||
@@ -153,7 +169,10 @@ fun RenderVideoPlayer(
|
||||
playerModifier
|
||||
.onSizeChanged {
|
||||
containerWidth[0] = it.width
|
||||
applyViewportConstraint(controllerState.controller, it.width, it.height)
|
||||
lastMeasured[0] = it.width
|
||||
lastMeasured[1] = it.height
|
||||
val (w, h) = clampViewportShortSide(it.width, it.height, viewportCeiling)
|
||||
applyViewportConstraint(controllerState.controller, w, h)
|
||||
}.pointerInput(isLive, controllerState) {
|
||||
detectTapGestures(
|
||||
onTap = { controllerVisible.value = !controllerVisible.value },
|
||||
|
||||
+39
-1
@@ -33,6 +33,7 @@ import androidx.media3.common.util.UnstableApi
|
||||
import com.vitorpamplona.amethyst.service.playback.PLAYBACK_DIAG_TAG
|
||||
import com.vitorpamplona.quartz.utils.Log
|
||||
import com.vitorpamplona.quartz.utils.LogLevel
|
||||
import kotlin.math.ceil
|
||||
|
||||
internal fun getVideoTrackGroup(tracks: Tracks): Tracks.Group? = tracks.groups.firstOrNull { it.type == C.TRACK_TYPE_VIDEO && it.length > 0 }
|
||||
|
||||
@@ -54,7 +55,44 @@ internal fun getVideoTrackGroup(tracks: Tracks): Tracks.Group? = tracks.groups.f
|
||||
* away. A manual pick from the quality menu still wins — overrides are re-applied after
|
||||
* constraint-based selection runs.
|
||||
*/
|
||||
internal fun Modifier.constrainVideoQualityToViewport(player: Player): Modifier = onSizeChanged { applyViewportConstraint(player, it.width, it.height) }
|
||||
internal fun Modifier.constrainVideoQualityToViewport(
|
||||
player: Player,
|
||||
shouldApply: () -> Boolean = { true },
|
||||
): Modifier =
|
||||
onSizeChanged {
|
||||
// Evaluated at measure time, not at composition: a caller whose window is still resizing
|
||||
// (PiP) needs the answer for *this* layout pass.
|
||||
if (shouldApply()) applyViewportConstraint(player, it.width, it.height)
|
||||
}
|
||||
|
||||
/**
|
||||
* Short side the viewport is capped to on a metered connection.
|
||||
*
|
||||
* Sizing the ladder to the player is the right default on wifi, but on mobile data it would hand a
|
||||
* full-width card most of the ladder with nothing holding it back — the app's other lever is the
|
||||
* autoplay `ConnectivityType` gate, which decides *whether* to play, not how much to pull. 480 on
|
||||
* the short side keeps a card watchable while staying near the rung the old fixed-lowest policy
|
||||
* would have picked.
|
||||
*/
|
||||
const val METERED_MAX_SHORT_SIDE_PX = 480
|
||||
|
||||
/**
|
||||
* Scales a measured player size down until its short side fits [maxShortSidePx], preserving aspect
|
||||
* so the viewport still describes the shape of the player and not just its area. Sizes already
|
||||
* within the cap, and a non-positive cap, pass through untouched.
|
||||
*/
|
||||
internal fun clampViewportShortSide(
|
||||
widthPx: Int,
|
||||
heightPx: Int,
|
||||
maxShortSidePx: Int,
|
||||
): Pair<Int, Int> {
|
||||
val shortSide = minOf(widthPx, heightPx)
|
||||
if (maxShortSidePx <= 0 || shortSide <= 0 || shortSide <= maxShortSidePx) return widthPx to heightPx
|
||||
|
||||
val scale = maxShortSidePx.toDouble() / shortSide
|
||||
// Round up so the cap is never undershot into a lower rung by a rounding artifact.
|
||||
return ceil(widthPx * scale).toInt() to ceil(heightPx * scale).toInt()
|
||||
}
|
||||
|
||||
// Runs from onSizeChanged on the player's application looper (main thread), which is where
|
||||
// trackSelectionParameters must be written. Writing them re-runs track selection and, for a
|
||||
|
||||
+14
-1
@@ -20,6 +20,7 @@
|
||||
*/
|
||||
package com.vitorpamplona.amethyst.service.playback.pip
|
||||
|
||||
import androidx.activity.compose.LocalActivity
|
||||
import androidx.annotation.OptIn
|
||||
import androidx.compose.foundation.layout.Box
|
||||
import androidx.compose.foundation.layout.Row
|
||||
@@ -70,7 +71,19 @@ fun RenderPipVideo(
|
||||
}
|
||||
}
|
||||
|
||||
Box(modifier.constrainVideoQualityToViewport(controller.controller), contentAlignment = Alignment.Center) {
|
||||
// processIntentForPiP calls enterPictureInPictureMode from composition (PiPFromIntents), so the
|
||||
// first layout pass can measure the activity at full screen before the window shrinks. Pushing
|
||||
// that size would hand the selector a full-screen viewport for the first seconds of a PiP that
|
||||
// is a few inches wide; the shrink relayouts and pushes the real size.
|
||||
val activity = LocalActivity.current
|
||||
|
||||
Box(
|
||||
modifier.constrainVideoQualityToViewport(
|
||||
player = controller.controller,
|
||||
shouldApply = { activity?.isInPictureInPictureMode == true },
|
||||
),
|
||||
contentAlignment = Alignment.Center,
|
||||
) {
|
||||
ContentFrame(
|
||||
player = controller.controller,
|
||||
keepContentOnReset = true,
|
||||
|
||||
+36
@@ -20,6 +20,7 @@
|
||||
*/
|
||||
package com.vitorpamplona.amethyst.service.playback.composable.controls
|
||||
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
@@ -61,6 +62,41 @@ class ViewportQualityConstraintTest {
|
||||
assertFalse(needsViewportUpdate(1080, 1920, 0, 1920))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun leavesAnUncappedViewportAlone() {
|
||||
// ceiling 0 is the wifi case: the measured size is the viewport.
|
||||
assertEquals(1080 to 1920, clampViewportShortSide(1080, 1920, 0))
|
||||
// Already inside the cap.
|
||||
assertEquals(360 to 640, clampViewportShortSide(360, 640, METERED_MAX_SHORT_SIDE_PX))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun scalesAMeteredViewportDownByItsShortSide() {
|
||||
// A full-width portrait card on a 3x phone: the short side is what the cap addresses, and
|
||||
// the aspect has to survive so the viewport still describes the player's shape.
|
||||
val (w, h) = clampViewportShortSide(1080, 1920, METERED_MAX_SHORT_SIDE_PX)
|
||||
assertEquals(METERED_MAX_SHORT_SIDE_PX, w)
|
||||
assertEquals(854, h)
|
||||
|
||||
// Landscape: height is the short side, so that is what gets pinned to the cap.
|
||||
val (lw, lh) = clampViewportShortSide(1920, 1080, METERED_MAX_SHORT_SIDE_PX)
|
||||
assertEquals(METERED_MAX_SHORT_SIDE_PX, lh)
|
||||
assertEquals(854, lw)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun neverRoundsAMeteredViewportBelowTheCap() {
|
||||
// Rounding down here would ask for 479 on the short side and drop a ladder whose rung sits
|
||||
// exactly at 480 — the cap is a floor for the chosen rung, not a target to undershoot.
|
||||
val (_, h) = clampViewportShortSide(481, 641, METERED_MAX_SHORT_SIDE_PX)
|
||||
assertTrue(h >= METERED_MAX_SHORT_SIDE_PX)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun ignoresAnUnmeasuredSizeWhenClamping() {
|
||||
assertEquals(0 to 0, clampViewportShortSide(0, 0, METERED_MAX_SHORT_SIDE_PX))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun ignoresANegativeSize() {
|
||||
assertFalse(needsViewportUpdate(1080, 1920, -1, -1))
|
||||
|
||||
Reference in New Issue
Block a user