[video_player_avfoundation] Implement preventsDisplaySleepDuringVideoPlayback#11547
[video_player_avfoundation] Implement preventsDisplaySleepDuringVideoPlayback#11547shrabanti722 wants to merge 3 commits into
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements the setPreventsDisplaySleepDuringVideoPlayback feature for the AVFoundation video player, enabling control over display sleep during playback. The changes include updates to Pigeon message definitions, the Dart platform implementation, and the native Objective-C code. Feedback indicates that the implementation lacks required unit or integration tests as specified in the repository style guide.
| Future<void> setPreventsDisplaySleepDuringVideoPlayback( | ||
| int playerId, | ||
| bool preventsDisplaySleepDuringVideoPlayback, | ||
| ) { | ||
| return _playerWith(id: playerId).setPreventsDisplaySleepDuringVideoPlayback( | ||
| preventsDisplaySleepDuringVideoPlayback, | ||
| ); | ||
| } |
There was a problem hiding this comment.
This PR implements a new feature setPreventsDisplaySleepDuringVideoPlayback, but it does not include any unit or integration tests to verify the implementation. Per the repository style guide, all new code should be tested. Please add tests in packages/video_player/video_player_avfoundation/test/ to ensure that the platform implementation correctly calls the Pigeon API and that the native side behaves as expected.
…Playback Implements the platform interface method `setPreventsDisplaySleepDuringVideoPlayback` using `AVPlayer.preventsDisplaySleepDuringVideoPlayback` on iOS and macOS. AVFoundation implementation breakout PR for flutter#11225. Depends on the platform interface breakout PR (6.7.0). Made-with: Cursor
shrabanti722
force-pushed
the
video-player-prevents-display-sleep-avfoundation
branch
from
431a14a to
b53cddc
Compare
There was a problem hiding this comment.
Code Review
This pull request implements setPreventsDisplaySleepDuringVideoPlayback for the AVFoundation video player implementation, allowing control over display sleep during video playback. The changes include Pigeon-generated message updates, Dart and Objective-C platform code additions, and corresponding unit tests. Feedback on the changes suggests removing a redundant @available check in FVPVideoPlayer.m since the package's minimum supported versions are already higher than the check specifies.
| - (void)setPreventsDisplaySleepDuringVideoPlayback:(BOOL)preventsDisplaySleepDuringVideoPlayback error:(FlutterError *_Nullable *_Nonnull)error { | ||
| if (@available(iOS 12.0, macOS 10.14, *)) { | ||
| self.player.preventsDisplaySleepDuringVideoPlayback = preventsDisplaySleepDuringVideoPlayback; | ||
| } | ||
| } |
There was a problem hiding this comment.
Since the minimum supported versions for this package are iOS 13.0 and macOS 10.15 (as of version 2.8.5), the @available(iOS 12.0, macOS 10.14, *) check is redundant. We can safely remove it to simplify the code.
- (void)setPreventsDisplaySleepDuringVideoPlayback:(BOOL)preventsDisplaySleepDuringVideoPlayback error:(FlutterError *_Nullable *_Nonnull)error {
self.player.preventsDisplaySleepDuringVideoPlayback = preventsDisplaySleepDuringVideoPlayback;
}…tion Resolves conflicts caused by the avfoundation Pigeon and source-directory restructure on main: - Drops stale pigeons/messages.dart, lib/src/messages.g.dart, and the old messages.g.h (the file split into instance/plugin pigeons under video_player_avfoundation_objc/). - Re-applies setPreventsDisplaySleepDuringVideoPlayback to pigeons/video_player_instance_messages.dart. - Accepts upstream copies of regenerated files (VideoPlayerInstanceMessages.g.m, avfoundation_video_player_test.mocks.dart) — will be regenerated next. - Bumps version 2.10.0 -> 2.11.0 and platform_interface dep ^6.7.0 -> ^6.8.0.
- Regenerates VideoPlayerInstanceMessages.g.{h,m,dart} so the new
setPreventsDisplaySleepDuringVideoPlayback method is plumbed through.
- Regenerates avfoundation_video_player_test.mocks.dart to include the new
method's mock entry.
- clang-formats the regenerated ObjC files.
- dart format --page-width 100 across lib/, test/, pigeons/.
- Adds the new pigeon method to video_player_instance_messages.dart.
|
@stuartmorgan-g @tarrinneal this is the AVFoundation child PR for the screen auto-lock change. |
1 participant
Description
Implements the
setPreventsDisplaySleepDuringVideoPlaybackplatform interface method usingAVPlayer.preventsDisplaySleepDuringVideoPlaybackon iOS and macOS.AVFoundation implementation breakout PR for #11225.
Dependencies
video_player_platform_interface6.7.0 breakout PR. CI will fail until that PR lands and the new interface version is published. Keeping this PR in draft until then.Changes
setPreventsDisplaySleepDuringVideoPlaybackinFVPVideoPlayer.mand wires it through the Dart platform implementation.video_player_avfoundationto 2.10.0 and updates thevideo_player_platform_interfaceconstraint to ^6.7.0.Related