From cd44ecc77836d9fea95c4e91d71421171bfb6df3 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 21 Nov 2025 11:04:45 -0500 Subject: [PATCH 1/2] [video_player_avfoundation] Create a Dart player instance Refactors the Dart internals to create a pleyer instance class on the Dart side, and intermediates the event stream with an instance-managed stream controller that would allow for alternate event sources (e.g., Dart code). This prepares for future moves of more logic to Dart, and parallels Android Dart code changes made in https://github.com/flutter/packages/pull/9771 Part of https://github.com/flutter/flutter/issues/172763 --- .../video_player_avfoundation/CHANGELOG.md | 4 + .../FVPVideoPlayerPlugin.m | 2 +- .../lib/src/avfoundation_video_player.dart | 162 +++++++++++------- .../video_player_avfoundation/pubspec.yaml | 2 +- .../test/avfoundation_video_player_test.dart | 82 +++++---- 5 files changed, 151 insertions(+), 101 deletions(-) diff --git a/packages/video_player/video_player_avfoundation/CHANGELOG.md b/packages/video_player/video_player_avfoundation/CHANGELOG.md index 9ddb15d3a51..7f509fcc462 100644 --- a/packages/video_player/video_player_avfoundation/CHANGELOG.md +++ b/packages/video_player/video_player_avfoundation/CHANGELOG.md @@ -1,3 +1,7 @@ +## 2.8.8 + +* Refactors Dart internals for maintainability. + ## 2.8.7 * Updates to Pigeon 26. diff --git a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m index d1914d9f7c1..dbc130acc50 100644 --- a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m +++ b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m @@ -116,7 +116,7 @@ - (int64_t)configurePlayer:(FVPVideoPlayer *)player // Set up the event channel. FVPEventBridge *eventBridge = [[FVPEventBridge alloc] initWithMessenger:messenger - channelName:[NSString stringWithFormat:@"flutter.io/videoPlayer/videoEvents%@", + channelName:[NSString stringWithFormat:@"flutter.dev/videoPlayer/videoEvents%@", channelSuffix]]; player.eventListener = eventBridge; diff --git a/packages/video_player/video_player_avfoundation/lib/src/avfoundation_video_player.dart b/packages/video_player/video_player_avfoundation/lib/src/avfoundation_video_player.dart index 4c1719578f6..0bb2593bd35 100644 --- a/packages/video_player/video_player_avfoundation/lib/src/avfoundation_video_player.dart +++ b/packages/video_player/video_player_avfoundation/lib/src/avfoundation_video_player.dart @@ -22,23 +22,16 @@ class AVFoundationVideoPlayer extends VideoPlayerPlatform { AVFoundationVideoPlayer({ @visibleForTesting AVFoundationVideoPlayerApi? pluginApi, @visibleForTesting - VideoPlayerInstanceApi Function(int playerId)? playerProvider, + VideoPlayerInstanceApi Function(int playerId)? playerApiProvider, }) : _api = pluginApi ?? AVFoundationVideoPlayerApi(), - _playerProvider = playerProvider ?? _productionApiProvider; + _playerApiProvider = playerApiProvider ?? _productionApiProvider; final AVFoundationVideoPlayerApi _api; // A method to create VideoPlayerInstanceApi instances, which can be // overridden for testing. - final VideoPlayerInstanceApi Function(int mapId) _playerProvider; + final VideoPlayerInstanceApi Function(int mapId) _playerApiProvider; - /// A map that associates player ID with a view state. - /// This is used to determine which view type to use when building a view. - @visibleForTesting - final Map playerViewStates = - {}; - - final Map _players = - {}; + final Map _players = {}; /// Registers this class as the default instance of [VideoPlayerPlatform]. static void registerWith() { @@ -52,9 +45,8 @@ class AVFoundationVideoPlayer extends VideoPlayerPlatform { @override Future dispose(int playerId) async { - final VideoPlayerInstanceApi? player = _players.remove(playerId); + final _PlayerInstance? player = _players.remove(playerId); await player?.dispose(); - playerViewStates.remove(playerId); } @override @@ -118,8 +110,7 @@ class AVFoundationVideoPlayer extends VideoPlayerPlatform { playerId = await _api.createForPlatformView(pigeonCreationOptions); state = const VideoPlayerPlatformViewState(); } - playerViewStates[playerId] = state; - ensureApiInitialized(playerId); + ensurePlayerInitialized(playerId, state); return playerId; } @@ -127,9 +118,16 @@ class AVFoundationVideoPlayer extends VideoPlayerPlatform { /// Returns the API instance for [playerId], creating it if it doesn't already /// exist. @visibleForTesting - VideoPlayerInstanceApi ensureApiInitialized(int playerId) { - return _players.putIfAbsent(playerId, () { - return _playerProvider(playerId); + void ensurePlayerInitialized(int playerId, VideoPlayerViewState viewState) { + _players.putIfAbsent(playerId, () { + return _PlayerInstance( + _playerApiProvider(playerId), + viewState, + eventChannel: EventChannel( + // This must match the channel name used in FVPVideoPlayerPlugin.m. + 'flutter.dev/videoPlayer/videoEvents$playerId', + ), + ); }); } @@ -162,48 +160,17 @@ class AVFoundationVideoPlayer extends VideoPlayerPlatform { @override Future seekTo(int playerId, Duration position) { - return _playerWith(id: playerId).seekTo(position.inMilliseconds); + return _playerWith(id: playerId).seekTo(position); } @override Future getPosition(int playerId) async { - final int position = await _playerWith(id: playerId).getPosition(); - return Duration(milliseconds: position); + return _playerWith(id: playerId).getPosition(); } @override Stream videoEventsFor(int playerId) { - return _eventChannelFor(playerId).receiveBroadcastStream().map(( - dynamic event, - ) { - final Map map = event as Map; - return switch (map['event']) { - 'initialized' => VideoEvent( - eventType: VideoEventType.initialized, - duration: Duration(milliseconds: map['duration'] as int), - size: Size( - (map['width'] as num?)?.toDouble() ?? 0.0, - (map['height'] as num?)?.toDouble() ?? 0.0, - ), - ), - 'completed' => VideoEvent(eventType: VideoEventType.completed), - 'bufferingUpdate' => VideoEvent( - buffered: (map['values'] as List) - .map(_toDurationRange) - .toList(), - eventType: VideoEventType.bufferingUpdate, - ), - 'bufferingStart' => VideoEvent( - eventType: VideoEventType.bufferingStart, - ), - 'bufferingEnd' => VideoEvent(eventType: VideoEventType.bufferingEnd), - 'isPlayingStateUpdate' => VideoEvent( - eventType: VideoEventType.isPlayingStateUpdate, - isPlaying: map['isPlaying'] as bool, - ), - _ => VideoEvent(eventType: VideoEventType.unknown), - }; - }); + return _playerWith(id: playerId).videoEvents; } @override @@ -219,16 +186,13 @@ class AVFoundationVideoPlayer extends VideoPlayerPlatform { @override Widget buildViewWithOptions(VideoViewOptions options) { final int playerId = options.playerId; - final VideoPlayerViewState? viewState = playerViewStates[playerId]; + final VideoPlayerViewState viewState = _playerWith(id: playerId).viewState; return switch (viewState) { VideoPlayerTextureViewState(:final int textureId) => Texture( textureId: textureId, ), VideoPlayerPlatformViewState() => _buildPlatformView(playerId), - null => throw Exception( - 'Could not find corresponding view type for playerId: $playerId', - ), }; } @@ -246,13 +210,89 @@ class AVFoundationVideoPlayer extends VideoPlayerPlatform { ); } - EventChannel _eventChannelFor(int playerId) { - return EventChannel('flutter.io/videoPlayer/videoEvents$playerId'); + _PlayerInstance _playerWith({required int id}) { + final _PlayerInstance? player = _players[id]; + return player ?? (throw StateError('No active player with ID $id.')); } +} - VideoPlayerInstanceApi _playerWith({required int id}) { - final VideoPlayerInstanceApi? player = _players[id]; - return player ?? (throw StateError('No active player with ID $id.')); +/// An instance of a video player, corresponding to a single player ID in +/// [AVFoundationVideoPlayer]. +class _PlayerInstance { + _PlayerInstance( + this._api, + this.viewState, { + required EventChannel eventChannel, + }) : _eventChannel = eventChannel; + + final VideoPlayerInstanceApi _api; + final VideoPlayerViewState viewState; + final EventChannel _eventChannel; + final StreamController _eventStreamController = + StreamController(); + StreamSubscription? _eventSubscription; + + Future play() => _api.play(); + + Future pause() => _api.pause(); + + Future setLooping(bool looping) => _api.setLooping(looping); + + Future setVolume(double volume) => _api.setVolume(volume); + + Future setPlaybackSpeed(double speed) => _api.setPlaybackSpeed(speed); + + Future seekTo(Duration position) { + return _api.seekTo(position.inMilliseconds); + } + + Future getPosition() async { + return Duration(milliseconds: await _api.getPosition()); + } + + Stream get videoEvents { + _eventSubscription ??= _eventChannel.receiveBroadcastStream().listen( + _onStreamEvent, + onError: (Object e) { + _eventStreamController.addError(e); + }, + ); + + return _eventStreamController.stream; + } + + Future dispose() async { + await _eventSubscription?.cancel(); + await _api.dispose(); + } + + void _onStreamEvent(dynamic event) { + final Map map = event as Map; + // The strings here must all match the strings in FVPEventBridge.m. + _eventStreamController.add(switch (map['event']) { + 'initialized' => VideoEvent( + eventType: VideoEventType.initialized, + duration: Duration(milliseconds: map['duration'] as int), + size: Size( + (map['width'] as num?)?.toDouble() ?? 0.0, + (map['height'] as num?)?.toDouble() ?? 0.0, + ), + ), + 'completed' => VideoEvent(eventType: VideoEventType.completed), + 'bufferingUpdate' => VideoEvent( + buffered: (map['values'] as List) + .map(_toDurationRange) + .toList(), + eventType: VideoEventType.bufferingUpdate, + ), + 'bufferingStart' => VideoEvent(eventType: VideoEventType.bufferingStart), + 'bufferingEnd' => VideoEvent(eventType: VideoEventType.bufferingEnd), + 'isPlayingStateUpdate' => VideoEvent( + eventType: VideoEventType.isPlayingStateUpdate, + isPlaying: map['isPlaying'] as bool, + ), + _ => VideoEvent(eventType: VideoEventType.unknown), + }); } DurationRange _toDurationRange(dynamic value) { diff --git a/packages/video_player/video_player_avfoundation/pubspec.yaml b/packages/video_player/video_player_avfoundation/pubspec.yaml index e9f7eba7ae9..9c326136d92 100644 --- a/packages/video_player/video_player_avfoundation/pubspec.yaml +++ b/packages/video_player/video_player_avfoundation/pubspec.yaml @@ -2,7 +2,7 @@ name: video_player_avfoundation description: iOS and macOS implementation of the video_player plugin. repository: https://github.com/flutter/packages/tree/main/packages/video_player/video_player_avfoundation issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+video_player%22 -version: 2.8.7 +version: 2.8.8 environment: sdk: ^3.9.0 diff --git a/packages/video_player/video_player_avfoundation/test/avfoundation_video_player_test.dart b/packages/video_player/video_player_avfoundation/test/avfoundation_video_player_test.dart index 11cac97a7dd..9bdba2a903c 100644 --- a/packages/video_player/video_player_avfoundation/test/avfoundation_video_player_test.dart +++ b/packages/video_player/video_player_avfoundation/test/avfoundation_video_player_test.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'package:flutter/services.dart'; +import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:mockito/annotations.dart'; import 'package:mockito/mockito.dart'; @@ -24,15 +25,20 @@ void main() { MockAVFoundationVideoPlayerApi, MockVideoPlayerInstanceApi, ) - setUpMockPlayer({required int playerId}) { + setUpMockPlayer({required int playerId, int? textureId}) { final MockAVFoundationVideoPlayerApi pluginApi = MockAVFoundationVideoPlayerApi(); final MockVideoPlayerInstanceApi instanceApi = MockVideoPlayerInstanceApi(); final AVFoundationVideoPlayer player = AVFoundationVideoPlayer( pluginApi: pluginApi, - playerProvider: (_) => instanceApi, + playerApiProvider: (_) => instanceApi, + ); + player.ensurePlayerInitialized( + playerId, + textureId == null + ? const VideoPlayerPlatformViewState() + : VideoPlayerTextureViewState(textureId: textureId), ); - player.ensureApiInitialized(playerId); return (player, pluginApi, instanceApi); } @@ -49,6 +55,7 @@ void main() { _, ) = setUpMockPlayer( playerId: 1, + textureId: 101, ); await player.init(); @@ -62,11 +69,11 @@ void main() { MockVideoPlayerInstanceApi playerApi, ) = setUpMockPlayer( playerId: 1, + textureId: 101, ); await player.dispose(1); verify(playerApi.dispose()); - expect(player.playerViewStates, isEmpty); }); test('create with asset', () async { @@ -76,12 +83,11 @@ void main() { _, ) = setUpMockPlayer( playerId: 1, + textureId: 101, ); const int newPlayerId = 2; - const int textureId = 100; when(api.createForTextureView(any)).thenAnswer( - (_) async => - TexturePlayerIds(playerId: newPlayerId, textureId: textureId), + (_) async => TexturePlayerIds(playerId: newPlayerId, textureId: 102), ); const String asset = 'someAsset'; @@ -105,8 +111,8 @@ void main() { expect(creationOptions.uri, assetUrl); expect(playerId, newPlayerId); expect( - player.playerViewStates[newPlayerId], - const VideoPlayerTextureViewState(textureId: textureId), + player.buildViewWithOptions(VideoViewOptions(playerId: playerId!)), + isA(), ); }); @@ -119,6 +125,7 @@ void main() { _, ) = setUpMockPlayer( playerId: 1, + textureId: 101, ); const String asset = 'someAsset'; @@ -145,12 +152,11 @@ void main() { _, ) = setUpMockPlayer( playerId: 1, + textureId: 101, ); const int newPlayerId = 2; - const int textureId = 100; when(api.createForTextureView(any)).thenAnswer( - (_) async => - TexturePlayerIds(playerId: newPlayerId, textureId: textureId), + (_) async => TexturePlayerIds(playerId: newPlayerId, textureId: 102), ); const String uri = 'https://example.com'; @@ -171,8 +177,8 @@ void main() { expect(creationOptions.httpHeaders, {}); expect(playerId, newPlayerId); expect( - player.playerViewStates[newPlayerId], - const VideoPlayerTextureViewState(textureId: textureId), + player.buildViewWithOptions(VideoViewOptions(playerId: playerId!)), + isA(), ); }); @@ -183,10 +189,11 @@ void main() { _, ) = setUpMockPlayer( playerId: 1, + textureId: 101, ); when( api.createForTextureView(any), - ).thenAnswer((_) async => TexturePlayerIds(playerId: 2, textureId: 100)); + ).thenAnswer((_) async => TexturePlayerIds(playerId: 2, textureId: 102)); const Map headers = { 'Authorization': 'Bearer token', @@ -213,12 +220,11 @@ void main() { _, ) = setUpMockPlayer( playerId: 1, + textureId: 101, ); const int newPlayerId = 2; - const int textureId = 100; when(api.createForTextureView(any)).thenAnswer( - (_) async => - TexturePlayerIds(playerId: newPlayerId, textureId: textureId), + (_) async => TexturePlayerIds(playerId: newPlayerId, textureId: 102), ); const String fileUri = 'file:///foo/bar'; @@ -233,8 +239,8 @@ void main() { expect(creationOptions.uri, fileUri); expect(playerId, newPlayerId); expect( - player.playerViewStates[newPlayerId], - const VideoPlayerTextureViewState(textureId: textureId), + player.buildViewWithOptions(VideoViewOptions(playerId: playerId!)), + isA(), ); }); @@ -245,12 +251,11 @@ void main() { _, ) = setUpMockPlayer( playerId: 1, + textureId: 101, ); const int newPlayerId = 2; - const int textureId = 100; when(api.createForTextureView(any)).thenAnswer( - (_) async => - TexturePlayerIds(playerId: newPlayerId, textureId: textureId), + (_) async => TexturePlayerIds(playerId: newPlayerId, textureId: 102), ); const String asset = 'someAsset'; @@ -276,8 +281,8 @@ void main() { expect(creationOptions.uri, assetUrl); expect(playerId, newPlayerId); expect( - player.playerViewStates[newPlayerId], - const VideoPlayerTextureViewState(textureId: textureId), + player.buildViewWithOptions(VideoViewOptions(playerId: playerId!)), + isA(), ); }); @@ -288,12 +293,11 @@ void main() { _, ) = setUpMockPlayer( playerId: 1, + textureId: 101, ); const int newPlayerId = 2; - const int textureId = 100; when(api.createForTextureView(any)).thenAnswer( - (_) async => - TexturePlayerIds(playerId: newPlayerId, textureId: textureId), + (_) async => TexturePlayerIds(playerId: newPlayerId, textureId: 102), ); const String uri = 'https://example.com'; @@ -317,8 +321,8 @@ void main() { expect(creationOptions.httpHeaders, {}); expect(playerId, newPlayerId); expect( - player.playerViewStates[newPlayerId], - const VideoPlayerTextureViewState(textureId: textureId), + player.buildViewWithOptions(VideoViewOptions(playerId: playerId!)), + isA(), ); }); @@ -329,10 +333,11 @@ void main() { _, ) = setUpMockPlayer( playerId: 1, + textureId: 101, ); const int newPlayerId = 2; when(api.createForTextureView(any)).thenAnswer( - (_) async => TexturePlayerIds(playerId: newPlayerId, textureId: 100), + (_) async => TexturePlayerIds(playerId: newPlayerId, textureId: 102), ); const Map headers = { @@ -389,8 +394,8 @@ void main() { expect(creationOptions.uri, fileUri); expect(playerId, newPlayerId); expect( - player.playerViewStates[newPlayerId], - const VideoPlayerTextureViewState(textureId: textureId), + player.buildViewWithOptions(VideoViewOptions(playerId: playerId!)), + isA(), ); }); @@ -417,8 +422,8 @@ void main() { expect(playerId, newPlayerId); expect( - player.playerViewStates[newPlayerId], - const VideoPlayerPlatformViewState(), + player.buildViewWithOptions(VideoViewOptions(playerId: playerId!)), + isA(), ); }); @@ -552,14 +557,15 @@ void main() { }); test('videoEventsFor', () async { + const int playerId = 1; final ( AVFoundationVideoPlayer player, MockAVFoundationVideoPlayerApi api, _, ) = setUpMockPlayer( - playerId: 1, + playerId: playerId, ); - const String mockChannel = 'flutter.io/videoPlayer/videoEvents123'; + const String mockChannel = 'flutter.dev/videoPlayer/videoEvents$playerId'; TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger .setMockMessageHandler(mockChannel, (ByteData? message) async { final MethodCall methodCall = const StandardMethodCodec() @@ -666,7 +672,7 @@ void main() { } }); expect( - player.videoEventsFor(123), + player.videoEventsFor(playerId), emitsInOrder([ VideoEvent( eventType: VideoEventType.initialized, From 0383fd49dee9aeb7a6d1d0f966a7b20d2a00bbd5 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 21 Nov 2025 13:11:40 -0500 Subject: [PATCH 2/2] Gemini review --- .../lib/src/avfoundation_video_player.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/video_player/video_player_avfoundation/lib/src/avfoundation_video_player.dart b/packages/video_player/video_player_avfoundation/lib/src/avfoundation_video_player.dart index 0bb2593bd35..b4de6a0775e 100644 --- a/packages/video_player/video_player_avfoundation/lib/src/avfoundation_video_player.dart +++ b/packages/video_player/video_player_avfoundation/lib/src/avfoundation_video_player.dart @@ -229,7 +229,7 @@ class _PlayerInstance { final VideoPlayerViewState viewState; final EventChannel _eventChannel; final StreamController _eventStreamController = - StreamController(); + StreamController.broadcast(); StreamSubscription? _eventSubscription; Future play() => _api.play(); @@ -263,6 +263,7 @@ class _PlayerInstance { Future dispose() async { await _eventSubscription?.cancel(); + unawaited(_eventStreamController.close()); await _api.dispose(); }