[video_player_avfoundation] Implement preventsDisplaySleepDuringVideoPlayback#11547
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.
…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
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. |
hellohuanlin
left a comment
hellohuanlin
left a comment
There was a problem hiding this comment.
Seems to be a straight forward change, but it's missing unit tests on native side.
|
@hellohuanlin In the future, please try to check for missing tests when reviewing the combo PR. Part of the goal of getting all necessary approvals on the combo PR before splitting out the sub PRs is to have the sub PRs all land very quickly so that we don't end up in a limbo where features are partially landed for extended periods of time. |
|
|
||
| - (void)setPreventsDisplaySleepDuringVideoPlayback:(BOOL)preventsDisplaySleepDuringVideoPlayback | ||
| error:(FlutterError *_Nullable *_Nonnull)error { | ||
| if (@available(iOS 12.0, macOS 10.14, *)) { |
There was a problem hiding this comment.
Why are you doing runtime checks for OS versions that are lower than our minimum compilation targets? This condition can never be false.
- Remove the redundant @available(iOS 12.0, macOS 10.14, *) check in setPreventsDisplaySleepDuringVideoPlayback; the package minimums are iOS 13 / macOS 10.15, so the condition can never be false. - Add a native Swift unit test verifying the setter updates the underlying AVPlayer in both directions.
|
@shrabanti722 It looks like you either didn't run the repo |
Yeah @stuartmorgan-g I actually came on a vacation and don’t have my Mac with me, so couldn’t run it locally, I used a windows laptop for this since it was getting delayed. But looks like I will be able to fix it only after I come back :( which will be 5 - 6 days. Sorry for that. |
|
hey @stuartmorgan-g, thanks for your commit. I am back. Anything pending from my side for this PR ? |
No, this is just waiting for a second approval from @hellohuanlin or @LouiseHsu. |
hellohuanlin
added
the
autosubmit
…ished versions Now that flutter#11546 (video_player_platform_interface 6.8.0) and flutter#11547 (video_player_avfoundation 2.11.0) have landed, drop the landed child-PR changes from this branch, remove the 'FOR TESTING AND INITIAL REVIEW ONLY' dependency_overrides, and depend on the published versions instead.
…11225) ## Description Adds support for screen auto-lock during video playback on iOS. By default, the video player keeps the screen awake during playback. Some apps (e.g. login screens with background videos, ambient content) want the screen to be able to auto-lock while video plays. This PR adds an option to allow that behavior. ### Changes * Add `preventsDisplaySleepDuringVideoPlayback` to `VideoPlayerOptions` (default: `true` to preserve existing behavior) * When `false`, the screen can auto-lock during playback * Implemented on iOS/macOS via `AVPlayer.preventsDisplaySleepDuringVideoPlayback` * Platform interface: `setPreventsDisplaySleepDuringVideoPlayback(int playerId, bool preventsDisplaySleepDuringVideoPlayback)` ### Usage ```dart VideoPlayerController.networkUrl( url, videoPlayerOptions: VideoPlayerOptions(preventsDisplaySleepDuringVideoPlayback: false), ); ``` ## Breakout PRs 1. Platform interface: #11546 2. AVFoundation (iOS/macOS): #11547 ## Issues fixed * Fixes: [flutter/flutter#183520](flutter/flutter#183520) (How to disable wakelock on iOS side?) * Addresses the need to allow screen auto-lock during video playback on iOS (e.g. for ambient/background videos) ## Pre-Review Checklist
…er#189387) flutter/packages@20928d5...ad2eab1 2026-07-13 [email protected] [camera] Guard CameraController value updates after dispose (flutter/packages#11861) 2026-07-13 [email protected] [material_ui] Remove unused example files (flutter/packages#12179) 2026-07-13 [email protected] [google_maps_flutter_platform_interface] Adds support for mapTypeControlEnabled, fullscreenControlEnabled, and streetViewControlEnabled for web (flutter/packages#12191) 2026-07-13 [email protected] [pigeon] Report a clear error for enhanced enums (flutter/packages#11880) 2026-07-13 49699333+dependabot[bot]@users.noreply.github.com Bump actions/labeler from 6.1.0 to 6.2.0 in the all-github-actions group (flutter/packages#12190) 2026-07-13 [email protected] [shared_preferences] Convert legacy codepath to Kotlin (flutter/packages#12139) 2026-07-12 [email protected] Roll Flutter from f7b66f3 to cf9e8af (11 revisions) (flutter/packages#12188) 2026-07-11 [email protected] [video_player] Improve seek performance on Android (flutter/packages#11810) 2026-07-11 [email protected] [pigeon] Disable iOS test parallelization (flutter/packages#12177) 2026-07-11 [email protected] [material_ui, cupertino_ui] Migrate snippet TODOs to `<callout-box>` (flutter/packages#12146) 2026-07-11 [email protected] [cupertino_ui] Fix example path (flutter/packages#12151) 2026-07-10 [email protected] Roll Flutter from dc2a870 to f7b66f3 (12 revisions) (flutter/packages#12175) 2026-07-10 [email protected] Roll Flutter (stable) from f94f4fc to ee80f08 (3 revisions) (flutter/packages#12171) 2026-07-10 [email protected] Manual roll Flutter from 91939cc to dc2a870 (50 revisions) (flutter/packages#12169) 2026-07-09 [email protected] Update `legacy` test to be in Warn Range (flutter/packages#12168) 2026-07-09 [email protected] [material_ui, cupertino_ui] Migrate dartpad TODOs to `<callout-box>` (flutter/packages#12120) 2026-07-09 [email protected] [video_player_avfoundation] Implement preventsDisplaySleepDuringVideoPlayback (flutter/packages#11547) 2026-07-09 [email protected] [pigeon] Add usage docs to generated Dart event channel methods (flutter/packages#11894) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
3 participants
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