Skip to content

Commit 1ed7c54

Browse files
oceanjulescopybara-github
authored andcommitted
[ui-compose] Round up current position of ProgressStateWithTickInterval
Until this change, `currentPositionMs` and `bufferedPositionMs` were a direct value extracted from the underlying Player. This resulted in the consumer having to make decisions about how to round up the value for the final display (e.g. how to parse the milliseconds into the HH:MM:SS string). Whatever the user has provided as the interval is most likely the granularity they desire for the `currentPositionMs` and `bufferedPositionMs`. And while we attempt to poll the Player at exactly this interval, it is not always possible to obtain a precise value that is a multiple of the interval (e.g. falls directly on the grid of [0, interval, 2xinterval, 3xinterval ...]). This change ensures that the `currentPositionMs` and `bufferedPositionMs` returned by `ProgressStateWithTickInterval` is already rounded up correctly. Note that it will still act like a "stopwatch" and not jump to the next second/minute value until we are incredibly close to the boundary (i.e. not from 50% of the interval or 50% of the remaining minute). Instead, this boundary is defined at 10ms to account for any potential jitter in Player reporting its current position. PiperOrigin-RevId: 781159734
1 parent a6f1096 commit 1ed7c54

File tree

2 files changed

+64
-10
lines changed

2 files changed

+64
-10
lines changed

libraries/ui_compose/src/main/java/androidx/media3/ui/compose/state/ProgressStateWithTickInterval.kt

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ fun rememberProgressStateWithTickInterval(
8888
* to some UI element, the scope of the Composable will ensure the job is cancelled when the
8989
* element is disposed.
9090
* @property[currentPositionMs] The playback position in the current content or ad, in milliseconds,
91-
* matches [Player.getCurrentPosition].
91+
* matches [Player.getCurrentPosition] that is rounded to a multiple of [tickIntervalMs].
9292
* @property[bufferedPositionMs] An estimate of the position in the current content or ad up to
9393
* which data is buffered, in milliseconds.
9494
* @property[durationMs] The duration of the current content or ad in milliseconds, matches
@@ -100,10 +100,12 @@ class ProgressStateWithTickInterval(
100100
@IntRange(from = 0) private val tickIntervalMs: Long = 0,
101101
scope: CoroutineScope,
102102
) {
103-
var currentPositionMs by mutableLongStateOf(getCurrentPositionMsOrDefault(player))
103+
var currentPositionMs by
104+
mutableLongStateOf(snapPositionToNearestTick(player, ::getCurrentPositionMsOrDefault))
104105
private set
105106

106-
var bufferedPositionMs by mutableLongStateOf(getBufferedPositionMsOrDefault(player))
107+
var bufferedPositionMs by
108+
mutableLongStateOf(snapPositionToNearestTick(player, ::getBufferedPositionMsOrDefault))
107109
private set
108110

109111
var durationMs by mutableLongStateOf(getDurationMsOrDefault(player))
@@ -115,8 +117,8 @@ class ProgressStateWithTickInterval(
115117
scope,
116118
nextMediaTickMsSupplier = ::nextMediaWakeUpPositionMs,
117119
scheduledTask = {
118-
currentPositionMs = getCurrentPositionMsOrDefault(player)
119-
bufferedPositionMs = getBufferedPositionMsOrDefault(player)
120+
currentPositionMs = snapPositionToNearestTick(player, ::getCurrentPositionMsOrDefault)
121+
bufferedPositionMs = snapPositionToNearestTick(player, ::getBufferedPositionMsOrDefault)
120122
durationMs = getDurationMsOrDefault(player)
121123
},
122124
)
@@ -144,4 +146,22 @@ class ProgressStateWithTickInterval(
144146
}
145147
return nextMediaWakeUpPositionMs
146148
}
149+
150+
/**
151+
* Round the actual position to the nearest tick (i.e. integer number of tickIntervals). Rounding
152+
* happens in a stop-watch manner, i.e. always down - hence integer division.
153+
*
154+
* Note how this is different to [ProgressStateWithTickCount] rounding that takes half of the
155+
* interval into account to round up.
156+
*/
157+
private fun snapPositionToNearestTick(player: Player, positionSupplier: (Player) -> Long): Long {
158+
val actualPositionMs = positionSupplier(player)
159+
if (tickIntervalMs == 0L || actualPositionMs % tickIntervalMs == 0L) {
160+
return actualPositionMs
161+
}
162+
val adjustedCurrentTick = (actualPositionMs + POSITION_CORRECTION_OFFSET_MS) / tickIntervalMs
163+
return adjustedCurrentTick * tickIntervalMs
164+
}
147165
}
166+
167+
private const val POSITION_CORRECTION_OFFSET_MS = 10

