[video_player] Simplify native iOS code (#9800)

Refactors and simplifies some of the native code in `video_player_avfoundation`:
- Separates `create` into two different entry points for the two display types, instead of having one entry point and then branching several times based on the display type.
- Moves `dispose` to the player instance, with the other instance-specific calls, by making plugin-level tracking cleanup part of `onDispose`.
- Moves the URL->`AVPlayerItem` logic out of the two player classes and into a central helper in the plugin class, reducing duplication and simplifying the player initialization logic.
- Addresses the TODO to separate texture IDs from player IDs.
- Makes `expectFrame` a private implementation detail rather than making a client call it.
- Removes an ARC check; we have used ARC everywhere in this repo for a long time.

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.

Author: stuartmorgan-g

packages/video_player/video_player_avfoundation/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 2.8.4
+
+* Simplifies native code.
+
 ## 2.8.3
 
 * Removes calls to self from init and dealloc, for maintainability.

packages/video_player/video_player_avfoundation/darwin/RunnerTests/VideoPlayerTests.m

@@ -28,29 +28,13 @@ @interface FVPTextureBasedVideoPlayer ()
 // (e.g., after a seek while paused). If YES, the display link should continue to run until the next
 // frame is successfully provided.
 @property(nonatomic, assign) BOOL waitingForFrame;
+
+/// Ensures that the frame updater runs until a frame is rendered, regardless of play/pause state.
+- (void)expectFrame;
 @end
 
 @implementation FVPTextureBasedVideoPlayer
 
-- (instancetype)initWithURL:(NSURL *)url
-               frameUpdater:(FVPFrameUpdater *)frameUpdater
-                displayLink:(NSObject<FVPDisplayLink> *)displayLink
-                httpHeaders:(nonnull NSDictionary<NSString *, NSString *> *)headers
-                  avFactory:(id<FVPAVFactory>)avFactory
-               viewProvider:(NSObject<FVPViewProvider> *)viewProvider {
-  NSDictionary<NSString *, id> *options = nil;
-  if ([headers count] != 0) {
-    options = @{@"AVURLAssetHTTPHeaderFieldsKey" : headers};
-  }
-  AVURLAsset *urlAsset = [AVURLAsset URLAssetWithURL:url options:options];
-  AVPlayerItem *item = [AVPlayerItem playerItemWithAsset:urlAsset];
-  return [self initWithPlayerItem:item
-                     frameUpdater:frameUpdater
-                      displayLink:displayLink
-                        avFactory:avFactory
-                     viewProvider:viewProvider];
-}
-
 - (instancetype)initWithPlayerItem:(AVPlayerItem *)item
                       frameUpdater:(FVPFrameUpdater *)frameUpdater
                        displayLink:(NSObject<FVPDisplayLink> *)displayLink
@@ -81,6 +65,10 @@ - (void)dealloc {
 
 - (void)setTextureIdentifier:(int64_t)textureIdentifier {
   self.frameUpdater.textureIdentifier = textureIdentifier;
+
+  // Ensure that the first frame is drawn once available, even if the video isn't played, since
+  // the engine is now expecting the texture to be populated.
+  [self expectFrame];
 }
 
 - (void)expectFrame {
@@ -118,8 +106,8 @@ - (void)seekTo:(NSInteger)position completion:(void (^)(FlutterError *_Nullable)
       }];
 }
 
-- (void)dispose {
-  [super dispose];
+- (void)disposeWithError:(FlutterError *_Nullable *_Nonnull)error {
+  [super disposeWithError:error];
 
   [self.playerLayer removeFromSuperlayer];
 
@@ -214,7 +202,8 @@ - (CVPixelBufferRef)copyPixelBuffer {
 - (void)onTextureUnregistered:(NSObject<FlutterTexture> *)texture {
   dispatch_async(dispatch_get_main_queue(), ^{
     if (!self.disposed) {
-      [self dispose];
+      FlutterError *error;
+      [self disposeWithError:&error];
     }
   });
 }

packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPTextureBasedVideoPlayer.m

@@ -75,19 +75,6 @@ @implementation FVPVideoPlayer {
   BOOL _listenersRegistered;
 }
 
-- (instancetype)initWithURL:(NSURL *)url
-                httpHeaders:(nonnull NSDictionary<NSString *, NSString *> *)headers
-                  avFactory:(id<FVPAVFactory>)avFactory
-               viewProvider:(NSObject<FVPViewProvider> *)viewProvider {
-  NSDictionary<NSString *, id> *options = nil;
-  if ([headers count] != 0) {
-    options = @{@"AVURLAssetHTTPHeaderFieldsKey" : headers};
-  }
-  AVURLAsset *urlAsset = [AVURLAsset URLAssetWithURL:url options:options];
-  AVPlayerItem *item = [AVPlayerItem playerItemWithAsset:urlAsset];
-  return [self initWithPlayerItem:item avFactory:avFactory viewProvider:viewProvider];
-}
-
 - (instancetype)initWithPlayerItem:(AVPlayerItem *)item
                          avFactory:(id<FVPAVFactory>)avFactory
                       viewProvider:(NSObject<FVPViewProvider> *)viewProvider {
@@ -152,7 +139,7 @@ - (void)dealloc {
   }
 }
 
-- (void)dispose {
+- (void)disposeWithError:(FlutterError *_Nullable *_Nonnull)error {
   // In some hot restart scenarios, dispose can be called twice, so no-op after the first time.
   if (_disposed) {
     return;

packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayer.m

@@ -18,10 +18,6 @@
 // https://github.com/flutter/packages/pull/6675/#discussion_r1591210702
 #import "./include/video_player_avfoundation/messages.g.h"
 
-#if !__has_feature(objc_arc)
-#error Code Requires ARC.
-#endif
-
 /// Non-test implementation of the diplay link factory.
 @interface FVPDefaultDisplayLinkFactory : NSObject <FVPDisplayLinkFactory>
 @end
@@ -48,8 +44,7 @@ @interface FVPVideoPlayerPlugin ()
 @property(nonatomic, strong) id<FVPDisplayLinkFactory> displayLinkFactory;
 @property(nonatomic, strong) id<FVPAVFactory> avFactory;
 @property(nonatomic, strong) NSObject<FVPViewProvider> *viewProvider;
-// TODO(stuartmorgan): Decouple identifiers for platform views and texture views.
-@property(nonatomic, assign) int64_t nextNonTexturePlayerIdentifier;
+@property(nonatomic, assign) int64_t nextPlayerIdentifier;
 @end
 
 @implementation FVPVideoPlayerPlugin
@@ -84,49 +79,39 @@ - (instancetype)initWithAVFactory:(id<FVPAVFactory>)avFactory
   _avFactory = avFactory ?: [[FVPDefaultAVFactory alloc] init];
   _viewProvider = viewProvider ?: [[FVPDefaultViewProvider alloc] initWithRegistrar:registrar];
   _playersByIdentifier = [NSMutableDictionary dictionaryWithCapacity:1];
-  // Initialized to a high number to avoid collisions with texture identifiers (which are generated
-  // separately).
-  _nextNonTexturePlayerIdentifier = INT_MAX;
+  _nextPlayerIdentifier = 1;
   return self;
 }
 
 - (void)detachFromEngineForRegistrar:(NSObject<FlutterPluginRegistrar> *)registrar {
+  FlutterError *error;
   for (FVPVideoPlayer *player in self.playersByIdentifier.allValues) {
     // Remove the channel and texture cleanup, and the event listener, to ensure that the player
     // doesn't message the engine that is no longer connected.
     player.onDisposed = nil;
     player.eventListener = nil;
-    [player dispose];
+    [player disposeWithError:&error];
   }
   [self.playersByIdentifier removeAllObjects];
   SetUpFVPAVFoundationVideoPlayerApi(registrar.messenger, nil);
 }
 
-- (int64_t)onPlayerSetup:(FVPVideoPlayer *)player {
-  FVPTextureBasedVideoPlayer *textureBasedPlayer =
-      [player isKindOfClass:[FVPTextureBasedVideoPlayer class]]
-          ? (FVPTextureBasedVideoPlayer *)player
-          : nil;
-
-  int64_t playerIdentifier;
-  if (textureBasedPlayer) {
-    playerIdentifier = [self.registrar.textures registerTexture:textureBasedPlayer];
-    [textureBasedPlayer setTextureIdentifier:playerIdentifier];
-  } else {
-    playerIdentifier = self.nextNonTexturePlayerIdentifier--;
-  }
+- (int64_t)configurePlayer:(FVPVideoPlayer *)player
+    withExtraDisposeHandler:(nullable void (^)(void))extraDisposeHandler {
+  int64_t playerIdentifier = self.nextPlayerIdentifier++;
+  self.playersByIdentifier[@(playerIdentifier)] = player;
 
   NSObject<FlutterBinaryMessenger> *messenger = self.registrar.messenger;
   NSString *channelSuffix = [NSString stringWithFormat:@"%lld", playerIdentifier];
   // Set up the player-specific API handler, and its onDispose unregistration.
   SetUpFVPVideoPlayerInstanceApiWithSuffix(messenger, player, channelSuffix);
   __weak typeof(self) weakSelf = self;
-  BOOL isTextureBased = textureBasedPlayer != nil;
   player.onDisposed = ^() {
     SetUpFVPVideoPlayerInstanceApiWithSuffix(messenger, nil, channelSuffix);
-    if (isTextureBased) {
-      [weakSelf.registrar.textures unregisterTexture:playerIdentifier];
+    if (extraDisposeHandler) {
+      extraDisposeHandler();
     }
+    [weakSelf.playersByIdentifier removeObjectForKey:@(playerIdentifier)];
   };
   // Set up the event channel.
   FVPEventBridge *eventBridge = [[FVPEventBridge alloc]
@@ -135,12 +120,6 @@ - (int64_t)onPlayerSetup:(FVPVideoPlayer *)player {
                                                    channelSuffix]];
   player.eventListener = eventBridge;
 
-  self.playersByIdentifier[@(playerIdentifier)] = player;
-
-  // Ensure that the first frame is drawn once available, even if the video isn't played, since
-  // the engine is now expecting the texture to be populated.
-  [textureBasedPlayer expectFrame];
-
   return playerIdentifier;
 }
 
@@ -186,55 +165,64 @@ - (void)initialize:(FlutterError *__autoreleasing *)error {
   upgradeAudioSessionCategory(AVAudioSessionCategoryPlayback, 0, 0);
 #endif
 
-  [self.playersByIdentifier.allValues makeObjectsPerformSelector:@selector(dispose)];
+  FlutterError *disposeError;
+  // Disposing a player removes it from the dictionary, so iterate over a copy.
+  NSArray<FVPVideoPlayer *> *players = [self.playersByIdentifier.allValues copy];
+  for (FVPVideoPlayer *player in players) {
+    [player disposeWithError:&disposeError];
+  }
   [self.playersByIdentifier removeAllObjects];
 }
 
-- (nullable NSNumber *)createWithOptions:(nonnull FVPCreationOptions *)options
-                                   error:(FlutterError **)error {
-  BOOL textureBased = options.viewType == FVPPlatformVideoViewTypeTextureView;
-
+- (nullable NSNumber *)createPlatformViewPlayerWithOptions:(nonnull FVPCreationOptions *)options
+                                                     error:(FlutterError **)error {
   @try {
-    FVPVideoPlayer *player = textureBased ? [self texturePlayerWithOptions:options]
-                                          : [self platformViewPlayerWithOptions:options];
-    return @([self onPlayerSetup:player]);
+    AVPlayerItem *item = [self playerItemWithCreationOptions:options];
+
+    // FVPVideoPlayer contains all required logic for platform views.
+    FVPVideoPlayer *player = [[FVPVideoPlayer alloc] initWithPlayerItem:item
+                                                              avFactory:self.avFactory
+                                                           viewProvider:self.viewProvider];
+
+    return @([self configurePlayer:player withExtraDisposeHandler:nil]);
   } @catch (NSException *exception) {
     *error = [FlutterError errorWithCode:@"video_player" message:exception.reason details:nil];
     return nil;
   }
 }
 
-- (nonnull FVPTextureBasedVideoPlayer *)texturePlayerWithOptions:
-    (nonnull FVPCreationOptions *)options {
-  FVPFrameUpdater *frameUpdater =
-      [[FVPFrameUpdater alloc] initWithRegistry:self.registrar.textures];
-  NSObject<FVPDisplayLink> *displayLink =
-      [self.displayLinkFactory displayLinkWithRegistrar:_registrar
-                                               callback:^() {
-                                                 [frameUpdater displayLinkFired];
-                                               }];
-
-  return [[FVPTextureBasedVideoPlayer alloc] initWithURL:[NSURL URLWithString:options.uri]
-                                            frameUpdater:frameUpdater
-                                             displayLink:displayLink
-                                             httpHeaders:options.httpHeaders
-                                               avFactory:self.avFactory
-                                            viewProvider:self.viewProvider];
-}
-
-- (nonnull FVPVideoPlayer *)platformViewPlayerWithOptions:(nonnull FVPCreationOptions *)options {
-  // FVPVideoPlayer contains all required logic for platform views.
-  return [[FVPVideoPlayer alloc] initWithURL:[NSURL URLWithString:options.uri]
-                                 httpHeaders:options.httpHeaders
-                                   avFactory:self.avFactory
-                                viewProvider:self.viewProvider];
-}
-
-- (void)disposePlayer:(NSInteger)playerIdentifier error:(FlutterError **)error {
-  NSNumber *playerKey = @(playerIdentifier);
-  FVPVideoPlayer *player = self.playersByIdentifier[playerKey];
-  [self.playersByIdentifier removeObjectForKey:playerKey];
-  [player dispose];
+- (nullable FVPTexturePlayerIds *)createTexturePlayerWithOptions:
+                                      (nonnull FVPCreationOptions *)options
+                                                           error:(FlutterError **)error {
+  @try {
+    AVPlayerItem *item = [self playerItemWithCreationOptions:options];
+    FVPFrameUpdater *frameUpdater =
+        [[FVPFrameUpdater alloc] initWithRegistry:self.registrar.textures];
+    NSObject<FVPDisplayLink> *displayLink =
+        [self.displayLinkFactory displayLinkWithRegistrar:_registrar
+                                                 callback:^() {
+                                                   [frameUpdater displayLinkFired];
+                                                 }];
+
+    FVPTextureBasedVideoPlayer *player =
+        [[FVPTextureBasedVideoPlayer alloc] initWithPlayerItem:item
+                                                  frameUpdater:frameUpdater
+                                                   displayLink:displayLink
+                                                     avFactory:self.avFactory
+                                                  viewProvider:self.viewProvider];
+
+    int64_t textureIdentifier = [self.registrar.textures registerTexture:player];
+    [player setTextureIdentifier:textureIdentifier];
+    __weak typeof(self) weakSelf = self;
+    int64_t playerIdentifier = [self configurePlayer:player
+                             withExtraDisposeHandler:^() {
+                               [weakSelf.registrar.textures unregisterTexture:textureIdentifier];
+                             }];
+    return [FVPTexturePlayerIds makeWithPlayerId:playerIdentifier textureId:textureIdentifier];
+  } @catch (NSException *exception) {
+    *error = [FlutterError errorWithCode:@"video_player" message:exception.reason details:nil];
+    return nil;
+  }
 }
 
 - (void)setMixWithOthers:(BOOL)mixWithOthers
@@ -274,4 +262,14 @@ - (nullable NSString *)fileURLForAssetWithName:(NSString *)asset
   return [NSURL fileURLWithPath:path].absoluteString;
 }
 
+/// Returns the AVPlayerItem corresponding to the given player creation options.
+- (nonnull AVPlayerItem *)playerItemWithCreationOptions:(nonnull FVPCreationOptions *)options {
+  NSDictionary<NSString *, NSString *> *headers = options.httpHeaders;
+  NSDictionary<NSString *, id> *itemOptions =
+      headers.count == 0 ? nil : @{@"AVURLAssetHTTPHeaderFieldsKey" : headers};
+  AVURLAsset *asset = [AVURLAsset URLAssetWithURL:[NSURL URLWithString:options.uri]
+                                          options:itemOptions];
+  return [AVPlayerItem playerItemWithAsset:asset];
+}
+
 @end

packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m

@@ -14,22 +14,17 @@ NS_ASSUME_NONNULL_BEGIN
 /// updates frames, and handles display link callbacks.
 /// If you need to display a video using platform view, use FVPVideoPlayer instead.
 @interface FVPTextureBasedVideoPlayer : FVPVideoPlayer <FlutterTexture>
-/// Initializes a new instance of FVPTextureBasedVideoPlayer with the given URL, frame updater,
-/// display link, HTTP headers, AV factory, and registrar.
-- (instancetype)initWithURL:(NSURL *)url
-               frameUpdater:(FVPFrameUpdater *)frameUpdater
-                displayLink:(NSObject<FVPDisplayLink> *)displayLink
-                httpHeaders:(nonnull NSDictionary<NSString *, NSString *> *)headers
-                  avFactory:(id<FVPAVFactory>)avFactory
-               viewProvider:(NSObject<FVPViewProvider> *)viewProvider;
+/// Initializes a new instance of FVPTextureBasedVideoPlayer with the given player item,
+/// frame updater, display link, AV factory, and view provider.
+- (instancetype)initWithPlayerItem:(AVPlayerItem *)item
+                      frameUpdater:(FVPFrameUpdater *)frameUpdater
+                       displayLink:(NSObject<FVPDisplayLink> *)displayLink
+                         avFactory:(id<FVPAVFactory>)avFactory
+                      viewProvider:(NSObject<FVPViewProvider> *)viewProvider;
 
 /// Sets the texture Identifier for the frame updater. This method should be called once the texture
 /// identifier is obtained from the texture registry.
 - (void)setTextureIdentifier:(int64_t)textureIdentifier;
-
-/// Tells the player to run its frame updater until it receives a frame, regardless of the
-/// play/pause state.
-- (void)expectFrame;
 @end
 
 NS_ASSUME_NONNULL_END

packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/include/video_player_avfoundation/FVPTextureBasedVideoPlayer.h

@@ -36,15 +36,11 @@ NS_ASSUME_NONNULL_BEGIN
 /// A block that will be called when dispose is called.
 @property(nonatomic, nullable, copy) void (^onDisposed)(void);
 
-/// Initializes a new instance of FVPVideoPlayer with the given URL, HTTP headers, AV factory, and
-/// view provider.
-- (instancetype)initWithURL:(NSURL *)url
-                httpHeaders:(nonnull NSDictionary<NSString *, NSString *> *)headers
-                  avFactory:(id<FVPAVFactory>)avFactory
-               viewProvider:(NSObject<FVPViewProvider> *)viewProvider;
-
-/// Disposes the video player and releases any resources it holds.
-- (void)dispose;
+/// Initializes a new instance of FVPVideoPlayer with the given AVPlayerItem, AV factory, and view
+/// provider.
+- (instancetype)initWithPlayerItem:(AVPlayerItem *)item
+                         avFactory:(id<FVPAVFactory>)avFactory
+                      viewProvider:(NSObject<FVPViewProvider> *)viewProvider;
 
 @end
 

packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/include/video_player_avfoundation/FVPVideoPlayer.h

@@ -25,12 +25,6 @@ NS_ASSUME_NONNULL_BEGIN
 /// Indicates whether the video player has been initialized.
 @property(nonatomic, readonly) BOOL isInitialized;
 
-/// Initializes a new instance of FVPVideoPlayer with the given AVPlayerItem, frame updater, display
-/// link, AV factory, and view provider.
-- (instancetype)initWithPlayerItem:(AVPlayerItem *)item
-                         avFactory:(id<FVPAVFactory>)avFactory
-                      viewProvider:(NSObject<FVPViewProvider> *)viewProvider;
-
 /// Updates the playing state of the video player.
 - (void)updatePlayingState;
 @end