-
Notifications
You must be signed in to change notification settings - Fork 992
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
hd vids #228
Conversation
deleting unused code
…ocking or actually using real android src. Updated VideoViewModel tests.
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.
PR feedback from Nino
this.loadingIndicatorProgressBar.setVisibility(View.GONE); | ||
} | ||
} | ||
|
||
private void releasePlayer() { |
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.
Move this to the bottom with the other private methods.
@TargetApi(19) | ||
private boolean isMediaControllerAttachedToWindow() { | ||
return ApiCapabilities.canCheckMediaControllerIsAttachedToWindow() && this.mediaController.isAttachedToWindow(); | ||
private String getUserAgent() { |
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.
To not have repeat code, let's make a static helper method in WebRequestInterceptor
so that we can reuse whenever we need a user agent
// Configure the view model with a project intent. | ||
vm.intent(new Intent().putExtra(IntentKey.PROJECT, project)); | ||
|
||
preparePlayerWithUrl.assertValues(project.video().hls()); |
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.
We should use assertValue
instead and in the second test
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.
HD 🥇 !! Awesome job, @eoji !
Upgrading ExoPlayer to 2.7.0 and supporting HLS videos.
what
Supporting HD videos.
how
Updating ExoPlayer to 2.7.0 and checking for HLS videos.
Removing some custom code that we had.
see
Run this build on a device 🤓