-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Android TV support #2806
Android TV support #2806
Conversation
@Alexander-- Thanks for the updated PR.
In this case, I can only review the code and point out what's going wrong in the emulator, but we need some more people who can test this on real devices.
Okay. My standard test workflow is to test with Android 9, 7 and 4.4 to catch all edge cases. However, this PR should not make NewPipe unusable :) |
I agree, that not having real Android 9 devices is unfortunate. Amazon released Fire TV Cube few days ago (it is their first TV device with Android 9 I think), but I don't have one yet.
That sounds problematic. Let's be honest — folks, who use TVs in 2019, aren't exactly a type to build NewPipe from source. Until these changes are accepted there will be no ready-to-use apk and thus close to zero potential testers. |
I might be able to test it on my Android 9 and androidTV, i have no idea how to put applications on androidTV though. Is it through some kind of ADB? |
I tried running this branch on my FireTV Stick and basic functionalities like navigation, playing video, search, focus on search suggestions, player controls like play/pause/seek etc are working fine. Fire TV Home Version Fire OS Some issues noticed
android.content.ActivityNotFoundException: No Activity found to handle Intent { act=android.settings.CAPTIONING_SETTINGS }
E/ACRA (14796): ACRA caught a ActivityNotFoundException for org.schabi.newpipe [34/19256]
E/ACRA (14796): android.content.ActivityNotFoundException: No Activity found to handle Intent { act=android.settings.CAPTIONING_SETTINGS }
E/ACRA (14796): at android.app.Instrumentation.checkStartActivityResult(Instrumentation.java:1797)
E/ACRA (14796): at android.app.Instrumentation.execStartActivity(Instrumentation.java:1517)
E/ACRA (14796): at android.app.Activity.startActivityForResult(Activity.java:3761)
E/ACRA (14796): at androidx.fragment.app.FragmentActivity.startActivityForResult(FragmentActivity.java:676)
E/ACRA (14796): at androidx.core.app.ActivityCompat.startActivityForResult(ActivityCompat.java:234)
E/ACRA (14796): at androidx.fragment.app.FragmentActivity.startActivityFromFragment(FragmentActivity.java:791)
E/ACRA (14796): at androidx.fragment.app.FragmentActivity$HostCallbacks.onStartActivityFromFragment(FragmentActivity.java:933)
E/ACRA (14796): at androidx.fragment.app.Fragment.startActivity(Fragment.java:1185)
E/ACRA (14796): at androidx.fragment.app.Fragment.startActivity(Fragment.java:1173)
E/ACRA (14796): at org.schabi.newpipe.settings.AppearanceSettingsFragment.onPreferenceTreeClick(AppearanceSettingsFragment.java:45)
E/ACRA (14796): at androidx.preference.Preference.performClick(Preference.java:1173)
E/ACRA (14796): at androidx.preference.Preference.performClick(Preference.java:1148)
E/ACRA (14796): at androidx.preference.Preference$1.onClick(Preference.java:172)
E/ACRA (14796): at android.view.View.performClick(View.java:5047)
E/ACRA (14796): at android.view.View.onKeyUp(View.java:9301)
E/ACRA (14796): at android.view.KeyEvent.dispatch(KeyEvent.java:2687)
E/ACRA (14796): at android.view.View.dispatchKeyEvent(View.java:8683)
E/ACRA (14796): at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1503)
E/ACRA (14796): at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1508)
E/ACRA (14796): at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1508)
E/ACRA (14796): at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1508)
E/ACRA (14796): at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1508)
E/ACRA (14796): at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1508)
E/ACRA (14796): at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1508)
E/ACRA (14796): at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1508)
E/ACRA (14796): at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1508)
E/ACRA (14796): at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1508)
E/ACRA (14796): at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1508)
E/ACRA (14796): at com.android.internal.policy.impl.PhoneWindow$DecorView.superDispatchKeyEvent(PhoneWindow.java:2405)
E/ACRA (14796): at com.android.internal.policy.impl.PhoneWindow.superDispatchKeyEvent(PhoneWindow.java:1738)
E/ACRA (14796): at androidx.core.view.KeyEventDispatcher.activitySuperDispatchKeyEventPre28(KeyEventDispatcher.java:130)
E/ACRA (14796): at androidx.core.view.KeyEventDispatcher.dispatchKeyEvent(KeyEventDispatcher.java:87)
E/ACRA (14796): at androidx.core.app.ComponentActivity.dispatchKeyEvent(ComponentActivity.java:133)
E/ACRA (14796): at androidx.appcompat.app.AppCompatActivity.dispatchKeyEvent(AppCompatActivity.java:558)
E/ACRA (14796): at androidx.appcompat.view.WindowCallbackWrapper.dispatchKeyEvent(WindowCallbackWrapper.java:59)
E/ACRA (14796): at androidx.appcompat.app.AppCompatDelegateImpl$AppCompatWindowCallback.dispatchKeyEvent(AppCompatDelegateImpl.java:2814)
E/ACRA (14796): at androidx.appcompat.view.WindowCallbackWrapper.dispatchKeyEvent(WindowCallbackWrapper.java:59)
E/ACRA (14796): at com.android.internal.policy.impl.PhoneWindow$DecorView.dispatchKeyEvent(PhoneWindow.java:2320)
E/ACRA (14796): at android.view.ViewRootImpl$ViewPostImeInputStage.processKeyEvent(ViewRootImpl.java:4088)
E/ACRA (14796): at android.view.ViewRootImpl$ViewPostImeInputStage.onProcess(ViewRootImpl.java:4038)
E/ACRA (14796): at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:3600)
E/ACRA (14796): at android.view.ViewRootImpl$InputStage.onDeliverToNext(ViewRootImpl.java:3653)
E/ACRA (14796): at android.view.ViewRootImpl$InputStage.forward(ViewRootImpl.java:3619)
E/ACRA (14796): at android.view.ViewRootImpl$AsyncInputStage.forward(ViewRootImpl.java:3736)
E/ACRA (14796): at android.view.ViewRootImpl$InputStage.apply(ViewRootImpl.java:3627)
E/ACRA (14796): at android.view.ViewRootImpl$AsyncInputStage.apply(ViewRootImpl.java:3793)
E/ACRA (14796): at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:3600)
E/ACRA (14796): at android.view.ViewRootImpl$InputStage.onDeliverToNext(ViewRootImpl.java:3653)
E/ACRA (14796): at android.view.ViewRootImpl$InputStage.forward(ViewRootImpl.java:3619)
E/ACRA (14796): at android.view.ViewRootImpl$InputStage.app
|
wtf... those settings are actually not focusable?! |
AppBarLayout mostly gets it, but we still need to uphold our own part - expanding it back after focus returns to it
We don't hide MainFragment when search is show, so FocusFinder sometimes gives focus to (obscured) main content
GridLayoutManager is buggy - https://issuetracker.google.com/issues/37067220: it randomly loses or incorrectly assigns focus when being scrolled via direction-based navigation. This commit reimplements onFocusSearchFailed() on top of scrollBy() to work around that problem. Ordinary touch-based navigation should not be affected.
Focus isn't needed for marquee, only selection
The same logic is present in RecyclerView, ScrollView etc. Android really should default to this behavior for all Views with isScrollContainer = true
* Hide player controls when back is pressed (only on TV devices) * Do not hide control after click unless in touch mode * Show player controls on dpad usage * Notably increase control hide timeout when not in touch mode
Buttons are more likely to have "correct" styling and are focusable/clickable out of box
Upstream DrawerLayout does override addFocusables, but incorrectly checks for isDrawerOpen instread of isDrawerVisible
It is still buggy because of NavigationView (why the hell is NavigationMenuView marked as focusable?) but at least initial opening works as intended
FocusFinder has glitches when some of target Views have different size. Fortunately LayoutManager can redefine focus search strategy to override the default behavior.
* Move all focus-related work arouns to NewPipeRecyclerView * Try to pass focus within closer parents first * Do small arrow scroll if there are not more focusables in move direction
Video descriptions can be very long. Some of them are basically walls of text with couple of lines at top or bottom. They are also not scrolled within TextView itself, - instead NewPipe expects user to scroll their containing ViewGroup. This renders all builtin MovementMethod implementations useless. This commit adds a new MovementMethod, that uses requestRectangleOnScreen to intelligently re-position the TextView within it's scrollable container.
Prevents comment list from losing focus to some outside View when user tries to scroll down after reaching "end"
While testing in a Android 7 emulator, I found two small things. Not sure, how reliable that emulator is. So you should test this on a real devices before trying to fix something which already works properly :) |
I think so too. Unfortunately, ripples are part of the background drawables — you can't simply toggle some global flag and have them gone. |
You could call that method at the end of private void applyTVChanges() {
if (!AndroidTvUtils.isTv()) {
return;
}
// remove ripple effects
final int transparent = getResources().getColor(R.color.transparent_background_color);
detailControlsAddToPlaylist.setBackgroundColor(transparent);
detailControlsBackground.setBackgroundColor(transparent);
detailControlsPopup.setBackgroundColor(transparent);
detailControlsDownload.setBackgroundColor(transparent);
// do some other TV specific initialization stuff here
} |
Jap, I can approve this optical glitch in my Android-PIE AndroidTV stock rom: (nvidia shield os 8.0.2) I love the to see the progress here. Think this will become awesome! I am using the app every second day and it worked wonderful. I will retest all my mentions some post ago here in separate issues when a final release is there. In my opinion the debug app here is stable enough to be released. I think the most of the issues here are not necessary to run the app.
Can you check this icon launcher "hack" on your firetv device: |
I received two requests via email for a new debug APK. Here it is: NewPipe_android_tv-debug.apk.zip. @Alexander-- This PR does not need to be bring perfect Android TV compatibility. I almost finished code review and I think this should be good to go soon. Do you mind, if I help fixing some remaining bugs or glitches and push to your branch? |
Sure, go ahead. |
@Alexander-- Thanks. Please allow us to push to your branch. |
I don't see that checkbox (seems like a it does not work for many people) I suggest, that you simply push these commits to your own branch and work from there. |
@Alexander--: That's because your PR was made from the @chdir organization, not your own profile. @TobiGr: Could you create some new branch in TeamNewPipe and push there (so I could also push there)? Edit: I created the branch here. |
@wb9688 Thank you. @Alexander-- I've invited you to collaborate in the NewPipe repository. After accepting the invitation, I'll give you access to the |
Here's the (most likely) last APK for this PR. NewPipe_androidtv-debug.apk.zip @Alexander-- I turned out that I need to give you complete write access to the repo (inlcuding issue & pr management) to enable you to push to one branch. That's why I decided to not do this. Did you prepare any further commits which you'd like to share with us? If yes, you could pull from out |
@Alexander-- Thanks again for your patience and the effort you put into this (and the first) PR! Your improvements will be included in 0.19.3. |
@Override | ||
public void safeHideControls(final long duration, final long delay) { | ||
if (DEBUG) { | ||
Log.d(TAG, "safeHideControls() called with: delay = [" + delay + "]"); | ||
} | ||
|
||
View controlsRoot = getControlsRoot(); | ||
if (controlsRoot.isInTouchMode()) { | ||
getControlsVisibilityHandler().removeCallbacksAndMessages(null); | ||
getControlsVisibilityHandler().postDelayed(() -> | ||
animateView(controlsRoot, false, duration, 0, | ||
MainVideoPlayer.this::hideSystemUi), delay); | ||
} | ||
} | ||
|
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.
I have a question: why was this added? It is causing trouble with controls not being hidden after pausing and resuming (See #3403)
@TobiGr @Alexander--
Question: when Newpipe 19.4 comes out, will we be easily able to update Newpipe via the Android TV or Fire TV remote control or do we need to request such a feature? I was thinking it would be great if Newpipe updated on my FireTV the same way as https://smartyoutubetv.github.io currently does. When it is opened, it displays the changelog and the choice to upgrade or not. Upgrading downloads the file and you can just press the remote buttons to install. |
No. You'll have to use your previous provider, then F-Droid which isn't great for TV, GitHub which can be terrible depending on your browser (I recommend you Firefox TV, the navigation is great), or another provider (not recommended at all). |
In that case, here's my FR. #3630 |
Wait, I can get F-Droid on my TV? And NewPipe? |
Yes, and yes. You can download APK from your browser, then install it. Then you can download F-Droid from F-Droid website and download NewPipe from F-Droid |
Hi guys, it is me again.
This PR improves upon the previous one in many areas.
Long video descriptions were made properly scrollable (and actually convenient to read!) on TV devices.
NewPipeRecyclerView works much better. The new version is so good, it can actually be used as drop-in replacement for RecyclerView by itself — the separate patched LayoutManager is no longer necessary!
I have identified a bunch of bugs in FocusFinder on various Android versions. Those bugs are no big deal, but they make focus transitions in lists unpredictable if list items have different size (to be precise, when focusable areas have different size). I have implemented a fix in SuperScrollLayoutManager. This is not the same as previous situation, when LayoutManager was necessary to avoid focus loss during scroll — you can now use NewPipeRecuclerView without SuperScrollLayoutManager, having later just improves experience further.
@TobiGr I have tested on Android 9 emulator and found out, why it was hanging for you — it was trying to rotate into portrait mode and apparently failing to do so.
In my experience, real Android TV devices have patched framework classes, that make switching to portrait mode utterly impossible (both automatically and manually). Android emulator does not contain those fixes, so when running on it the player still tried to enter portrait mode... This is not really problem with NewPipe or my changes, — just emulator doing poor job at emulating Android TV boxes. But I still added a change to use portrait mode for player by default when Android TV device is detected.
In general I don't advise to test this on Android 9 — it has several mis-features, that make NewPipe a pain to use. I will send fixes for those later. For now please test on Android 7 and older versions (most TV boxes don't use Android 9 anyway).
supplants #2701
fixes #1363 fixes #899 fixes #717 fixes #3428