diff --git a/packages/video_player/video_player_avfoundation/CHANGELOG.md b/packages/video_player/video_player_avfoundation/CHANGELOG.md index 5e7d48ef4a0a..98e61d1dd9f3 100644 --- a/packages/video_player/video_player_avfoundation/CHANGELOG.md +++ b/packages/video_player/video_player_avfoundation/CHANGELOG.md @@ -1,3 +1,8 @@ +## 2.4.9 + +* Fixes the iOS crash when using multiple players on the same screen. + See: https://github.com/flutter/flutter/issues/124937 + ## 2.4.8 * Fixes missing `isPlaybackLikelyToKeepUp` check for iOS video player `bufferingEnd` event and `bufferingStart` event. diff --git a/packages/video_player/video_player_avfoundation/example/ios/RunnerTests/VideoPlayerTests.m b/packages/video_player/video_player_avfoundation/example/ios/RunnerTests/VideoPlayerTests.m index 17c46ad6f57a..ef22338be36b 100644 --- a/packages/video_player/video_player_avfoundation/example/ios/RunnerTests/VideoPlayerTests.m +++ b/packages/video_player/video_player_avfoundation/example/ios/RunnerTests/VideoPlayerTests.m @@ -14,6 +14,8 @@ @interface FLTVideoPlayer : NSObject @property(readonly, nonatomic) AVPlayer *player; @property(readonly, nonatomic) AVPlayerLayer *playerLayer; @property(readonly, nonatomic) int64_t position; + +- (void)onTextureUnregistered:(NSObject *)texture; @end @interface FLTVideoPlayerPlugin (Test) @@ -442,6 +444,110 @@ - (void)testSeekToleranceWhenSeekingToEnd { return initializationEvent; } +// Checks whether [AVPlayer rate] KVO observations are correctly detached. +// - https://github.com/flutter/flutter/issues/124937 +// +// Failing to de-register results in a crash in [AVPlayer willChangeValueForKey:]. +- (void)testDoesNotCrashOnRateObservationAfterDisposal { + NSObject *registry = + (NSObject *)[[UIApplication sharedApplication] delegate]; + NSObject *registrar = + [registry registrarForPlugin:@"testDoesNotCrashOnRateObservationAfterDisposal"]; + + AVPlayer *avPlayer = nil; + __weak FLTVideoPlayer *player = nil; + + // Autoreleasepool is needed to simulate conditions of FLTVideoPlayer deallocation. + @autoreleasepool { + FLTVideoPlayerPlugin *videoPlayerPlugin = + (FLTVideoPlayerPlugin *)[[FLTVideoPlayerPlugin alloc] initWithRegistrar:registrar]; + + FlutterError *error; + [videoPlayerPlugin initialize:&error]; + XCTAssertNil(error); + + FLTCreateMessage *create = [FLTCreateMessage + makeWithAsset:nil + uri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/bee.mp4" + packageName:nil + formatHint:nil + httpHeaders:@{}]; + FLTTextureMessage *textureMessage = [videoPlayerPlugin create:create error:&error]; + XCTAssertNil(error); + XCTAssertNotNil(textureMessage); + + player = videoPlayerPlugin.playersByTextureId[textureMessage.textureId]; + XCTAssertNotNil(player); + avPlayer = player.player; + + [videoPlayerPlugin dispose:textureMessage error:&error]; + XCTAssertNil(error); + } + + // [FLTVideoPlayerPlugin dispose:error:] selector is dispatching the [FLTVideoPlayer dispose] call + // with a 1-second delay keeping a strong reference to the player. The polling ensures the player + // was truly deallocated. + [self expectationForPredicate:[NSPredicate predicateWithFormat:@"self != nil"] + evaluatedWithObject:player + handler:nil]; + [self waitForExpectationsWithTimeout:10.0 handler:nil]; + + [avPlayer willChangeValueForKey:@"rate"]; // No assertions needed. Lack of crash is a success. +} + +// During the hot reload: +// 1. `[FLTVideoPlayer onTextureUnregistered:]` gets called. +// 2. `[FLTVideoPlayerPlugin initialize:]` gets called. +// +// Both of these methods dispatch [FLTVideoPlayer dispose] on the main thread +// leading to a possible crash when de-registering observers twice. +- (void)testHotReloadDoesNotCrash { + NSObject *registry = + (NSObject *)[[UIApplication sharedApplication] delegate]; + NSObject *registrar = + [registry registrarForPlugin:@"testHotReloadDoesNotCrash"]; + + __weak FLTVideoPlayer *player = nil; + + // Autoreleasepool is needed to simulate conditions of FLTVideoPlayer deallocation. + @autoreleasepool { + FLTVideoPlayerPlugin *videoPlayerPlugin = + (FLTVideoPlayerPlugin *)[[FLTVideoPlayerPlugin alloc] initWithRegistrar:registrar]; + + FlutterError *error; + [videoPlayerPlugin initialize:&error]; + XCTAssertNil(error); + + FLTCreateMessage *create = [FLTCreateMessage + makeWithAsset:nil + uri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/bee.mp4" + packageName:nil + formatHint:nil + httpHeaders:@{}]; + FLTTextureMessage *textureMessage = [videoPlayerPlugin create:create error:&error]; + XCTAssertNil(error); + XCTAssertNotNil(textureMessage); + + player = videoPlayerPlugin.playersByTextureId[textureMessage.textureId]; + XCTAssertNotNil(player); + + [player onTextureUnregistered:nil]; + XCTAssertNil(error); + + [videoPlayerPlugin initialize:&error]; + XCTAssertNil(error); + } + + // [FLTVideoPlayerPlugin dispose:error:] selector is dispatching the [FLTVideoPlayer dispose] call + // with a 1-second delay keeping a strong reference to the player. The polling ensures the player + // was truly deallocated. + [self expectationForPredicate:[NSPredicate predicateWithFormat:@"self != nil"] + evaluatedWithObject:player + handler:nil]; + [self waitForExpectationsWithTimeout:10.0 + handler:nil]; // No assertions needed. Lack of crash is a success. +} + - (void)validateTransformFixForOrientation:(UIImageOrientation)orientation { AVAssetTrack *track = [[FakeAVAssetTrack alloc] initWithOrientation:orientation]; CGAffineTransform t = FLTGetStandardizedTransformForTrack(track); diff --git a/packages/video_player/video_player_avfoundation/ios/Classes/FLTVideoPlayerPlugin.m b/packages/video_player/video_player_avfoundation/ios/Classes/FLTVideoPlayerPlugin.m index 1c37d630ea2b..f4504aac2417 100644 --- a/packages/video_player/video_player_avfoundation/ios/Classes/FLTVideoPlayerPlugin.m +++ b/packages/video_player/video_player_avfoundation/ios/Classes/FLTVideoPlayerPlugin.m @@ -505,6 +505,14 @@ - (FlutterError *_Nullable)onListenWithArguments:(id _Nullable)arguments /// is useful for the case where the Engine is in the process of deconstruction /// so the channel is going to die or is already dead. - (void)disposeSansEventChannel { + // This check prevents the crash caused by removing the KVO observers twice. + // When performing a Hot Restart, the leftover players are disposed once directly + // by [FLTVideoPlayerPlugin initialize:] method and then disposed again by + // [FLTVideoPlayer onTextureUnregistered:] call leading to possible over-release. + if (_disposed) { + return; + } + _disposed = YES; [_playerLayer removeFromSuperlayer]; [_displayLink invalidate]; @@ -514,6 +522,7 @@ - (void)disposeSansEventChannel { [currentItem removeObserver:self forKeyPath:@"presentationSize"]; [currentItem removeObserver:self forKeyPath:@"duration"]; [currentItem removeObserver:self forKeyPath:@"playbackLikelyToKeepUp"]; + [self.player removeObserver:self forKeyPath:@"rate"]; [self.player replaceCurrentItemWithPlayerItem:nil]; [[NSNotificationCenter defaultCenter] removeObserver:self]; diff --git a/packages/video_player/video_player_avfoundation/pubspec.yaml b/packages/video_player/video_player_avfoundation/pubspec.yaml index 5c8339305728..606c587a41bd 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 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.4.8 +version: 2.4.9 environment: sdk: ">=2.18.0 <4.0.0"