[video_player] Passing http headers to file constructor

[abdelaziz-mahdy]

when passing m3u8 file to videoplayer as videoplayercontroller.file links insided m3u8 file has to be used with http headers

so i passed it in the videoplayercontroller.file and changed all things related to it

issue flutter/flutter#119467

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/plugins repo does use dart format.)
• [ x] I signed the CLA.
• [ x] The title of the PR starts with the name of the plugin 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.

[flutter-dashboard]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

[camsim99]

@zezo357 I am ready to add tests but I have some follow up questions first to make sure I understand the change.

1. Why did you delete the check for isHttp(...) and use new DefaultDataSource.Factory(context, httpDataSourceFactory) by default versus httpDataSourceFactory.setDefaultRequestProperties(httpHeaders)? What happens if the uri is not HTTP?
2. As a follow up to (1), was the fix for passing mu38 files to add the .setUserAgent(httpHeaders.get(USER_AGENT)) line if USER_AGENT is present the HTTP headers?

[abdelaziz-mahdy]

1-If not http all functions will work and it's the main fix since when playing a m3u8 file it's not http

2- user agent should be added to the http headers If the user wants it (it's needed because some hosts do not allow the default user agent)

[stuartmorgan-g]

Triage checkin: what's the state of this PR? Is it just waiting for tests?

[abdelaziz-mahdy]

Triage checkin: what's the state of this PR? Is it just waiting for tests?

Waiting on the java native tests since I couldn't implement those, but the dart tests I already implemented and they are passing

[camsim99]

@zezo357 Sorry for the delay on this. Just added tests and made some minor changes. Let me know what you think!

[abdelaziz-mahdy]

Will check it thoroughly tomorrow and test it

But for now I see it looks good

[abdelaziz-mahdy]

looks good to me.

[camsim99]

@zezo357 Can you fix the failing Dart test? Then we'll need a second review.

[abdelaziz-mahdy]

@zezo357 Can you fix the failing Dart test? Then we'll need a second review.

will check them

[abdelaziz-mahdy]

I fixed the dart test related to my pull request there is another one that fails
file with special characters
with error

Invalid argument(s): Illegal character in path
dart:core                                      new _Uri.file
package:video_player/video_player.dart 254:26  new VideoPlayerController.file
test\video_player_test.dart 352:35             main.<fn>.<fn>.<fn>

[camsim99]

@zezo357 I don't see that failure, but there is a formatting error, but you can run the command suggested to fix it https://github.com/flutter/plugins/pull/6213/checks?check_run_id=10935748930

[abdelaziz-mahdy]

@zezo357 I don't see that failure, but there is a formatting error, but you can run the command suggested to fix it https://github.com/flutter/plugins/pull/6213/checks?check_run_id=10935748930

fixed formatting

[stuartmorgan-g]

• I listed at least one issue that this PR fixes in the description above.

Please add the issue link to the PR description.

Is this problem actually Android-specific, or does it apply to iOS as well? It's fine to fix it only for Android for now if you don't have iOS experience, but if it's broken there as well we should update the relevant issue accordingly.

[abdelaziz-mahdy]

• I listed at least one issue that this PR fixes in the description above.

Please add the issue link to the PR description.

Is this problem actually Android-specific, or does it apply to iOS as well? It's fine to fix it only for Android for now if you don't have iOS experience, but if it's broken there as well we should update the relevant issue accordingly.

i didnt test on ios, but the issue was for me on android. and i did describe the issue on the pull request should i open an issue?

[stuartmorgan-g]

Yes, that checklist item is about having an issue filed clearly describing the problem, including reproduction steps, so there's a place to reference details about the issue being fixed.

[abdelaziz-mahdy]

Yes, that checklist item is about having an issue filed clearly describing the problem, including reproduction steps, so there's a place to reference details about the issue being fixed.

ok i will open one and try to make the steps to reproduce it.

[abdelaziz-mahdy]

i added the issue in the description with example repo due to using packages to retrieve new urls

since the urls lifetime is short

[stuartmorgan-g]

We've just completed the migration of the plugin code to the flutter/packages repository, as described in https://flutter.dev/go/flutter-plugins-repo-migration, and this repository is now being archived. Unfortunately that means that all in-progress PRs here must be moved to flutter/packages.

Please see our instructions for an explanation of how to move your PR, and if you have any issues moving your PR please don't hesitate to reach out in the #hackers-ecosystem channel in Discord.

Our apologies that your PR was caught in this one-time transition. We're aware that it's disruptive in the short term, and appreciate your help in getting us to a better long-term state!