From 28bc1081392b0d4df2ff359d7451ac681cb011ff Mon Sep 17 00:00:00 2001 From: Huan Lin Date: Tue, 27 Sep 2022 15:00:31 -0700 Subject: [PATCH 1/2] [video_player]fix wrong aspect ratio for some video streams bump version --- .../video_player_avfoundation/CHANGELOG.md | 3 +- .../ios/RunnerTests/VideoPlayerTests.m | 28 ++++++------- .../ios/Classes/FLTVideoPlayerPlugin.m | 42 +++++++++---------- .../video_player_avfoundation/pubspec.yaml | 2 +- 4 files changed, 35 insertions(+), 40 deletions(-) diff --git a/packages/video_player/video_player_avfoundation/CHANGELOG.md b/packages/video_player/video_player_avfoundation/CHANGELOG.md index 7cfa025271cf..ed2f345784bd 100644 --- a/packages/video_player/video_player_avfoundation/CHANGELOG.md +++ b/packages/video_player/video_player_avfoundation/CHANGELOG.md @@ -1,5 +1,6 @@ -## NEXT +## 2.3.7 +* Fixes a bug where the aspect ratio of some HLS videos are incorrectly inverted. * Updates code for `no_leading_underscores_for_local_identifiers` lint. ## 2.3.6 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 813fca2b8e7d..a96b2621bf9d 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 @@ -11,9 +11,11 @@ @interface FLTVideoPlayer : NSObject @property(readonly, nonatomic) AVPlayer *player; -// This is to fix a bug (https://github.com/flutter/flutter/issues/111457) in iOS 16 with blank -// video for encrypted video streams. An invisible AVPlayerLayer is used to overwrite the -// protection of pixel buffers in those streams. +// This is to fix 2 bugs: 1. blank video for encrypted video streams on iOS 16 +// (https://github.com/flutter/flutter/issues/111457) and 2. swapped width and height for some video +// streams (not just iOS 16). (https://github.com/flutter/flutter/issues/109116). +// An invisible AVPlayerLayer is used to overwrite the protection of pixel buffers in those streams +// for issue #1, and restore the correct width and height for issue #2. @property(readonly, nonatomic) AVPlayerLayer *playerLayer; @end @@ -66,12 +68,11 @@ @interface VideoPlayerTests : XCTestCase @implementation VideoPlayerTests - (void)testIOS16BugWithEncryptedVideoStream { - // This is to fix a bug (https://github.com/flutter/flutter/issues/111457) in iOS 16 with blank - // video for encrypted video streams. An invisible AVPlayerLayer is used to overwrite the - // protection of pixel buffers in those streams. - // Note that a better unit test is to validate that `copyPixelBuffer` API returns the pixel - // buffers as expected, which requires setting up the video player properly with a protected video - // stream (.m3u8 file). + // This is to fix 2 bugs: 1. blank video for encrypted video streams on iOS 16 + // (https://github.com/flutter/flutter/issues/111457) and 2. swapped width and height for some + // video streams (not just iOS 16). (https://github.com/flutter/flutter/issues/109116). An + // invisible AVPlayerLayer is used to overwrite the protection of pixel buffers in those streams + // for issue #1, and restore the correct width and height for issue #2. NSObject *registry = (NSObject *)[[UIApplication sharedApplication] delegate]; NSObject *registrar = @@ -95,13 +96,8 @@ - (void)testIOS16BugWithEncryptedVideoStream { FLTVideoPlayer *player = videoPlayerPlugin.playersByTextureId[textureMessage.textureId]; XCTAssertNotNil(player); - if (@available(iOS 16.0, *)) { - XCTAssertNotNil(player.playerLayer, @"AVPlayerLayer should be present for iOS 16."); - XCTAssertNotNil(player.playerLayer.superlayer, - @"AVPlayerLayer should be added on screen for iOS 16."); - } else { - XCTAssertNil(player.playerLayer, @"AVPlayerLayer should not be present before iOS 16."); - } + XCTAssertNotNil(player.playerLayer, @"AVPlayerLayer should be present."); + XCTAssertNotNil(player.playerLayer.superlayer, @"AVPlayerLayer should be added on screen."); } - (void)testSeekToInvokesTextureFrameAvailableOnTextureRegistry { 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 645c86d6eade..3b066769621c 100644 --- a/packages/video_player/video_player_avfoundation/ios/Classes/FLTVideoPlayerPlugin.m +++ b/packages/video_player/video_player_avfoundation/ios/Classes/FLTVideoPlayerPlugin.m @@ -36,7 +36,11 @@ - (void)onDisplayLink:(CADisplayLink *)link { @interface FLTVideoPlayer : NSObject @property(readonly, nonatomic) AVPlayer *player; @property(readonly, nonatomic) AVPlayerItemVideoOutput *videoOutput; -/// An invisible player layer used to access the pixel buffers in protected video streams in iOS 16. +// This is to fix 2 bugs: 1. blank video for encrypted video streams on iOS 16 +// (https://github.com/flutter/flutter/issues/111457) and 2. swapped width and height for some video +// streams (not just iOS 16). (https://github.com/flutter/flutter/issues/109116). +// An invisible AVPlayerLayer is used to overwrite the protection of pixel buffers in those streams +// for issue #1, and restore the correct width and height for issue #2. @property(readonly, nonatomic) AVPlayerLayer *playerLayer; @property(readonly, nonatomic) CADisplayLink *displayLink; @property(nonatomic) FlutterEventChannel *eventChannel; @@ -134,17 +138,13 @@ NS_INLINE CGFloat radiansToDegrees(CGFloat radians) { return degrees; }; -NS_INLINE UIViewController *rootViewController() API_AVAILABLE(ios(16.0)) { - for (UIScene *scene in UIApplication.sharedApplication.connectedScenes) { - if ([scene isKindOfClass:UIWindowScene.class]) { - for (UIWindow *window in ((UIWindowScene *)scene).windows) { - if (window.isKeyWindow) { - return window.rootViewController; - } - } - } - } - return nil; +NS_INLINE UIViewController *rootViewController() { +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" + // TODO: (hellohuanlin) Provide a non-deprecated codepath. See + // https://github.com/flutter/flutter/issues/104117 + return UIApplication.sharedApplication.keyWindow.rootViewController; +#pragma clang diagnostic pop } - (AVMutableVideoComposition *)getVideoCompositionWithTransform:(CGAffineTransform)transform @@ -242,13 +242,13 @@ - (instancetype)initWithPlayerItem:(AVPlayerItem *)item _player = [AVPlayer playerWithPlayerItem:item]; _player.actionAtItemEnd = AVPlayerActionAtItemEndNone; - // This is to fix a bug (https://github.com/flutter/flutter/issues/111457) in iOS 16 with blank - // video for encrypted video streams. An invisible AVPlayerLayer is used to overwrite the - // protection of pixel buffers in those streams. - if (@available(iOS 16.0, *)) { - _playerLayer = [AVPlayerLayer playerLayerWithPlayer:_player]; - [rootViewController().view.layer addSublayer:_playerLayer]; - } + // This is to fix 2 bugs: 1. blank video for encrypted video streams on iOS 16 + // (https://github.com/flutter/flutter/issues/111457) and 2. swapped width and height for some + // video streams (not just iOS 16). (https://github.com/flutter/flutter/issues/109116). An + // invisible AVPlayerLayer is used to overwrite the protection of pixel buffers in those streams + // for issue #1, and restore the correct width and height for issue #2. + _playerLayer = [AVPlayerLayer playerLayerWithPlayer:_player]; + [rootViewController().view.layer addSublayer:_playerLayer]; [self createVideoOutputAndDisplayLink:frameUpdater]; @@ -481,9 +481,7 @@ - (FlutterError *_Nullable)onListenWithArguments:(id _Nullable)arguments /// so the channel is going to die or is already dead. - (void)disposeSansEventChannel { _disposed = YES; - if (@available(iOS 16.0, *)) { - [_playerLayer removeFromSuperlayer]; - } + [_playerLayer removeFromSuperlayer]; [_displayLink invalidate]; AVPlayerItem *currentItem = self.player.currentItem; [currentItem removeObserver:self forKeyPath:@"status"]; diff --git a/packages/video_player/video_player_avfoundation/pubspec.yaml b/packages/video_player/video_player_avfoundation/pubspec.yaml index bd88ddf94876..6da166791281 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/plugins/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.3.6 +version: 2.3.7 environment: sdk: ">=2.14.0 <3.0.0" From f30a7b1e1a3f20cba30e997f7752351e49c64ed6 Mon Sep 17 00:00:00 2001 From: Huan Lin Date: Mon, 3 Oct 2022 16:16:11 -0700 Subject: [PATCH 2/2] address comment --- .../example/ios/RunnerTests/VideoPlayerTests.m | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) 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 a96b2621bf9d..f9f66e04bcb3 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 @@ -11,11 +11,6 @@ @interface FLTVideoPlayer : NSObject @property(readonly, nonatomic) AVPlayer *player; -// This is to fix 2 bugs: 1. blank video for encrypted video streams on iOS 16 -// (https://github.com/flutter/flutter/issues/111457) and 2. swapped width and height for some video -// streams (not just iOS 16). (https://github.com/flutter/flutter/issues/109116). -// An invisible AVPlayerLayer is used to overwrite the protection of pixel buffers in those streams -// for issue #1, and restore the correct width and height for issue #2. @property(readonly, nonatomic) AVPlayerLayer *playerLayer; @end @@ -67,7 +62,7 @@ @interface VideoPlayerTests : XCTestCase @implementation VideoPlayerTests -- (void)testIOS16BugWithEncryptedVideoStream { +- (void)testBlankVideoBugWithEncryptedVideoStreamAndInvertedAspectRatioBugForSomeVideoStream { // This is to fix 2 bugs: 1. blank video for encrypted video streams on iOS 16 // (https://github.com/flutter/flutter/issues/111457) and 2. swapped width and height for some // video streams (not just iOS 16). (https://github.com/flutter/flutter/issues/109116). An