libraries/ui_compose/src/test/java/androidx/media3/ui/compose/state/ProgressStateWithTickIntervalTest.kt

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,40 @@ class ProgressStateWithTickIntervalTest {
9898
assertThat(state.currentPositionMs).isEqualTo(2000)
9999
}
100100

101+
@Test
102+
fun progressUpdatingOnTheSecondMark_moveClockByFractionalSeconds_positionRoundsUpToTheGrid() =
103+
runTest(testDispatcher) {
104+
val player =
105+
TestPlayer(
106+
playWhenReady = true,
107+
playlist = listOf(MediaItemData.Builder("SingleItem").build()),
108+
)
109+
player.setPositionSupplierDrivenBy(testDispatcher.scheduler)
110+
lateinit var state: ProgressStateWithTickInterval
111+
composeTestRule.setContent {
112+
state =
113+
rememberProgressStateWithTickInterval(
114+
player,
115+
tickIntervalMs = 500,
116+
scope = backgroundScope,
117+
)
118+
}
119+
120+
advanceTimeByInclusive(1800.milliseconds)
121+
player.setDuration("SingleItem", 5000)
122+
composeTestRule.waitForIdle()
123+
124+
assertThat(player.currentPosition).isEqualTo(1800)
125+
assertThat(state.currentPositionMs).isEqualTo(1500)
126+
127+
advanceTimeByInclusive(195.milliseconds)
128+
player.setDuration("SingleItem", 7000)
129+
composeTestRule.waitForIdle()
130+
131+
assertThat(player.currentPosition).isEqualTo(1995)
132+
assertThat(state.currentPositionMs).isEqualTo(2000)
133+
}
134+
101135
@Test
102136
fun progressUpdatingContinuouslyEveryFrame_positionChangesByOneFrame() =
103137
runTest(testDispatcher) {
@@ -202,17 +236,17 @@ class ProgressStateWithTickIntervalTest {
202236
advanceTimeByInclusive(666.milliseconds)
203237

204238
assertThat(player.currentPosition).isEqualTo(999)
205-
assertThat(state.currentPositionMs).isEqualTo(999)
239+
assertThat(state.currentPositionMs).isEqualTo(1000)
206240

207241
advanceTimeByInclusive(400.milliseconds)
208242

209243
assertThat(player.currentPosition).isEqualTo(1599)
210-
assertThat(state.currentPositionMs).isEqualTo(999) // doesn't ever reach 1000
244+
assertThat(state.currentPositionMs).isEqualTo(1000)
211245

212246
advanceTimeByInclusive(267.milliseconds)
213247

214248
assertThat(player.currentPosition).isEqualTo(1999)
215-
assertThat(state.currentPositionMs).isEqualTo(1999) // first bump since 999
249+
assertThat(state.currentPositionMs).isEqualTo(2000)
216250

217251
advanceTimeByInclusive(667.milliseconds)
218252

@@ -321,7 +355,7 @@ class ProgressStateWithTickIntervalTest {
321355
state =
322356
rememberProgressStateWithTickInterval(
323357
player,
324-
tickIntervalMs = 5000,
358+
tickIntervalMs = 100,
325359
scope = backgroundScope,
326360
)
327361
}
@@ -337,7 +371,7 @@ class ProgressStateWithTickIntervalTest {
337371
advanceTimeByInclusive((PAUSED_UPDATE_INTERVAL_MS - 500).milliseconds)
338372

339373
assertThat(player.bufferedPosition).isEqualTo(123)
340-
assertThat(state.bufferedPositionMs).isEqualTo(123)
374+
assertThat(state.bufferedPositionMs).isEqualTo(100)
341375
}
342376

343377
/**

0 commit comments

Comments
 (0)