- 
                Notifications
    You must be signed in to change notification settings 
- Fork 3.5k
[video_player] Passing http headers to file constructor #3266
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[video_player] Passing http headers to file constructor #3266
Conversation
…ng-httpHeaders-to-file-constructor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Giving this a LGTM from my review of flutter/plugins#6213 (which I actually never marked...oops). Looks like you need to bump the plugin version, though.
| @camsim99 done | 
| @tarrinneal done | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this all looks good to me, thanks for putting this together!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall, just some small comments.
        
          
                ...r/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...r/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...deo_player_android/android/src/test/java/io/flutter/plugins/videoplayer/VideoPlayerTest.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
      …ng-httpHeaders-to-file-constructor
| 
 All of the review comments should be done now. | 
| I did the changes, but not sure if i understood both correctly can you check one more time? | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
| We forgot to trigger an integration test run; looks like this broke those tests. | 
This reverts commit 73e7ef7.
| 
 I am sorry for not understanding, But should I change something, or is it not related to me ? | 
| This PR failed our postsubmit tests, so it will be reverted shortly. You'll need to submit a new PR once we do that undoes the revert but also fixes the broken tests. Did the  | 
| Now that I'm at my work machine: the error is: It looks like something is violating the  | 
| In particular, | 
…or (flutter#3266)" (flutter#3424)" This reverts commit 88ca9e2.
| #3425 passed tests, so it looks like that's all it was. @zezo357 No need to submit another PR. | 
| 
 Thank you very much. | 
[video_player] Passing http headers to file constructor
…ter#3266)" (flutter#3424) This reverts commit 73e7ef7.
Recreating PR from flutter/plugins flutter/plugins#6213
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
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.