Skip to content

[video_player_platform_interface] Improve seek performance on Android#11932

Open
sailendrabathi wants to merge 3 commits into
flutter:mainfrom
sailendrabathi:video_player_platform_interface_seek
Open

[video_player_platform_interface] Improve seek performance on Android#11932
sailendrabathi wants to merge 3 commits into
flutter:mainfrom
sailendrabathi:video_player_platform_interface_seek

Conversation

@sailendrabathi

Copy link
Copy Markdown

Adds backBufferDurationMs to video_player_platform_interface

This PR has only the platform interface package changes from the main PR: #11810.

Following PR split as per Changing federated plugins

Pre-Review Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools.
  • I read the [Tree Hygiene] page, which explains my responsibilities.
  • I read and followed the [relevant style guides] and ran [the auto-formatter].
  • I signed the [CLA].
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I [linked to at least one issue that this PR fixes] in the description above.
  • I followed [the version and CHANGELOG instructions], using [semantic versioning] and the [repository CHANGELOG style], or I have commented below to indicate which documented exception this PR falls under[^1].
  • I updated/added any relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or I have commented below to indicate which [test exemption] this PR falls under[^1].
  • All existing and new tests are passing.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces backBufferDurationMs to VideoPlayerOptions to configure ExoPlayer's back buffer duration on Android, and adds videoPlayerOptions to VideoCreationOptions. Feedback suggests updating the assertion to allow a value of 0 (enabling developers to disable back buffering) and correcting the markdown spacing in the documentation comment for proper bold rendering.

@mboetger mboetger added the CICD Run CI/CD label Jun 22, 2026
@sailendrabathi sailendrabathi force-pushed the video_player_platform_interface_seek branch from c686c5b to 3360f98 Compare June 23, 2026 08:59
@github-actions github-actions Bot removed the CICD Run CI/CD label Jun 23, 2026
@mboetger mboetger added the CICD Run CI/CD label Jun 23, 2026

@mboetger mboetger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Holding on for ecosystem review

@stuartmorgan-g stuartmorgan-g added the federated: partial_changes PR that contains changes for only a single package of a federated plugin change label Jun 23, 2026
@sailendrabathi sailendrabathi force-pushed the video_player_platform_interface_seek branch from 3360f98 to 7811816 Compare June 26, 2026 18:15
@flutter-dashboard flutter-dashboard Bot removed the CICD Run CI/CD label Jun 26, 2026
@sailendrabathi

Copy link
Copy Markdown
Author

@mboetger, I updated the PR after approval from ecosystem reviewer.

Comment link for reference: #11810 (review)

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
/// Additional web controls
final VideoPlayerWebOptions? webOptions;

/// ** Android only **. Sets ExoPlayer's back buffer duration in milliseconds.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't make assertions about what platforms implement something in platform interface packgaes, because this package doesn't control that. What if a third-party implementation for another platform supports it, for instance? Or it's added to iOS later, without any change to this package?

We also don't describe behavior in terms of underlying SDK details unless there's no other way to. This comment should describe what the back buffer is, conceptually, without reference to ExoPlayer.

/// ** Android only **. Sets ExoPlayer's back buffer duration in milliseconds.
///
/// This is not possible on iOS because AVPlayer does not provide a way to
/// configure the back buffer duration.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here. You can say "Ignored on platforms that do not support controlling the back buffer.", which is both generic to other implementations, and future-proof.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to say this, but I figured once the interface is reworked to use the new standard of supportsX I would remove it myself.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but Stuart is right, that is protocol

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not just protocol; this class is directly re-exported from the app-facing package currently, so these will be client-facing docs as soon as they land.

@@ -1,3 +1,7 @@
## 6.9.0

* Adds `backBufferDurationMs` to `VideoPlayerOptions` to support configuring ExoPlayer back buffer duration on Android.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here; don't reference ExoPlayer or Android.

final VideoViewType viewType;

/// Additional configuration options for the video player.
final VideoPlayerOptions? videoPlayerOptions;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Composing the two different options classes seems potentially confusing, but it doesn't look like the API surface ever clearly distinguished these conceptually. Since VideoCreationOptions isn't exposed to app-level clients it's probably not a big deal.

@tarrinneal As part of revisiting the video_player API surface later, it would be good to better distinguish what classes are for what steps of the process (e.g., global vs player-level options, which may have been the original intent here?), and we should also fix the fact that some of these classes are being directly re-exported from the app-facing package, which we should stop doing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

federated: partial_changes PR that contains changes for only a single package of a federated plugin change p: video_player

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants