Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Improve Android TV home screen recommendations #2515

Merged
merged 19 commits into from
Apr 4, 2023
Merged

Improve Android TV home screen recommendations #2515

merged 19 commits into from
Apr 4, 2023

Conversation

OctoNezd
Copy link
Contributor

@OctoNezd OctoNezd commented Feb 20, 2023 •
edited
Loading

Changes

PR introduces 3 new home screen channels:

  • Latest media - new default channel
  • Movies
  • New episodes
    image

Issues

#2513

@OctoNezd
Copy link
Contributor Author

Also there probably should be option to make channel empty, considering that it cant be disabled at all on GoogleTV - I will implement it

github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 20, 2023
View reviewed changes
@nielsvanvelzen nielsvanvelzen added the enhancement New feature or request label Feb 21, 2023
@nielsvanvelzen nielsvanvelzen self-requested a review February 23, 2023 19:01
nielsvanvelzen
nielsvanvelzen requested changes Feb 26, 2023
View reviewed changes
Copy link
Member

@nielsvanvelzen nielsvanvelzen left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request. There are two major issues that need to be changed before I can start a code review:

  • Remove the "Display backdrop" option
    The backdrops are not meant to be used as thumbnails and they are not always in 16:9 aspect ratio. The Android TV launcher warps the image to fit the 16/9 ratio when the backdrop is a different size which looks awful.

  • Remove "leanbackPreferredDefaultChannel" option
    There can be only one default channel and it's set when the app first starts so changing it is just not possible. Your approach to change the reported values based on the preference is a hack that I don't want to support. This is a bit unfortunate for Google TV users but there's nothing we can do about it.

@jellyfin-bot jellyfin-bot added the merge conflict Conflicts prevent merging label Feb 26, 2023
@OctoNezd
Copy link
Contributor Author

OctoNezd commented Feb 26, 2023 •
edited
Loading

  1. Maybe change it to "Prefer thumbnails" instead then?

  2. Maybe change the default channel to something more useful than just media collections, e.g. just latest media, and option to just plain disable the home screen channels?
    Or, as another workaround, in case of google tv generate only one channel?

@OctoNezd
Remove leanbackPreferredDefaultChannel option
536877c
@OctoNezd
Change default channel to latest_media
873f371
@OctoNezd
fixup - set default channel to latest media
8032581 
The first channel your app creates becomes its default channel. The default channel automatically appears in the home screen. All other channels you create must be selected and accepted by the user before they can appear in the home screen.
@OctoNezd
Copy link
Contributor Author

  • The backdrops are not meant to be used as thumbnails and they are not always in 16:9 aspect ratio. The Android TV launcher warps the image to fit the 16/9 ratio when the backdrop is a different size which looks awful.

We can requested a cropped backdrop instead through fillHeight and fillWidth?

In addition to:

The backdrops are not meant to be used as thumbnails

Jellyfin web ui calls backdrops "thumbs"
image

@nielsvanvelzen
Copy link
Member

We can requested a cropped backdrop instead through fillHeight and fillWidth?

We try to use the least amount of parameters possible so we use our local image cache, so not a huge fan of this approach.

@OctoNezd
Copy link
Contributor Author

OctoNezd commented Mar 3, 2023 •
edited
Loading

The lack of prefer backdrops option results in ugly letterboxing on gtv devices, and having it results in ugly stretching on atv then. This is a bit problematic situation, both outcomes end up in situation where one part of userbase gets screwed :/

Will remove the feature tomorrow then I guess

@nielsvanvelzen
Copy link
Member

I tried setting up an Google TV emulator so I can test which features are supported to find a good solution but I have not been able to make it work. Even with the stable app installed via Play Store it won't show any of our channels or next up items.

@OctoNezd
Copy link
Contributor Author

OctoNezd commented Mar 4, 2023 •
edited
Loading

Weird - the channel shows up for me on google tv emulator created with Android Studio. No next up/continue watching though. Did you add Google Account? Check if you are running apps-only mode under your google account settings. Also, apparently GTV uses channels/recommendations only in specific regions, where "the full google tv experience" is not available (cant find source for this statement, it was in some atv documentation by google).

@OctoNezd
Copy link
Contributor Author

OctoNezd commented Mar 4, 2023

image
Here is a screenshot of gtv recommendations when using posters

@nielsvanvelzen
Copy link
Member

Also, apparently GTV uses channels/recommendations only in specific regions, where "the full google tv experience" is not available (cant find source for this statement, it was in some atv documentation by google).

I think this might be the issue then because I'm getting the full advertisement experience (Netherlands).

@OctoNezd
Copy link
Contributor Author

OctoNezd commented Mar 4, 2023

Also, apparently GTV uses channels/recommendations only in specific regions, where "the full google tv experience" is not available (cant find source for this statement, it was in some atv documentation by google).

I think this might be the issue then because I'm getting the full advertisement experience (Netherlands).

Yep - https://www.youtube.com/watch?v=O3BhUSEu8gg

8:51
You may have noticed home screen channels weren’t on the earlier Google TV screenshot.
8:56
For Google TV, home screen channels are only shown in markets
9:00
where we don't have enough information about content
9:02
to power the full Google TV experience.
9:05
That means they're not going to show up in markets like the US.

@nielsvanvelzen
Copy link
Member

Ok so the channels are technically not supported on Google TV. I don't think we should add any specific behavior for that platform in that case because we can't maintain it when Google slowly moves countries over to their advertisement-experience.

@OctoNezd
Copy link
Contributor Author

So - both options need to go then, I assume?

@nielsvanvelzen
Copy link
Member

Yup

@jellyfin-bot jellyfin-bot removed the merge conflict Conflicts prevent merging label Mar 22, 2023
@OctoNezd OctoNezd requested a review from nielsvanvelzen March 22, 2023 08:41
@OctoNezd
Copy link
Contributor Author

All new options were removed

github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 24, 2023
View reviewed changes
app/src/main/java/org/jellyfin/androidtv/integration/LeanbackChannelWorker.kt
.toContentValues()
}.let { context.contentResolver.bulkInsert(TvContractCompat.PreviewPrograms.CONTENT_URI, it.toTypedArray()) }
@SuppressLint("RestrictedApi")
private fun createPreviewProgram(

Check warning

Code scanning / detekt

Prefer splitting up complex methods into smaller, easier to test methods.

The function createPreviewProgram appears to be too complex based on Cyclomatic Complexity (complexity: 20). Defined complexity threshold for methods is set to '15'
@nielsvanvelzen nielsvanvelzen added this to the v0.16.0 milestone Apr 4, 2023
nielsvanvelzen
nielsvanvelzen approved these changes Apr 4, 2023
View reviewed changes
Copy link
Member

@nielsvanvelzen nielsvanvelzen left a comment

Choose a reason for hiding this comment

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

Thanks! Everything is good now. I've made some final small tweaks that you can see in the last commit.

@nielsvanvelzen nielsvanvelzen enabled auto-merge (squash) April 4, 2023 11:33
@nielsvanvelzen nielsvanvelzen merged commit ef5f800 into jellyfin:master Apr 4, 2023
@nielsvanvelzen nielsvanvelzen mentioned this pull request May 11, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nielsvanvelzen nielsvanvelzen nielsvanvelzen approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
v0.16.0
Development

Successfully merging this pull request may close these issues.
3 participants
@OctoNezd @nielsvanvelzen @jellyfin-bot