Skip to content

Commit 5d2a10f

Browse files
[video_player] Move more Obj-C logic to Dart (#9685)
Moves some logic from native code to Dart: - Asset -> URL lookup. - Conversion from native buffer range format (start+duration) to Dart range format (start+end). Also does some minor native cleanup: - Removes unnecessary state from `FVPVideoPlayerPlugin`; several registrar-vended objects were being stored, and later changes required storing the registrar itself, so now we can just get those from the registrar on demand. - Removes nil player handling; the player can never return nil now. - Removes `formatHint` from the Pigeon interface; it was only ever used on Android, so doesn't need to be sent across the language boundary. 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.
1 parent 792b70b commit 5d2a10f

File tree

17 files changed

+261
-324
lines changed

17 files changed

+261
-324
lines changed

packages/video_player/video_player_avfoundation/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
## 2.8.1
2+
3+
* Restructures internal logic to move more code to Dart.
4+
15
## 2.8.0
26

37
* Adds platform view support for macOS.

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

Lines changed: 36 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -168,32 +168,6 @@ - (instancetype)init {
168168

169169
@implementation VideoPlayerTests
170170

171-
- (void)testCreateWithOptionsReturnsErrorForInvalidAssetPath {
172-
NSObject<FlutterPluginRegistrar> *registrar = OCMProtocolMock(@protocol(FlutterPluginRegistrar));
173-
OCMStub([registrar lookupKeyForAsset:[OCMArg any]]).andReturn(nil);
174-
FVPVideoPlayerPlugin *videoPlayerPlugin =
175-
[[FVPVideoPlayerPlugin alloc] initWithRegistrar:registrar];
176-
177-
FlutterError *initializationError;
178-
[videoPlayerPlugin initialize:&initializationError];
179-
XCTAssertNil(initializationError);
180-
181-
FVPCreationOptions *create =
182-
[FVPCreationOptions makeWithAsset:@"invalid/path/to/asset"
183-
uri:nil
184-
packageName:nil
185-
formatHint:nil
186-
httpHeaders:@{}
187-
viewType:FVPPlatformVideoViewTypeTextureView];
188-
189-
FlutterError *createError;
190-
NSNumber *playerIdentifier = [videoPlayerPlugin createWithOptions:create error:&createError];
191-
192-
XCTAssertNil(playerIdentifier);
193-
XCTAssertNotNil(createError);
194-
XCTAssertEqualObjects(createError.code, @"video_player");
195-
}
196-
197171
- (void)testBlankVideoBugWithEncryptedVideoStreamAndInvertedAspectRatioBugForSomeVideoStream {
198172
// This is to fix 2 bugs: 1. blank video for encrypted video streams on iOS 16
199173
// (https://github.com/flutter/flutter/issues/111457) and 2. swapped width and height for some
@@ -218,12 +192,9 @@ - (void)testBlankVideoBugWithEncryptedVideoStreamAndInvertedAspectRatioBugForSom
218192
XCTAssertNil(error);
219193

220194
FVPCreationOptions *create = [FVPCreationOptions
221-
makeWithAsset:nil
222-
uri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/bee.mp4"
223-
packageName:nil
224-
formatHint:nil
225-
httpHeaders:@{}
226-
viewType:FVPPlatformVideoViewTypeTextureView];
195+
makeWithUri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/bee.mp4"
196+
httpHeaders:@{}
197+
viewType:FVPPlatformVideoViewTypeTextureView];
227198
NSNumber *playerIdentifier = [videoPlayerPlugin createWithOptions:create error:&error];
228199
XCTAssertNil(error);
229200
XCTAssertNotNil(playerIdentifier);
@@ -253,12 +224,9 @@ - (void)testPlayerForPlatformViewDoesNotRegisterTexture {
253224
[videoPlayerPlugin initialize:&initalizationError];
254225
XCTAssertNil(initalizationError);
255226
FVPCreationOptions *create = [FVPCreationOptions
256-
makeWithAsset:nil
257-
uri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/hls/bee.m3u8"
258-
packageName:nil
259-
formatHint:nil
260-
httpHeaders:@{}
261-
viewType:FVPPlatformVideoViewTypePlatformView];
227+
makeWithUri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/hls/bee.m3u8"
228+
httpHeaders:@{}
229+
viewType:FVPPlatformVideoViewTypePlatformView];
262230
FlutterError *createError;
263231
[videoPlayerPlugin createWithOptions:create error:&createError];
264232

@@ -284,12 +252,9 @@ - (void)testSeekToWhilePausedStartsDisplayLinkTemporarily {
284252
[videoPlayerPlugin initialize:&initalizationError];
285253
XCTAssertNil(initalizationError);
286254
FVPCreationOptions *create = [FVPCreationOptions
287-
makeWithAsset:nil
288-
uri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/hls/bee.m3u8"
289-
packageName:nil
290-
formatHint:nil
291-
httpHeaders:@{}
292-
viewType:FVPPlatformVideoViewTypeTextureView];
255+
makeWithUri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/hls/bee.m3u8"
256+
httpHeaders:@{}
257+
viewType:FVPPlatformVideoViewTypeTextureView];
293258
FlutterError *createError;
294259
NSNumber *playerIdentifier = [videoPlayerPlugin createWithOptions:create error:&createError];
295260
FVPTextureBasedVideoPlayer *player =
@@ -344,12 +309,9 @@ - (void)testInitStartsDisplayLinkTemporarily {
344309
[videoPlayerPlugin initialize:&initalizationError];
345310
XCTAssertNil(initalizationError);
346311
FVPCreationOptions *create = [FVPCreationOptions
347-
makeWithAsset:nil
348-
uri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/hls/bee.m3u8"
349-
packageName:nil
350-
formatHint:nil
351-
httpHeaders:@{}
352-
viewType:FVPPlatformVideoViewTypeTextureView];
312+
makeWithUri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/hls/bee.m3u8"
313+
httpHeaders:@{}
314+
viewType:FVPPlatformVideoViewTypeTextureView];
353315
FlutterError *createError;
354316
NSNumber *playerIdentifier = [videoPlayerPlugin createWithOptions:create error:&createError];
355317

@@ -393,12 +355,9 @@ - (void)testSeekToWhilePlayingDoesNotStopDisplayLink {
393355
[videoPlayerPlugin initialize:&initalizationError];
394356
XCTAssertNil(initalizationError);
395357
FVPCreationOptions *create = [FVPCreationOptions
396-
makeWithAsset:nil
397-
uri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/hls/bee.m3u8"
398-
packageName:nil
399-
formatHint:nil
400-
httpHeaders:@{}
401-
viewType:FVPPlatformVideoViewTypeTextureView];
358+
makeWithUri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/hls/bee.m3u8"
359+
httpHeaders:@{}
360+
viewType:FVPPlatformVideoViewTypeTextureView];
402361
FlutterError *createError;
403362
NSNumber *playerIdentifier = [videoPlayerPlugin createWithOptions:create error:&createError];
404363
FVPTextureBasedVideoPlayer *player =
@@ -451,12 +410,9 @@ - (void)testPauseWhileWaitingForFrameDoesNotStopDisplayLink {
451410
[videoPlayerPlugin initialize:&initalizationError];
452411
XCTAssertNil(initalizationError);
453412
FVPCreationOptions *create = [FVPCreationOptions
454-
makeWithAsset:nil
455-
uri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/hls/bee.m3u8"
456-
packageName:nil
457-
formatHint:nil
458-
httpHeaders:@{}
459-
viewType:FVPPlatformVideoViewTypeTextureView];
413+
makeWithUri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/hls/bee.m3u8"
414+
httpHeaders:@{}
415+
viewType:FVPPlatformVideoViewTypeTextureView];
460416
FlutterError *createError;
461417
NSNumber *playerIdentifier = [videoPlayerPlugin createWithOptions:create error:&createError];
462418
FVPTextureBasedVideoPlayer *player =
@@ -481,12 +437,9 @@ - (void)testDeregistersFromPlayer {
481437
XCTAssertNil(error);
482438

483439
FVPCreationOptions *create = [FVPCreationOptions
484-
makeWithAsset:nil
485-
uri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/bee.mp4"
486-
packageName:nil
487-
formatHint:nil
488-
httpHeaders:@{}
489-
viewType:FVPPlatformVideoViewTypeTextureView];
440+
makeWithUri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/bee.mp4"
441+
httpHeaders:@{}
442+
viewType:FVPPlatformVideoViewTypeTextureView];
490443
NSNumber *playerIdentifier = [videoPlayerPlugin createWithOptions:create error:&error];
491444
XCTAssertNil(error);
492445
XCTAssertNotNil(playerIdentifier);
@@ -513,12 +466,9 @@ - (void)testBufferingStateFromPlayer {
513466
XCTAssertNil(error);
514467

515468
FVPCreationOptions *create = [FVPCreationOptions
516-
makeWithAsset:nil
517-
uri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/bee.mp4"
518-
packageName:nil
519-
formatHint:nil
520-
httpHeaders:@{}
521-
viewType:FVPPlatformVideoViewTypeTextureView];
469+
makeWithUri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/bee.mp4"
470+
httpHeaders:@{}
471+
viewType:FVPPlatformVideoViewTypeTextureView];
522472
NSNumber *playerIdentifier = [videoPlayerPlugin createWithOptions:create error:&error];
523473
XCTAssertNil(error);
524474
XCTAssertNotNil(playerIdentifier);
@@ -709,12 +659,9 @@ - (void)testDoesNotCrashOnRateObservationAfterDisposal {
709659
XCTAssertNil(error);
710660

711661
FVPCreationOptions *create = [FVPCreationOptions
712-
makeWithAsset:nil
713-
uri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/bee.mp4"
714-
packageName:nil
715-
formatHint:nil
716-
httpHeaders:@{}
717-
viewType:FVPPlatformVideoViewTypeTextureView];
662+
makeWithUri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/bee.mp4"
663+
httpHeaders:@{}
664+
viewType:FVPPlatformVideoViewTypeTextureView];
718665
NSNumber *playerIdentifier = [videoPlayerPlugin createWithOptions:create error:&error];
719666
XCTAssertNil(error);
720667
XCTAssertNotNil(playerIdentifier);
@@ -763,12 +710,9 @@ - (void)testHotReloadDoesNotCrash {
763710
XCTAssertNil(error);
764711

765712
FVPCreationOptions *create = [FVPCreationOptions
766-
makeWithAsset:nil
767-
uri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/bee.mp4"
768-
packageName:nil
769-
formatHint:nil
770-
httpHeaders:@{}
771-
viewType:FVPPlatformVideoViewTypeTextureView];
713+
makeWithUri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/bee.mp4"
714+
httpHeaders:@{}
715+
viewType:FVPPlatformVideoViewTypeTextureView];
772716
NSNumber *playerIdentifier = [videoPlayerPlugin createWithOptions:create error:&error];
773717
XCTAssertNil(error);
774718
XCTAssertNotNil(playerIdentifier);
@@ -830,13 +774,9 @@ - (void)testFailedToLoadVideoEventShouldBeAlwaysSent {
830774

831775
[videoPlayerPlugin initialize:&error];
832776

833-
FVPCreationOptions *create =
834-
[FVPCreationOptions makeWithAsset:nil
835-
uri:@""
836-
packageName:nil
837-
formatHint:nil
838-
httpHeaders:@{}
839-
viewType:FVPPlatformVideoViewTypeTextureView];
777+
FVPCreationOptions *create = [FVPCreationOptions makeWithUri:@""
778+
httpHeaders:@{}
779+
viewType:FVPPlatformVideoViewTypeTextureView];
840780
NSNumber *playerIdentifier = [videoPlayerPlugin createWithOptions:create error:&error];
841781
FVPVideoPlayer *player = videoPlayerPlugin.playersByIdentifier[playerIdentifier];
842782
XCTAssertNotNil(player);
@@ -896,12 +836,9 @@ - (void)testPlayerShouldNotDropEverySecondFrame {
896836
[videoPlayerPlugin initialize:&error];
897837
XCTAssertNil(error);
898838
FVPCreationOptions *create = [FVPCreationOptions
899-
makeWithAsset:nil
900-
uri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/bee.mp4"
901-
packageName:nil
902-
formatHint:nil
903-
httpHeaders:@{}
904-
viewType:FVPPlatformVideoViewTypeTextureView];
839+
makeWithUri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/bee.mp4"
840+
httpHeaders:@{}
841+
viewType:FVPPlatformVideoViewTypeTextureView];
905842
NSNumber *playerIdentifier = [videoPlayerPlugin createWithOptions:create error:&error];
906843
FVPTextureBasedVideoPlayer *player =
907844
(FVPTextureBasedVideoPlayer *)videoPlayerPlugin.playersByIdentifier[playerIdentifier];

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

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,18 +31,6 @@ @interface FVPTextureBasedVideoPlayer ()
3131
@end
3232

3333
@implementation FVPTextureBasedVideoPlayer
34-
- (instancetype)initWithAsset:(NSString *)asset
35-
frameUpdater:(FVPFrameUpdater *)frameUpdater
36-
displayLink:(NSObject<FVPDisplayLink> *)displayLink
37-
avFactory:(id<FVPAVFactory>)avFactory
38-
viewProvider:(NSObject<FVPViewProvider> *)viewProvider {
39-
return [self initWithURL:[NSURL fileURLWithPath:[FVPVideoPlayer absolutePathForAssetName:asset]]
40-
frameUpdater:frameUpdater
41-
displayLink:displayLink
42-
httpHeaders:@{}
43-
avFactory:avFactory
44-
viewProvider:viewProvider];
45-
}
4634

4735
- (instancetype)initWithURL:(NSURL *)url
4836
frameUpdater:(FVPFrameUpdater *)frameUpdater

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

Lines changed: 4 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,6 @@
1717
static void *rateContext = &rateContext;
1818

1919
@implementation FVPVideoPlayer
20-
- (instancetype)initWithAsset:(NSString *)asset
21-
avFactory:(id<FVPAVFactory>)avFactory
22-
viewProvider:(NSObject<FVPViewProvider> *)viewProvider {
23-
return [self initWithURL:[NSURL fileURLWithPath:[FVPVideoPlayer absolutePathForAssetName:asset]]
24-
httpHeaders:@{}
25-
avFactory:avFactory
26-
viewProvider:viewProvider];
27-
}
2820

2921
- (instancetype)initWithURL:(NSURL *)url
3022
httpHeaders:(nonnull NSDictionary<NSString *, NSString *> *)headers
@@ -122,19 +114,6 @@ - (void)dispose {
122114
[_eventChannel setStreamHandler:nil];
123115
}
124116

125-
+ (NSString *)absolutePathForAssetName:(NSString *)assetName {
126-
NSString *path = [[NSBundle mainBundle] pathForResource:assetName ofType:nil];
127-
#if TARGET_OS_OSX
128-
// See https://github.com/flutter/flutter/issues/135302
129-
// TODO(stuartmorgan): Remove this if the asset APIs are adjusted to work better for macOS.
130-
if (!path) {
131-
path = [NSURL URLWithString:assetName relativeToURL:NSBundle.mainBundle.bundleURL].path;
132-
}
133-
#endif
134-
135-
return path;
136-
}
137-
138117
- (void)addObserversForItem:(AVPlayerItem *)item player:(AVPlayer *)player {
139118
[item addObserver:self
140119
forKeyPath:@"loadedTimeRanges"
@@ -250,8 +229,10 @@ - (void)observeValueForKeyPath:(NSString *)path
250229
NSMutableArray<NSArray<NSNumber *> *> *values = [[NSMutableArray alloc] init];
251230
for (NSValue *rangeValue in [object loadedTimeRanges]) {
252231
CMTimeRange range = [rangeValue CMTimeRangeValue];
253-
int64_t start = FVPCMTimeToMillis(range.start);
254-
[values addObject:@[ @(start), @(start + FVPCMTimeToMillis(range.duration)) ]];
232+
[values addObject:@[
233+
@(FVPCMTimeToMillis(range.start)),
234+
@(FVPCMTimeToMillis(range.duration)),
235+
]];
255236
}
256237
_eventSink(@{@"event" : @"bufferingUpdate", @"values" : values});
257238
}

0 commit comments

Comments
 (0)