Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[video_player] Wait to initialize m3u8 videos on iOS until size is set #4727

Merged
fluttergithubbot merged 2 commits into from
Feb 4, 2022
Merged

[video_player] Wait to initialize m3u8 videos on iOS until size is set #4727

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5d80f3d
[video_player] Wait to initialize m3u8 videos on iOS until size is set
jmagman Feb 2, 2022
9707baa
Review edits
jmagman Feb 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/video_player/video_player_avfoundation/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## NEXT
## 2.2.18

* Wait to initialize m3u8 videos until size is set, fixing aspect ratio

@AlexV525 AlexV525 Feb 3, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Wait to initialize m3u8 videos until size is set, fixing aspect ratio
* Wait to initialize m3u8 videos until size is set, fixing aspect ratio.

@jmagman jmagman Feb 3, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

* Adjusts test timeouts for network-dependent native tests to avoid flake.

## 2.2.17
Expand Down
4 changes: 2 additions & 2 deletions packages/video_player/video_player_avfoundation/example/ios/Runner.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
archiveVersion = 1;
classes = {
};
objectVersion = 50;
objectVersion = 46;

@jmagman jmagman Feb 3, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let Xcode touch the project.

objects = {

/* Begin PBXBuildFile section */
Expand Down Expand Up @@ -269,7 +269,7 @@
97C146E61CF9000F007C117D /* Project object */ = {
isa = PBXProject;
attributes = {
LastUpgradeCheck = 1300;
LastUpgradeCheck = 1320;
ORGANIZATIONNAME = "The Flutter Authors";
TargetAttributes = {
97C146ED1CF9000F007C117D = {
Expand Down
2 changes: 1 addition & 1 deletion ...o_player_avfoundation/example/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
<Scheme
LastUpgradeVersion = "1300"
LastUpgradeVersion = "1320"
version = "1.3">
<BuildAction
parallelizeBuildables = "YES"
Expand Down
16 changes: 16 additions & 0 deletions packages/video_player/video_player_avfoundation/example/ios/RunnerTests/VideoPlayerTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,22 @@ - (void)testAudioControls {
XCTAssertEqualWithAccuracy([audioInitialization[@"duration"] intValue], 5400, 200);
}

- (void)testHlsControls {

@stuartmorgan-g stuartmorgan-g Feb 3, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

testHLSControls, since it's ObjC.

@jmagman jmagman Feb 3, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

NSObject<FlutterPluginRegistry> *registry =
(NSObject<FlutterPluginRegistry> *)[[UIApplication sharedApplication] delegate];
NSObject<FlutterPluginRegistrar> *registrar = [registry registrarForPlugin:@"TestHlsControls"];

FLTVideoPlayerPlugin *videoPlayerPlugin =
(FLTVideoPlayerPlugin *)[[FLTVideoPlayerPlugin alloc] initWithRegistrar:registrar];

NSDictionary<NSString *, id> *videoInitialization =
[self testPlugin:videoPlayerPlugin
uri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/hls/bee.m3u8"];
XCTAssertEqualObjects(videoInitialization[@"height"], @720);
XCTAssertEqualObjects(videoInitialization[@"width"], @1280);
XCTAssertEqualWithAccuracy([videoInitialization[@"duration"] intValue], 4000, 200);
}

- (NSDictionary<NSString *, id> *)testPlugin:(FLTVideoPlayerPlugin *)videoPlayerPlugin
uri:(NSString *)uri {
FlutterError *error;
Expand Down
6 changes: 4 additions & 2 deletions packages/video_player/video_player_avfoundation/ios/Classes/FLTVideoPlayerPlugin.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,9 +354,11 @@ - (void)setupEventSinkIfReadyToPlay {
}

BOOL hasVideoTracks = [asset tracksWithMediaType:AVMediaTypeVideo].count != 0;
BOOL hasNoTracks = asset.tracks.count == 0;

// The player has not yet initialized when it contains video tracks.
if (hasVideoTracks && height == CGSizeZero.height && width == CGSizeZero.width) {
// The player has not yet initialized when it has no size, unless it is an audio-only track.
// HLS m3u8 video files never load any tracks, and are also not yet initialized until they have a size.

@AlexV525 AlexV525 Feb 3, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible that any other formats would suffer from this?

@jmagman jmagman Feb 3, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly, we have limited testing for the range of possible video types.
At least now it will be no different than before #4639 in terms of file formats that don't have tracks.

if ((hasVideoTracks || hasNoTracks) && height == CGSizeZero.height && width == CGSizeZero.width) {
return;
}
// The player may be initialized but still needs to determine the duration.
Expand Down
2 changes: 1 addition & 1 deletion packages/video_player/video_player_avfoundation/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: video_player_avfoundation
description: iOS implementation of the video_player plugin.
repository: https://github.com/flutter/plugins/tree/master/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.2.17
version: 2.2.18

@jmagman jmagman Feb 3, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stuartmorgan is there anything else I need to do to version this federated plugin?

@stuartmorgan-g stuartmorgan-g Feb 3, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we don't push version changes through to the app-facing plugin; the expectation is that people are updating transitive dependencies.


environment:
sdk: ">=2.14.0 <3.0.0"
Expand Down