[video_player] Add forwardBufferDuration to cap network read-ahead#11950
[video_player] Add forwardBufferDuration to cap network read-ahead#11950mhmdfathy96 wants to merge 2 commits into
Conversation
Adds an optional VideoPlayerOptions.forwardBufferDuration so apps can limit how far ahead of the playhead the player buffers from the network. Upstream defaults to aggressive read-ahead (ExoPlayer ~50s; AVPlayer may fetch most of the file), which wastes bandwidth when a user abandons or seeks away. Requested in flutter/flutter#40931, #141511, #163918. null preserves current behavior. Threaded through VideoCreationOptions and the pigeon CreationOptions to: - Android: ExoPlayer DefaultLoadControl (VideoPlayer.cappedLoadControl, clamped playback-start thresholds). - iOS/macOS: AVPlayerItem.preferredForwardBufferDuration. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QywXM9rpYfg579LrEFqTfw
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
- Dart: umbrella passes the option into VideoCreationOptions; android & avfoundation pass forwardBufferDurationMs (and default to null). - Android native: VideoPlayer.cappedLoadControl null/non-null + threshold clamping; fixes existing VideoPlayerPluginTest for the new create() arity. - iOS native: forwardBufferDuration applied to the player item / left untouched when null. Also exposes preferredForwardBufferDuration on the FVPAVPlayerItem abstraction (protocol + default impl) so the Swift plugin can set it and tests can verify it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QywXM9rpYfg579LrEFqTfw
There was a problem hiding this comment.
Code Review
This pull request adds support for a new forwardBufferDuration option to VideoPlayerOptions and VideoCreationOptions across the video_player packages, allowing users to cap network buffering. The implementation spans the platform interface, Android (ExoPlayer), and iOS/macOS (AVFoundation) implementations, along with corresponding tests. Feedback suggests adding non-negative assertions to the Dart constructors, clamping the duration on Android to prevent integer overflow, and clamping it on iOS/macOS to prevent potential crashes from negative values.
| // nil leaves AVPlayer's automatic (aggressive) buffering; a value (seconds) | ||
| // limits read-ahead to save bandwidth. | ||
| if let forwardBufferDurationMs = options.forwardBufferDurationMs { | ||
| item.preferredForwardBufferDuration = Double(forwardBufferDurationMs) / 1000.0 |
There was a problem hiding this comment.
If forwardBufferDurationMs is negative, preferredForwardBufferDuration will be set to a negative value. According to Apple's documentation, setting preferredForwardBufferDuration to a negative value raises an exception and will crash the app. We should clamp the value to be at least 0.0 to prevent crashes.
| item.preferredForwardBufferDuration = Double(forwardBufferDurationMs) / 1000.0 | |
| item.preferredForwardBufferDuration = max(0.0, Double(forwardBufferDurationMs) / 1000.0) |
| if (forwardBufferDurationMs == null) { | ||
| return null; | ||
| } | ||
| int bufferMs = (int) Math.max(1L, forwardBufferDurationMs); |
There was a problem hiding this comment.
If forwardBufferDurationMs is extremely large (e.g., greater than Integer.MAX_VALUE), casting it directly to int will cause an integer overflow, resulting in a negative value or unexpected behavior in DefaultLoadControl. We should clamp the value to Integer.MAX_VALUE before casting to ensure robustness.
| int bufferMs = (int) Math.max(1L, forwardBufferDurationMs); | |
| int bufferMs = (int) Math.min((long) Integer.MAX_VALUE, Math.max(1L, forwardBufferDurationMs)); |
| this.mixWithOthers = false, | ||
| this.allowBackgroundPlayback = false, | ||
| this.webOptions, | ||
| this.forwardBufferDuration, | ||
| }); |
There was a problem hiding this comment.
To prevent negative durations from being passed to the platform channels, we should add an assertion in the VideoPlayerOptions constructor to ensure forwardBufferDuration is non-negative.
| this.mixWithOthers = false, | |
| this.allowBackgroundPlayback = false, | |
| this.webOptions, | |
| this.forwardBufferDuration, | |
| }); | |
| this.mixWithOthers = false, | |
| this.allowBackgroundPlayback = false, | |
| this.webOptions, | |
| this.forwardBufferDuration, | |
| }) : assert(forwardBufferDuration == null || !forwardBufferDuration.isNegative, 'forwardBufferDuration must be non-negative.'); |
| const VideoCreationOptions({ | ||
| required this.dataSource, | ||
| required this.viewType, | ||
| this.forwardBufferDuration, | ||
| }); |
There was a problem hiding this comment.
Similarly, we should add an assertion in the VideoCreationOptions constructor to ensure forwardBufferDuration is non-negative.
| const VideoCreationOptions({ | |
| required this.dataSource, | |
| required this.viewType, | |
| this.forwardBufferDuration, | |
| }); | |
| const VideoCreationOptions({ | |
| required this.dataSource, | |
| required this.viewType, | |
| this.forwardBufferDuration, | |
| }) : assert(forwardBufferDuration == null || !forwardBufferDuration.isNegative, 'forwardBufferDuration must be non-negative.'); |
Description
Adds an optional
VideoPlayerOptions.forwardBufferDurationso apps can cap howfar ahead of the playhead the player buffers from the network.
Today buffering is fully automatic and aggressive — ExoPlayer's
DefaultLoadControlbuffers ~50s, and AVPlayer can prefetch most of a short file on iOS. That wastes
bandwidth whenever a user abandons a video or seeks away. This has been requested
for years without an implementation:
The option is threaded through
VideoCreationOptionsand the pigeonCreationOptionsto the engines:DefaultLoadControl(VideoPlayer.cappedLoadControl,with playback-start thresholds clamped to the cap).
AVPlayerItem.preferredForwardBufferDuration(exposed via theFVPAVPlayerItemtest abstraction).nullpreserves current behavior on every platform.Tests
VideoCreationOptions; android &avfoundation pass
forwardBufferDurationMsand default to null.cappedLoadControlnull/non-null + small-capthreshold clamping.
untouched when null.
All pass locally.
Pre-Review Checklist
dart run pigeonwith the repo's pinnedtoolchain (generated files were edited by hand to keep a minimal diff;
flagging for the CI generated-code check).