Skip to content

Commit 5c52c55

Browse files
[video_player] Move Android buffer updates to Dart (#9771)
Since ExoPlayer does not generate events for changes to the buffer position, the `video_player_android` synthesizes them via polling. This moves the logic to generate these events from Java to Dart, to reduce the amount of logic on the Java side. This does not attempt to fix the structural problem that the workaround was piggy-backed on the fact that the app-facing package polls frequently for the playback position, generating buffer update events when that polling happens rather than via any independent internal polling system. This approach is not only fragile, as it depends on logic defined in another package, but already doesn't work in the case of a paused video. However, this refactoring will make it possible to change this behavior in Dart code in a follow-up focused on that behavioral change. This change should be behavior-neutral, except that it adds the simple improvement of not creating events if the buffer position hasn't changed, so that clients aren't getting a stream of useless buffer position events during playback. This version will also be more efficient from a platform channel standpoint, since instead of sending a separate message with the buffer position, it's returned in the same call as the playback position. To support synthesizing events on the Dart side, this adds a custom stream controller in Dart, instead of creating it directly from the EventChannel stream; this change will make future changes to a non-method-channel approach (e.g., FFI callbacks) straightforward. As this adds non-trivial additional per-player-instance state on the Dart side, this adds a Dart player instance class, and consolidates existing state into that class, including passthrough methods for the API calls, making it clearer which state is plugin level vs player level. Part of flutter/flutter#172763 ## Pre-Review Checklist **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
1 parent f53df16 commit 5c52c55

File tree

10 files changed

+445
-146
lines changed

10 files changed

+445
-146
lines changed

packages/video_player/video_player_android/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
## 2.8.12
2+
3+
* Moves buffer position update event generation to Dart.
4+
15
## 2.8.11
26

37
* Updates kotlin version to 2.2.0 to enable gradle 8.11 support.

packages/video_player/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/Messages.java

Lines changed: 120 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,100 @@ ArrayList<Object> toList() {
324324
}
325325
}
326326

