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

[video_player] add support for content-uri based videos (android only) #4261

Closed
wants to merge 2 commits into from

Conversation

byunme
Copy link
Contributor

@byunme byunme commented Aug 24, 2021

Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.

This PR adds playback from contentUri support for the video_player plugin (Android only).

The current Android implementation uses ExoPlayer, which already supports playback from uri, so this change is just to surface contentUri as a valid video datasource on the dart side.

List which issues are fixed by this PR. You must list at least one issue.
flutter/flutter#88130

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Note that unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • 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 updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
  • All existing and new tests are passing.

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

@@ -1,5 +1,6 @@
## NEXT
## 2.1.15
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: a new feature should bump the minor version rather than the patch.

@stuartmorgan stuartmorgan requested a review from blasten August 26, 2021 20:24
@stuartmorgan
Copy link
Contributor

Thanks for the submission!

@blasten This looks pretty straightforward; can you take a look?

Comment on lines +36 to +41

# FOR TESTING ONLY. DO NOT MERGE.
dependency_overrides:
video_player_platform_interface:
path:
../../video_player_platform_interface
Copy link

Choose a reason for hiding this comment

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

don't forget to delete

Copy link
Contributor

Choose a reason for hiding this comment

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

It'll be removed once the PR is split (https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changing-federated-plugins). It can't accidentally be left because this fails publishable.

@stuartmorgan
Copy link
Contributor

@byunme Are you planning on splitting this into the component PRs now so that we can move forward with landing this?

@byunme
Copy link
Contributor Author

byunme commented Sep 3, 2021

@stuartmorgan Sorry for the delay, I've split off the interface part here: #4307

@jmagman
Copy link
Member

jmagman commented Sep 9, 2021

This needs to be merged ASAP to unblock the framework tree. #4307

I don't have permissions to push to this branch, taking it to #4330

@jmagman
Copy link
Member

jmagman commented Sep 9, 2021

Obsolesced by #4330.

Thanks for the contribution, @byunme, sorry I had to split the PR out to reopen the submit queues ASAP.

@jmagman jmagman closed this Sep 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants