send video load failure even when eventsink was initialized late

flutter · Jul 22, 2024 · 16d3d7a · 16d3d7a
1 parent 7022a44
commit 16d3d7a
Show file tree

Hide file tree

Showing 4 changed files with 93 additions and 13 deletions.
diff --git a/packages/video_player/video_player_avfoundation/CHANGELOG.md b/packages/video_player/video_player_avfoundation/CHANGELOG.md
@@ -1,3 +1,8 @@
+## 2.6.2
+
+* Fixes VideoPlayerController.initialize() future never resolving with invalid video file.
+* Adds more details to the error message returned by VideoPlayerController.initialize().
+
 ## 2.6.1
 
 * Adds files to make include directory permanent.

diff --git a/packages/video_player/video_player_avfoundation/darwin/RunnerTests/VideoPlayerTests.m b/packages/video_player/video_player_avfoundation/darwin/RunnerTests/VideoPlayerTests.m
@@ -790,6 +790,39 @@ - (void)testPublishesInRegistration {
   XCTAssertTrue([publishedValue isKindOfClass:[FVPVideoPlayerPlugin class]]);
 }
 
+- (void)testFailedToLoadVideoEventShouldBeAlwaysSent {
+  NSObject<FlutterPluginRegistrar> *registrar =
+      [GetPluginRegistry() registrarForPlugin:@"testFailedToLoadVideoEventShouldBeAlwaysSent"];
+  FVPVideoPlayerPlugin *videoPlayerPlugin =
+      [[FVPVideoPlayerPlugin alloc] initWithRegistrar:registrar];
+  FlutterError *error;
+
+  [videoPlayerPlugin initialize:&error];
+
+  FVPCreationOptions *create = [FVPCreationOptions makeWithAsset:nil
+                                                             uri:@""
+                                                     packageName:nil
+                                                      formatHint:nil
+                                                     httpHeaders:@{}];
+  NSNumber *textureId = [videoPlayerPlugin createWithOptions:create error:&error];
+  FVPVideoPlayer *player = videoPlayerPlugin.playersByTextureId[textureId];
+  XCTAssertNotNil(player);
+
+  [self keyValueObservingExpectationForObject:(id)player.player.currentItem
+                                      keyPath:@"status"
+                                expectedValue:@(AVPlayerItemStatusFailed)];
+  [self waitForExpectationsWithTimeout:10.0 handler:nil];
+
+  XCTestExpectation *failedExpectation = [self expectationWithDescription:@"failed"];
+  [player onListenWithArguments:nil
+                      eventSink:^(FlutterError *event) {
+                        if ([event isKindOfClass:FlutterError.class]) {
+                          [failedExpectation fulfill];
+                        }
+                      }];
+  [self waitForExpectationsWithTimeout:10.0 handler:nil];
+}
+
 #if TARGET_OS_IOS
 - (void)validateTransformFixForOrientation:(UIImageOrientation)orientation {
   AVAssetTrack *track = [[FakeAVAssetTrack alloc] initWithOrientation:orientation];

diff --git a/...darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m b/...darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m
@@ -345,13 +345,7 @@ - (void)observeValueForKeyPath:(NSString *)path
     AVPlayerItem *item = (AVPlayerItem *)object;
     switch (item.status) {
       case AVPlayerItemStatusFailed:
-        if (_eventSink != nil) {
-          _eventSink([FlutterError
-              errorWithCode:@"VideoError"
-                    message:[@"Failed to load video: "
-                                stringByAppendingString:[item.error localizedDescription]]
-                    details:nil]);
-        }
+        [self sendFailedToLoadVideoEvent:item.error];
         break;
       case AVPlayerItemStatusUnknown:
         break;
@@ -406,9 +400,46 @@ - (void)updatePlayingState {
   _displayLink.running = _isPlaying || self.waitingForFrame;
 }
 
+- (void)sendFailedToLoadVideoEvent:(NSError *)error {
+  if (!NSThread.isMainThread) {
+    dispatch_async(dispatch_get_main_queue(), ^{
+      [self sendFailedToLoadVideoEvent:error];
+    });
+    return;
+  }
+  if (_eventSink == nil) {
+    return;
+  }
+  __block NSString *message = @"Failed to load video";
+  void (^add)(NSString *) = ^(NSString *detail) {
+    if (detail != nil) {
+      message = [message stringByAppendingFormat:@": %@", detail];
+    }
+  };
+  NSError *underlyingError = error.userInfo[NSUnderlyingErrorKey];
+  // add more details to error message
+  // https://github.com/flutter/flutter/issues/56665
+  add(error.localizedDescription);
+  add(error.localizedFailureReason);
+  add(underlyingError.localizedDescription);
+  add(underlyingError.localizedFailureReason);
+  _eventSink([FlutterError errorWithCode:@"VideoError" message:message details:nil]);
+}
+
 - (void)setupEventSinkIfReadyToPlay {
   if (_eventSink && !_isInitialized) {
     AVPlayerItem *currentItem = self.player.currentItem;
+    // if status is Failed here then this was called from onListenWithArguments which means
+    // _eventSink was nil when was called observeValueForKeyPath with updated Failed status
+    // and error was not sent and never will be so it is needed to send it here to have
+    // future returned by initialize resolved instead of in state of infinite waiting,
+    // see comment in onListenWithArguments
+    // https://github.com/flutter/flutter/issues/151475
+    // https://github.com/flutter/flutter/issues/147707
+    if (currentItem.status == AVPlayerItemStatusFailed) {
+      [self sendFailedToLoadVideoEvent:currentItem.error];
+      return;
+    }
     CGSize size = currentItem.presentationSize;
     CGFloat width = size.width;
     CGFloat height = size.height;
@@ -417,13 +448,24 @@ - (void)setupEventSinkIfReadyToPlay {
     AVAsset *asset = currentItem.asset;
     if ([asset statusOfValueForKey:@"tracks" error:nil] != AVKeyValueStatusLoaded) {
       void (^trackCompletionHandler)(void) = ^{
-        if ([asset statusOfValueForKey:@"tracks" error:nil] != AVKeyValueStatusLoaded) {
+        NSError *error;
+        switch ([asset statusOfValueForKey:@"tracks" error:&error]) {
+          case AVKeyValueStatusLoaded:
+            // This completion block will run on an AVFoundation background queue.
+            // Hop back to the main thread to set up event sink.
+            [self performSelector:_cmd
+                         onThread:NSThread.mainThread
+                       withObject:self
+                    waitUntilDone:NO];
+            break;
           // Cancelled, or something failed.
-          return;
+          case AVKeyValueStatusFailed:
+            // if something failed then future should result in error
+            [self sendFailedToLoadVideoEvent:error];
+            break;
+          default:
+            break;
         }
-        // This completion block will run on an AVFoundation background queue.
-        // Hop back to the main thread to set up event sink.
-        [self performSelector:_cmd onThread:NSThread.mainThread withObject:self waitUntilDone:NO];
       };
       [asset loadValuesAsynchronouslyForKeys:@[ @"tracks" ]
                            completionHandler:trackCompletionHandler];

diff --git 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.6.1
+version: 2.6.2
 
 environment:
   sdk: ^3.2.3