327+
/** Generated class from Pigeon that represents data sent in messages. */
328+
public static final class PlaybackState {
329+
/** The current playback position, in milliseconds. */
330+
private @NonNull Long playPosition;
331+
332+
public @NonNull Long getPlayPosition() {
333+
return playPosition;
334+
}
335+
336+
public void setPlayPosition(@NonNull Long setterArg) {
337+
if (setterArg == null) {
338+
throw new IllegalStateException("Nonnull field \"playPosition\" is null.");
339+
}
340+
this.playPosition = setterArg;
341+
}
342+
343+
/** The current buffer position, in milliseconds. */
344+
private @NonNull Long bufferPosition;
345+
346+
public @NonNull Long getBufferPosition() {
347+
return bufferPosition;
348+
}
349+
350+
public void setBufferPosition(@NonNull Long setterArg) {
351+
if (setterArg == null) {
352+
throw new IllegalStateException("Nonnull field \"bufferPosition\" is null.");
353+
}
354+
this.bufferPosition = setterArg;
355+
}
356+
357+
/** Constructor is non-public to enforce null safety; use Builder. */
358+
PlaybackState() {}
359+
360+
@Override
361+
public boolean equals(Object o) {
362+
if (this == o) {
363+
return true;
364+
}
365+
if (o == null || getClass() != o.getClass()) {
366+
return false;
367+
}
368+
PlaybackState that = (PlaybackState) o;
369+
return playPosition.equals(that.playPosition) && bufferPosition.equals(that.bufferPosition);
370+
}
371+
372+
@Override
373+
public int hashCode() {
374+
return Objects.hash(playPosition, bufferPosition);
375+
}
376+
377+
public static final class Builder {
378+
379+
private @Nullable Long playPosition;
380+
381+
@CanIgnoreReturnValue
382+
public @NonNull Builder setPlayPosition(@NonNull Long setterArg) {
383+
this.playPosition = setterArg;
384+
return this;
385+
}
386+
387+
private @Nullable Long bufferPosition;
388+
389+
@CanIgnoreReturnValue
390+
public @NonNull Builder setBufferPosition(@NonNull Long setterArg) {
391+
this.bufferPosition = setterArg;
392+
return this;
393+
}
394+
395+
public @NonNull PlaybackState build() {
396+
PlaybackState pigeonReturn = new PlaybackState();
397+
pigeonReturn.setPlayPosition(playPosition);
398+
pigeonReturn.setBufferPosition(bufferPosition);
399+
return pigeonReturn;
400+
}
401+
}
402+
403+
@NonNull
404+
ArrayList<Object> toList() {
405+
ArrayList<Object> toListResult = new ArrayList<>(2);
406+
toListResult.add(playPosition);
407+
toListResult.add(bufferPosition);
408+
return toListResult;
409+
}
410+
411+
static @NonNull PlaybackState fromList(@NonNull ArrayList<Object> pigeonVar_list) {
412+
PlaybackState pigeonResult = new PlaybackState();
413+
Object playPosition = pigeonVar_list.get(0);
414+
pigeonResult.setPlayPosition((Long) playPosition);
415+
Object bufferPosition = pigeonVar_list.get(1);
416+
pigeonResult.setBufferPosition((Long) bufferPosition);
417+
return pigeonResult;
418+
}
419+
}
420+
327421
private static class PigeonCodec extends StandardMessageCodec {
328422
public static final PigeonCodec INSTANCE = new PigeonCodec();
329423

@@ -346,6 +440,8 @@ protected Object readValueOfType(byte type, @NonNull ByteBuffer buffer) {
346440
return PlatformVideoViewCreationParams.fromList((ArrayList<Object>) readValue(buffer));
347441
case (byte) 132:
348442
return CreateMessage.fromList((ArrayList<Object>) readValue(buffer));
443+
case (byte) 133:
444+
return PlaybackState.fromList((ArrayList<Object>) readValue(buffer));
349445
default:
350446
return super.readValueOfType(type, buffer);
351447
}
@@ -365,6 +461,9 @@ protected void writeValue(@NonNull ByteArrayOutputStream stream, Object value) {
365461
} else if (value instanceof CreateMessage) {
366462
stream.write(132);
367463
writeValue(stream, ((CreateMessage) value).toList());
464+
} else if (value instanceof PlaybackState) {
465+
stream.write(133);
466+
writeValue(stream, ((PlaybackState) value).toList());
368467
} else {
369468
super.writeValue(stream, value);
370469
}
@@ -532,21 +631,26 @@ static void setUp(
532631
}
533632
/** Generated interface from Pigeon that represents a handler of messages from Flutter. */
534633
public interface VideoPlayerInstanceApi {
535-
634+
/** Sets whether to automatically loop playback of the video. */
536635
void setLooping(@NonNull Boolean looping);
537-
636+
/** Sets the volume, with 0.0 being muted and 1.0 being full volume. */
538637
void setVolume(@NonNull Double volume);
539-
638+
/** Sets the playback speed as a multiple of normal speed. */
540639
void setPlaybackSpeed(@NonNull Double speed);
541-
640+
/** Begins playback if the video is not currently playing. */
542641
void play();
543-
544-
@NonNull
545-
Long getPosition();
546-
547-
void seekTo(@NonNull Long position);
548-
642+
/** Pauses playback if the video is currently playing. */
549643
void pause();
644+
/** Seeks to the given playback position, in milliseconds. */
645+
void seekTo(@NonNull Long position);
646+
/**
647+
* Returns the current playback state.
648+
*
649+
* <p>This is combined into a single call to minimize platform channel calls for state that
650+
* needs to be polled frequently.
651+
*/
652+
@NonNull
653+
PlaybackState getPlaybackState();
550654

551655
/** The codec used by VideoPlayerInstanceApi. */
552656
static @NonNull MessageCodec<Object> getCodec() {
@@ -668,16 +772,16 @@ static void setUp(
668772
BasicMessageChannel<Object> channel =
669773
new BasicMessageChannel<>(
670774
binaryMessenger,
671-
"dev.flutter.pigeon.video_player_android.VideoPlayerInstanceApi.getPosition"
775+
"dev.flutter.pigeon.video_player_android.VideoPlayerInstanceApi.pause"
672776
+ messageChannelSuffix,
673777
getCodec());
674778
if (api != null) {
675779
channel.setMessageHandler(
676780
(message, reply) -> {
677781
ArrayList<Object> wrapped = new ArrayList<>();
678782
try {
679-
Long output = api.getPosition();
680-
wrapped.add(0, output);
783+
api.pause();
784+
wrapped.add(0, null);
681785
} catch (Throwable exception) {
682786
wrapped = wrapError(exception);
683787
}
@@ -716,16 +820,16 @@ static void setUp(
716820
BasicMessageChannel<Object> channel =
717821
new BasicMessageChannel<>(
718822
binaryMessenger,
719-
"dev.flutter.pigeon.video_player_android.VideoPlayerInstanceApi.pause"
823+
"dev.flutter.pigeon.video_player_android.VideoPlayerInstanceApi.getPlaybackState"
720824
+ messageChannelSuffix,
721825
getCodec());
722826
if (api != null) {
723827
channel.setMessageHandler(
724828
(message, reply) -> {
725829
ArrayList<Object> wrapped = new ArrayList<>();
726830
try {
727-
api.pause();
728-
wrapped.add(0, null);
831+
PlaybackState output = api.getPlaybackState();
832+
wrapped.add(0, output);
729833
} catch (Throwable exception) {
730834
wrapped = wrapError(exception);
731835
}

packages/video_player/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,6 @@ public void setDisposeHandler(@Nullable DisposeHandler handler) {
6666
protected abstract ExoPlayerEventListener createExoPlayerEventListener(
6767
@NonNull ExoPlayer exoPlayer, @Nullable SurfaceProducer surfaceProducer);
6868

69-
void sendBufferingUpdate() {
70-
videoPlayerEvents.onBufferingUpdate(exoPlayer.getBufferedPosition());
71-
}
72-
7369
private static void setAudioAttributes(ExoPlayer exoPlayer, boolean isMixMode) {
7470
exoPlayer.setAudioAttributes(
7571
new AudioAttributes.Builder().setContentType(C.AUDIO_CONTENT_TYPE_MOVIE).build(),
@@ -107,12 +103,11 @@ public void setPlaybackSpeed(@NonNull Double speed) {
107103
}
108104

109105
@Override
110-
public @NonNull Long getPosition() {
111-
long position = exoPlayer.getCurrentPosition();
112-
// TODO(stuartmorgan): Move this; this is relying on the fact that getPosition is called
113-
// frequently to drive buffering updates, which is a fragile hack.
114-
sendBufferingUpdate();
115-
return position;
106+
public @NonNull Messages.PlaybackState getPlaybackState() {
107+
return new Messages.PlaybackState.Builder()
108+
.setPlayPosition(exoPlayer.getCurrentPosition())
109+
.setBufferPosition(exoPlayer.getBufferedPosition())
110+
.build();
116111
}
117112

118113
@Override

packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/VideoPlayerTest.java

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -126,17 +126,6 @@ public void playsAndPausesProvidedMedia() {
126126
videoPlayer.dispose();
127127
}
128128

129-
@Test
130-
public void sendsBufferingUpdatesOnDemand() {
131-
VideoPlayer videoPlayer = createVideoPlayer();
132-
133-
when(mockExoPlayer.getBufferedPosition()).thenReturn(10L);
134-
videoPlayer.sendBufferingUpdate();
135-
verify(mockEvents).onBufferingUpdate(10L);
136-
137-
videoPlayer.dispose();
138-
}
139-
140129
@Test
141130
public void togglesLoopingEnablesAndDisablesRepeatMode() {
142131
VideoPlayer videoPlayer = createVideoPlayer();
@@ -177,14 +166,29 @@ public void setPlaybackSpeedSetsPlaybackParametersWithValue() {
177166
}
178167

179168
@Test
180-
public void seekAndGetPosition() {
169+
public void seekTo() {
181170
VideoPlayer videoPlayer = createVideoPlayer();
182171

183172
videoPlayer.seekTo(10L);
184173
verify(mockExoPlayer).seekTo(10);
185174

186-
when(mockExoPlayer.getCurrentPosition()).thenReturn(20L);
187-
assertEquals(20L, videoPlayer.getPosition().longValue());
175+
videoPlayer.dispose();
176+
}
177+
178+
@Test
179+
public void getPlaybackState() {
180+
VideoPlayer videoPlayer = createVideoPlayer();
181+
182+
final long playbackPosition = 20L;
183+
final long bufferedPosition = 10L;
184+
when(mockExoPlayer.getCurrentPosition()).thenReturn(playbackPosition);
185+
when(mockExoPlayer.getBufferedPosition()).thenReturn(bufferedPosition);
186+
187+
final Messages.PlaybackState state = videoPlayer.getPlaybackState();
188+
assertEquals(playbackPosition, state.getPlayPosition().longValue());
189+
assertEquals(bufferedPosition, state.getBufferPosition().longValue());
190+
191+
videoPlayer.dispose();
188192
}
189193

190194
@Test

0 commit comments

Comments
 (0)