Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[video_player] Implement caching - #2130

Closed
999eagle wants to merge 13 commits into
flutter-team-archive:masterfrom
999eagle:feature/caching
Closed

[video_player] Implement caching#2130
999eagle wants to merge 13 commits into
flutter-team-archive:masterfrom
999eagle:feature/caching

Conversation

@999eagle

@999eagle 999eagle commented Oct 1, 2019

Copy link
Copy Markdown

Description

This PR adds a way to cache videos played from an Uri. Two new parameters (maxCacheSize and maxFileSize) have been added to the platform method call and the Dart side has been changed to pass these parameters as well.

Related Issues

Resolves flutter/flutter#28094.

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (flutter analyze) does not report any problems on my PR.
  • I read and followed the Flutter Style Guide.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I signed the CLA.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
  • No, this is not a breaking change.

@googlebot

Copy link
Copy Markdown

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@999eagle

Copy link
Copy Markdown
Author

@lukepighetti can you sign the google cla please to be able to merge this?

@lukepighetti

Copy link
Copy Markdown

@googlebot I consent.

@googlebot

Copy link
Copy Markdown

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@lukepighetti

Copy link
Copy Markdown

@999eagle did you end up getting eyes on the uri stuff on iOS to make sure it was setup appropriately? I kept having issues with certain URLs but I never was able to determine if that was a local issue or not.

@otopba

otopba commented Nov 24, 2019

Copy link
Copy Markdown
Contributor

@999eagle Thank you for your solution. Could you update this branch due master updates please?

@cbenhagen

Copy link
Copy Markdown
Contributor

@999eagle thank you for your contribution! Unfortunately the structure of the plugin has changed lately to support implementations on more platforms. Mainly the plugin was moved to a new subdirectory and now uses a common platform interface. Adapting your PR to the new structure should be straight forward. If you have questions please ask!

Please also make sure to include tests for the new functionality.

@sanekyy

sanekyy commented Dec 17, 2019

Copy link
Copy Markdown
Contributor

Hello @999eagle @cbenhagen @lukepighetti, @iskakaushik @cyanglaz.
I and @otopba created new PR which fix this PR to speed up the process of merging of this feature.

@sanekyy

sanekyy commented Feb 11, 2020

Copy link
Copy Markdown
Contributor

@999eagle Hello.

Can you confirm CLA in new PR?

@jimmywongz

Copy link
Copy Markdown

Hi Would like to ask when would this be merged? really want to have this feature

@lukepighetti

Copy link
Copy Markdown

What's blocking this from being merged?

@lukepighetti

Copy link
Copy Markdown

It appears this PR should be closed for being stale, and the tracking PR for this feature is located here: #2429 (comment)

@ditman

ditman commented Jan 15, 2021

Copy link
Copy Markdown
Contributor

Closing in favor of #2429

@ditman ditman closed this Jan 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[video_player] Add network cache functionality

10 participants