packages
packages copied to clipboard
[video_player_avfoundation] send video load failure even when eventsink was initialized late
Event initialized is sent from onListenWithArguments by calling setupEventSinkIfReadyToPlay if event sink was nil during observeValueForKeyPath. This change also sends failure in a similar way. Adds more details to the error message returned by VideoPlayerController.initialize().
- https://github.com/flutter/flutter/issues/151475
- https://github.com/flutter/flutter/issues/147707
- https://github.com/flutter/flutter/issues/56665 - now error message may look like:
Failed to load video: Operation Stopped: The server is not correctly configured.: The operation couldn’t be completed. (CoreMediaErrorDomain error -12939 - byte range and no content length - error code is 200)
Tests testSeekToWhilePausedStartsDisplayLinkTemporarily and testSeekToWhilePlayingDoesNotStopDisplayLink are failing on the main branch on ios. Seems position is correctly set to 1234 in seekTo completion handler but right after [mockDisplayLink setRunning:YES] is set again to 0 and after some time back to 1234 so those two tests sometimes fail (seems more often when running all tests).
Pre-launch Checklist
- [x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
- [x] I read the Tree Hygiene page, which explains my responsibilities.
- [x] I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use
dart format.) - [x] I signed the CLA.
- [x] The title of the PR starts with the name of the package surrounded by square brackets, e.g.
[shared_preferences] - [x] I linked to at least one issue that this PR fixes in the description above.
- [x] I updated
pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes. - [x] I updated
CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or this PR is exempt from CHANGELOG changes. - [x] I updated/added relevant documentation (doc comments with
///). - [x] I added new tests to check the change I am making, or this PR is test-exempt.
- [x] All existing and new tests are passing.
If you need help, consider asking for advice on the #hackers-new channel on Discord.
I have a hard time understanding this in the PR description:
Sends also failure similarly as is sent event initialized from onListenWithArguments through setupEventSinkIfReadyToPlay if event sink was not initialized during observeValueForKeyPath.
Could you rephrase a bit? That'd be helpful for a review. Thanks!
@hellohuanlin Please add tarrinneal (the ecosystem-level CODEOWNER) as the secondary reviewer once this has passed iOS review; the iOS team review is primary since this is internal to the iOS implementation.
I forgot to prepend those 3 issues with fixes. They can probably be closed now (or at least those which this PR covers in full).
Thanks, I closed one, and left a comment in another that is likely fixed but needs re-testing (unless you've already tested that one? If so we can close it). For the third it's not clear to me whether it's fully resolved.
That first which you closed actually contained another behaviour which is not covered by this PR. But I think it is rather not a fault in the package or flutter sdk but rather with something how flutter runs apps on ios devices. Seems sometimes the app "disappears" after running but after closer inspection it seems it was just probably somehow "pushed" to background.
I probably tested the second with some text file but I am not sure and probably not with that original file and probably not on macos either. Anyway the best would be confirmation from the author.
That third is a mess. Seems there are several different groups who interpret that issue differently. Sticking to what author wrote in description and repeated in comments seems that the issue is just about better error message https://github.com/flutter/flutter/issues/56665#issuecomment-1280557707. So the question is whether issue should be interpreted differently if a lot of users expect that that issue should be closed by fixing underlying behaviour (which probably cannot be done from flutter or video_player side as it does not have anything to do with http communication here, if of course there is not some flag somewhere in AVFoundation). Maybe better to ask the author also here (or maybe wait and see if any new comments are added in next few months).