[video_player] Add video caching to video player

[TimHoogstrate]

This resolves feature issue: https://github.com/flutter/flutter/issues/28094

Added caching functionality to video_player, video_player_android, video_player_avfoundation. Changed the platform interface.

Pre-launch Checklist

• [x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
• [x] I read the Tree Hygiene wiki page, which explains my responsibilities.
• [x] I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
• [x] I signed the CLA.
• [x] The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
• [x] I listed at least one issue that this PR fixes in the description above.
• [x] I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
• [x] I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
• [x] I updated/added relevant documentation (doc comments with ///).
• [x] I added new tests to check the change I am making, or this PR is test-exempt.
• [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[ditman]

The CLA robot is happy, but there's still some CI checks that are broken on this branch and that will need to be taken care of before we can move forward (feel free to ask in this PR if any of the checks have cryptic errors, it happens some times!)

For example, the analyze checks have the following to say:

Analyzing video_player_android...

   info - test/android_video_player_test.dart:169:19 - Type could be non-nullable. Try changing the type to be non-nullable. - unnecessary_nullable_for_final_variable_declarations

Analyzing video_player_avfoundation...

   info - test/avfoundation_video_player_test.dart:142:19 - Type could be non-nullable. Try changing the type to be non-nullable. - unnecessary_nullable_for_final_variable_declarations
   info - test/avfoundation_video_player_test.dart:227:22 - The value of the argument is redundant because it matches the default value. Try removing the argument. - avoid_redundant_argument_values
   info - test/avfoundation_video_player_test.dart:245:22 - The value of the argument is redundant because it matches the default value. Try removing the argument. - avoid_redundant_argument_values
   info - test/avfoundation_video_player_test.dart:263:22 - The value of the argument is redundant because it matches the default value. Try removing the argument. - avoid_redundant_argument_values

The full CI information is here, which links to the "stdout" logs of the build process.

[ditman]

For the remaining failing check, take a look at this:

• https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changing-federated-plugins (Especially the bit about make-deps-path-based).

(Also, please remember to undo the changes in video_player_web, if this is only updating metadata, it isn't needed, and it'll make your PR much simpler to manage!)

[ditman]

Unsubbing, please remember to revert the changes to video_player_web (or adding the required (noop?) behavior there)

[hellohuanlin]

unsubscribing too - please request my review again when it's ready.

[TimHoogstrate]

I'm not yet finished with the all the changes but I've pushed some already.

[hellohuanlin]

@TimHoogstrate Hi, let me know if you have addressed my comments (and my comments on the previously closed PR). I can take another look.

[kishangohel]

@TimHoogstrate @stuartmorgan @tarrinneal When can we expect this feature as this really needed feature and most developers switched to official player

[TimHoogstrate]

Dear @stuartmorgan,

I had some issues while merging macOS implementations into my branch. I fixed them and then, while testing, I noticed an issue, that a black video frame was shown in the player en I was running the example app op a real iPhone after clearing the cache. The simulator did not have that issue. I found out that I accidentally removed some code while documenting. Anyway, I fixed it and I had to make some changes for the macOS implementation. For now I omitted the caching feature for macOS. Do you want to have it implemented now or later? I am working on make it buildable again. It builds and tests locally for both MacOS and iOS. So I kicked off a new build and I'll see how I get this buildable on the build server later.

Dear @hellohuanlin,

I added the android tests and when everything is building you can re review the changes.

Kind regards,

[stuartmorgan-g]

For now I omitted the caching feature for macOS. Do you want to have it implemented now or later?

What's different between macOS and iOS for the cache? I would have thought it would be entirely shared code.

[cbenhagen]

What's different between macOS and iOS for the cache? I would have thought it would be entirely shared code.

@TimHoogstrate did you see this question? I would love to see macOS support from the beginning.

[TimHoogstrate]

@stuartmorgan, I was a bit lost regarding the cache and macOS implementation. The macOS Implementation was not working because it appears that the MobileCoreServices, (that I use to determine the mime and content-type) were not building (available) for MacOS. I migrated the MobileCoreServices to CoreServices (appearing that this was working for macOS). HOWEVER, I was testing the macOS cache feature (thinking that it would work out of the box), and I ran into some runtime errors. Therefore, in my first reaction was to omit the macOS feature for now (to get the tests and builds running). And then we could add those later as a new feature of version. HOWEVER, the tests were failing because I omitted macOS caching (isCacheAvailable if Mac then return no else yes). It seemed that I was not able to determine the platform or OS as expected... Then I noticed that in Xcode I run a build on "macOS (designed for iPAD)" I dug into this and then i found out that running a build or test from command line like e.g.: dart run script/tool/bin/flutter_plugin_tools.dart native-test --macos --packages video_player has a different result than running the same tests in Xcode. I don't fully understand why. But now that the builds are building again and the tests are success I can dive into the macOS feature, and why it gave me runtime errors.

[stuartmorgan-g]

Then I noticed that in Xcode I run a build on "macOS (designed for iPAD)"

That's Catalyst; it's not something we support in Flutter.

[TimHoogstrate]

@stuartmorgan, macOS is not accepting an url and throws an url not supported exception on macOS. Same is working on iOS. So I am not sure how to proceed. It seems that macOS needs a bit additional work. iOS is working fine.

Kind regards,

[stuartmorgan-g]

macOS is not accepting an url and throws an url not supported exception on macOS

Without more details and context I'm not sure what that would be. It's fine to disable caching on macOS with a TODO for now though, and we can investigate once mobile is working.

[kishangohel]

@TimHoogstrate On iOS player throws an internet error although the entire video is cached when the internet is turned off or the internet is not available

[hellohuanlin]

I am checking if this is ready for a second review on iOS part? Feel free to ping me whenever it's ready.

[TimHoogstrate]

I was able reproduce the internet error. The thing is that I don't know how to resolve it fast because I don't know what throws the error on iOS. It has been working fine but looks like the URLAsset is throwing an error on iOS (in case there is no internet) or on macOS (in case I use caching, because I use a caching path as an url). I'll try to find a solution in the next week. Then I'll fix the merge conflicts too and check if everything is working as expected.

[delfme]

It seems this might resolve to a dead end. If that is the case, would it be doable for flutter team to take over or implement from scratch? better_player has a cache feature for both ios and android under MIT. It is a few hundred lines of code for each platform. https://github.com/jhomlala/betterplayer/blob/master/ios/Classes/CacheManager.swift

@stuartmorgan we noticed that the hard work on flutter core and impeller migration has been taking the most of the time and core plugins like camera and videoplayer are a bit left behind. Even issues have been triaged, yet not worked. Do you have any plan to get a few devs helping on them on a daily basis? Thx

[stuartmorgan-g]

would it be doable for flutter team to take over or implement from scratch?

The plan is already for the Flutter team to continue this PR once we have time.

As for the rest of the comment, an individual PR is not the place for meta-discussion about overall team resourcing. A general discussion forum such as the Discord server would be the place for that.

[vashworth]

Converting to draft since it's not actively being worked on and not ready for review.

[stuartmorgan-g]

Update from triage: This is still on our radar, but other work has taken priority so far.

[stuartmorgan-g]

Closing for now so this doesn't show up in triage; we don't expect to have time to work on it in the next several months, and interop work using video_player would require reworking this further.