-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[video_player_avfoundation] send video load failure even when eventsink was initialized late #7194
Changes from all commits
16d3d7a
12a061e
902c3e6
4ab3c53
1e0964f
89db486
1a6fd6f
60330dc
01b21ac
ab57c33
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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]; | ||
break; | ||
case AVPlayerItemStatusUnknown: | ||
break; | ||
|
@@ -406,6 +400,32 @@ - (void)updatePlayingState { | |
_displayLink.running = _isPlaying || self.waitingForFrame; | ||
} | ||
|
||
- (void)sendFailedToLoadVideoEvent { | ||
if (_eventSink == nil) { | ||
return; | ||
} | ||
// Prefer more detailed error information from tracks loading. | ||
NSError *error; | ||
if ([self.player.currentItem.asset statusOfValueForKey:@"tracks" | ||
error:&error] != AVKeyValueStatusFailed) { | ||
error = self.player.currentItem.error; | ||
} | ||
__block NSMutableOrderedSet<NSString *> *details = | ||
[NSMutableOrderedSet orderedSetWithObject:@"Failed to load video"]; | ||
void (^add)(NSString *) = ^(NSString *detail) { | ||
if (detail != nil) { | ||
[details addObject:detail]; | ||
} | ||
}; | ||
NSError *underlyingError = error.userInfo[NSUnderlyingErrorKey]; | ||
add(error.localizedDescription); | ||
add(error.localizedFailureReason); | ||
add(underlyingError.localizedDescription); | ||
add(underlyingError.localizedFailureReason); | ||
NSString *message = [details.array componentsJoinedByString:@": "]; | ||
_eventSink([FlutterError errorWithCode:@"VideoError" message:message details:nil]); | ||
} | ||
|
||
- (void)setupEventSinkIfReadyToPlay { | ||
if (_eventSink && !_isInitialized) { | ||
AVPlayerItem *currentItem = self.player.currentItem; | ||
|
@@ -587,6 +607,13 @@ - (FlutterError *_Nullable)onListenWithArguments:(id _Nullable)arguments | |
// This line ensures the 'initialized' event is sent when the event | ||
// 'AVPlayerItemStatusReadyToPlay' fires before _eventSink is set (this function | ||
// onListenWithArguments is called) | ||
// and also send error in similar case with 'AVPlayerItemStatusFailed' | ||
// https://github.com/flutter/flutter/issues/151475 | ||
// https://github.com/flutter/flutter/issues/147707 | ||
if (self.player.currentItem.status == AVPlayerItemStatusFailed) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So this is the part that actually fix the bug? Can you add a comment explaining how we ended up with Failed state here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry I did not manage to proceed with this. Yes when |
||
[self sendFailedToLoadVideoEvent]; | ||
return nil; | ||
} | ||
[self setupEventSinkIfReadyToPlay]; | ||
return nil; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
by "similar case", do you mean AVPlayerItemStatusReadyToPlay fires before _eventSink is set? Can you be more explicit here in the comment? (Sorry I have some trouble understanding this bug fix)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I was following up with that comment above. But this time with
AVPlayerItemStatusFailed
instead ofAVPlayerItemStatusReadyToPlay
. If anything fires before is event sink set then it could not be sent. Would that comment be better if I added "in similar case as above"?