Skip to content

Commit

Permalink
Refactor VideoPlayer to be less exposed to EventChannel & related (
Browse files Browse the repository at this point in the history
…#6908)

Part of flutter/flutter#148417.

I'm working on re-landing #6456, this time without using the `ActivityAware` interface (see flutter/flutter#148417). As part of that work, I'll need to better control the `ExoPlayer` lifecycle and save/restore internal state.

These are some proposed refactors to limit how much work `VideoPlayer` is doing, so I can better understand what needs to be reset (or not) internally. Specifically, `VideoPlayer` no longer knows what an `EventChannel` or `EventSink` is, and does not need to manage the lifecycle (it stores a `private final VideoPlayerCallbacks` instead), and instead there is a `VideoPlayerCallbacks` interface that does all that.

I'm totally open to:
- Landing this as-is (+/- nits) and making minor improvements in follow-up PRs
- Making more significant changes to this PR and then landing it
- Not landing this PR at all because it doesn't follow the approach the folks who maintain the plugin prefer

Also happy to chat in VC/person about any of the changes.
  • Loading branch information
matanlurey committed Jun 13, 2024
1 parent 0891835 commit 1ad8d89
Show file tree
Hide file tree
Showing 5 changed files with 256 additions and 132 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,7 @@
import androidx.media3.datasource.DefaultHttpDataSource;
import androidx.media3.exoplayer.ExoPlayer;
import androidx.media3.exoplayer.source.DefaultMediaSourceFactory;
import io.flutter.plugin.common.EventChannel;
import io.flutter.view.TextureRegistry;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

final class VideoPlayer {
Expand All @@ -48,9 +43,7 @@ final class VideoPlayer {

private final TextureRegistry.SurfaceTextureEntry textureEntry;

private QueuingEventSink eventSink;

private final EventChannel eventChannel;
private final VideoPlayerCallbacks videoPlayerEvents;

private static final String USER_AGENT = "User-Agent";

Expand All @@ -62,13 +55,13 @@ final class VideoPlayer {

VideoPlayer(
Context context,
EventChannel eventChannel,
VideoPlayerCallbacks events,
TextureRegistry.SurfaceTextureEntry textureEntry,
String dataSource,
String formatHint,
@NonNull Map<String, String> httpHeaders,
VideoPlayerOptions options) {
this.eventChannel = eventChannel;
this.videoPlayerEvents = events;
this.textureEntry = textureEntry;
this.options = options;

Expand All @@ -86,24 +79,23 @@ final class VideoPlayer {
exoPlayer.setMediaItem(mediaItem);
exoPlayer.prepare();

setUpVideoPlayer(exoPlayer, new QueuingEventSink());
setUpVideoPlayer(exoPlayer);
}

// Constructor used to directly test members of this class.
@VisibleForTesting
VideoPlayer(
ExoPlayer exoPlayer,
EventChannel eventChannel,
VideoPlayerCallbacks events,
TextureRegistry.SurfaceTextureEntry textureEntry,
VideoPlayerOptions options,
QueuingEventSink eventSink,
DefaultHttpDataSource.Factory httpDataSourceFactory) {
this.eventChannel = eventChannel;
this.videoPlayerEvents = events;
this.textureEntry = textureEntry;
this.options = options;
this.httpDataSourceFactory = httpDataSourceFactory;

setUpVideoPlayer(exoPlayer, eventSink);
setUpVideoPlayer(exoPlayer);
}

@VisibleForTesting
Expand All @@ -118,37 +110,29 @@ public void configureHttpDataSourceFactory(@NonNull Map<String, String> httpHead
httpDataSourceFactory, httpHeaders, userAgent, httpHeadersNotEmpty);
}

private void setUpVideoPlayer(ExoPlayer exoPlayer, QueuingEventSink eventSink) {
private void setUpVideoPlayer(ExoPlayer exoPlayer) {
this.exoPlayer = exoPlayer;
this.eventSink = eventSink;

eventChannel.setStreamHandler(
new EventChannel.StreamHandler() {
@Override
public void onListen(Object o, EventChannel.EventSink sink) {
eventSink.setDelegate(sink);
}

@Override
public void onCancel(Object o) {
eventSink.setDelegate(null);
}
});

surface = new Surface(textureEntry.surfaceTexture());
exoPlayer.setVideoSurface(surface);
setAudioAttributes(exoPlayer, options.mixWithOthers);

// Avoids synthetic accessor.
VideoPlayerCallbacks events = this.videoPlayerEvents;

exoPlayer.addListener(
new Listener() {
private boolean isBuffering = false;

public void setBuffering(boolean buffering) {
if (isBuffering != buffering) {
isBuffering = buffering;
Map<String, Object> event = new HashMap<>();
event.put("event", isBuffering ? "bufferingStart" : "bufferingEnd");
eventSink.success(event);
if (isBuffering == buffering) {
return;
}
isBuffering = buffering;
if (buffering) {
events.onBufferingStart();
} else {
events.onBufferingEnd();
}
}

Expand All @@ -163,11 +147,8 @@ public void onPlaybackStateChanged(final int playbackState) {
sendInitialized();
}
} else if (playbackState == Player.STATE_ENDED) {
Map<String, Object> event = new HashMap<>();
event.put("event", "completed");
eventSink.success(event);
events.onCompleted();
}

if (playbackState != Player.STATE_BUFFERING) {
setBuffering(false);
}
Expand All @@ -180,30 +161,20 @@ public void onPlayerError(@NonNull final PlaybackException error) {
// See https://exoplayer.dev/live-streaming.html#behindlivewindowexception-and-error_code_behind_live_window
exoPlayer.seekToDefaultPosition();
exoPlayer.prepare();
} else if (eventSink != null) {
eventSink.error("VideoError", "Video player had error " + error, null);
} else {
events.onError("VideoError", "Video player had error " + error, null);
}
}

@Override
public void onIsPlayingChanged(boolean isPlaying) {
if (eventSink != null) {
Map<String, Object> event = new HashMap<>();
event.put("event", "isPlayingStateUpdate");
event.put("isPlaying", isPlaying);
eventSink.success(event);
}
events.onIsPlayingStateUpdate(isPlaying);
}
});
}

void sendBufferingUpdate() {
Map<String, Object> event = new HashMap<>();
event.put("event", "bufferingUpdate");
List<? extends Number> range = Arrays.asList(0, exoPlayer.getBufferedPosition());
// iOS supports a list of buffered ranges, so here is a list with a single range.
event.put("values", Collections.singletonList(range));
eventSink.success(event);
videoPlayerEvents.onBufferingUpdate(exoPlayer.getBufferedPosition());
}

private static void setAudioAttributes(ExoPlayer exoPlayer, boolean isMixMode) {
Expand Down Expand Up @@ -248,43 +219,36 @@ long getPosition() {
@SuppressWarnings("SuspiciousNameCombination")
@VisibleForTesting
void sendInitialized() {
if (isInitialized) {
Map<String, Object> event = new HashMap<>();
event.put("event", "initialized");
event.put("duration", exoPlayer.getDuration());

VideoSize videoSize = exoPlayer.getVideoSize();
int width = videoSize.width;
int height = videoSize.height;
if (width != 0 && height != 0) {
int rotationDegrees = videoSize.unappliedRotationDegrees;
// Switch the width/height if video was taken in portrait mode
if (rotationDegrees == 90 || rotationDegrees == 270) {
width = videoSize.height;
height = videoSize.width;
}
event.put("width", width);
event.put("height", height);

// Rotating the video with ExoPlayer does not seem to be possible with a Surface,
// so inform the Flutter code that the widget needs to be rotated to prevent
// upside-down playback for videos with rotationDegrees of 180 (other orientations work
// correctly without correction).
if (rotationDegrees == 180) {
event.put("rotationCorrection", rotationDegrees);
}
if (!isInitialized) {
return;
}
VideoSize videoSize = exoPlayer.getVideoSize();
int rotationCorrection = 0;
int width = videoSize.width;
int height = videoSize.height;
if (width != 0 && height != 0) {
int rotationDegrees = videoSize.unappliedRotationDegrees;
// Switch the width/height if video was taken in portrait mode
if (rotationDegrees == 90 || rotationDegrees == 270) {
width = videoSize.height;
height = videoSize.width;
}
// Rotating the video with ExoPlayer does not seem to be possible with a Surface,
// so inform the Flutter code that the widget needs to be rotated to prevent
// upside-down playback for videos with rotationDegrees of 180 (other orientations work
// correctly without correction).
if (rotationDegrees == 180) {
rotationCorrection = rotationDegrees;
}

eventSink.success(event);
}
videoPlayerEvents.onInitialized(width, height, exoPlayer.getDuration(), rotationCorrection);
}

void dispose() {
if (isInitialized) {
exoPlayer.stop();
}
textureEntry.release();
eventChannel.setStreamHandler(null);
if (surface != null) {
surface.release();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

package io.flutter.plugins.videoplayer;

import androidx.annotation.NonNull;
import androidx.annotation.Nullable;

/**
* Callbacks representing events invoked by {@link VideoPlayer}.
*
* <p>In the actual plugin, this will always be {@link VideoPlayerEventCallbacks}, which creates the
* expected events to send back through the plugin channel. In tests methods can be overridden in
* order to assert results.
*
* <p>See {@link androidx.media3.common.Player.Listener} for details.
*/
interface VideoPlayerCallbacks {
void onInitialized(int width, int height, long durationInMs, int rotationCorrectionInDegrees);

void onBufferingStart();

void onBufferingUpdate(long bufferedPosition);

void onBufferingEnd();

void onCompleted();

void onError(@NonNull String code, @Nullable String message, @Nullable Object details);

void onIsPlayingStateUpdate(boolean isPlaying);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

package io.flutter.plugins.videoplayer;

import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;
import io.flutter.plugin.common.EventChannel;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;

final class VideoPlayerEventCallbacks implements VideoPlayerCallbacks {
private final EventChannel.EventSink eventSink;

static VideoPlayerEventCallbacks bindTo(EventChannel eventChannel) {
QueuingEventSink eventSink = new QueuingEventSink();
eventChannel.setStreamHandler(
new EventChannel.StreamHandler() {
@Override
public void onListen(Object arguments, EventChannel.EventSink events) {
eventSink.setDelegate(events);
}

@Override
public void onCancel(Object arguments) {
eventSink.setDelegate(null);
}
});
return VideoPlayerEventCallbacks.withSink(eventSink);
}

@VisibleForTesting
static VideoPlayerEventCallbacks withSink(EventChannel.EventSink eventSink) {
return new VideoPlayerEventCallbacks(eventSink);
}

private VideoPlayerEventCallbacks(EventChannel.EventSink eventSink) {
this.eventSink = eventSink;
}

@Override
public void onInitialized(
int width, int height, long durationInMs, int rotationCorrectionInDegrees) {
Map<String, Object> event = new HashMap<>();
event.put("event", "initialized");
event.put("width", width);
event.put("height", height);
event.put("duration", durationInMs);
if (rotationCorrectionInDegrees != 0) {
event.put("rotationCorrection", rotationCorrectionInDegrees);
}
eventSink.success(event);
}

@Override
public void onBufferingStart() {
Map<String, Object> event = new HashMap<>();
event.put("event", "bufferingStart");
eventSink.success(event);
}

@Override
public void onBufferingUpdate(long bufferedPosition) {
// iOS supports a list of buffered ranges, so we send as a list with a single range.
Map<String, Object> event = new HashMap<>();
event.put("values", Collections.singletonList(bufferedPosition));
eventSink.success(event);
}

@Override
public void onBufferingEnd() {
Map<String, Object> event = new HashMap<>();
event.put("event", "bufferingEnd");
eventSink.success(event);
}

@Override
public void onCompleted() {
Map<String, Object> event = new HashMap<>();
event.put("event", "completed");
eventSink.success(event);
}

@Override
public void onError(@NonNull String code, @Nullable String message, @Nullable Object details) {
eventSink.error(code, message, details);
}

@Override
public void onIsPlayingStateUpdate(boolean isPlaying) {
Map<String, Object> event = new HashMap<>();
event.put("isPlaying", isPlaying);
eventSink.success(event);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ public void initialize() {
player =
new VideoPlayer(
flutterState.applicationContext,
eventChannel,
VideoPlayerEventCallbacks.bindTo(eventChannel),
handle,
"asset:///" + assetLookupKey,
null,
Expand All @@ -136,7 +136,7 @@ public void initialize() {
player =
new VideoPlayer(
flutterState.applicationContext,
eventChannel,
VideoPlayerEventCallbacks.bindTo(eventChannel),
handle,
arg.getUri(),
arg.getFormatHint(),
Expand Down
Loading

0 comments on commit 1ad8d89

Please sign in to comment.