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.

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

Fix watch next row ordering, Change watch next row meta data #4298

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

hdweiss
Copy link
Contributor

@hdweiss hdweiss commented Dec 14, 2024

Changes
Various fixes to the "play next"/"watch next" TV row on the homescreen.

  • episodes now have meta data in line with other streaming apps
  • watch next is now correctly ordered
  • movies to resume will now be inserted correctly

Issues

- episodes now have meta data in line with other streaming apps
- watch next is now correctly ordered
- movies to resume will now be inserted correctly
@nielsvanvelzen nielsvanvelzen self-requested a review December 19, 2024 20:51
@nielsvanvelzen
Copy link
Member

watch next is now correctly ordered

What is wrong with the order? Is there an open issue for that?

@hdweiss
Copy link
Contributor Author

hdweiss commented Dec 24, 2024

I have created issue #4340 to describe why this is an issue for me

@nielsvanvelzen
Copy link
Member

I understand you want this behavior to be different but we'll have to comply with the guidelines123. The engagement time must be set different for certain scenarios and the way it's implemented right now is how it must be. We cannot change that.

Footnotes

  1. https://developer.android.com/training/tv/discovery/guidelines-app-developers

  2. https://developer.android.com/training/tv/discovery/watch-next-programs

  3. https://developer.android.com/reference/androidx/tvprovider/media/tv/TvContractCompat.WatchNextPrograms#COLUMN_LAST_ENGAGEMENT_TIME_UTC_MILLIS()

@hdweiss
Copy link
Contributor Author

hdweiss commented Dec 25, 2024

In the current implementation, the timestamp for WATCH_TYPE_WATCH_NEXT elements is never set correctly. It takes the creation timestamp of the episode, instead of the last engagement time of the last watched episode. If there currently was a way to retrieve the last engagement time for a show from the Jellyfin API, this could be solved more straightforward. I do agree that the other three watch types are ordered correctly.

My PR changes the following. Episodes are inserted a single time and will only be deleted once they are removed from the Jellyfin "up next"/"continue watching". Continue watching episodes have an egangement timestamp, which will be continued to be used. For the WATCH_TYPE_WATCH_NEXT episodes, we must find the engagement timestamp of the last episode watched and not the current episodes engangement timestamp. I did not find a clean way to retrieve the last engangement timestamp on a TV show via the API, even though I did discover that this information is available. If there was an easy way to retrieve this via the API, I have missed it, apologies.

https://github.com/jellyfin/jellyfin/blob/5774b601f59762fb3a02a1555d2aed5803ac6693/Emby.Server.Implementations/Data/SqliteItemRepository.cs#L2864

When a WATCH_TYPE_WATCH_NEXT episode is missing from the row, it must mean that the egangement with a tv show must have happened after last time the LeanbackChannelWorker had been invoked. Because I did not have the ability to retrieve the exact engagement timestamp, I default to the current time/date. This will put all new WATCH_TYPE_WATCH_NEXT in the front of the row. Once a WATCH_TYPE_WATCH_NEXT episode is inserted, it will keep that timestamp indefinitely. I understand that strictly speaking the "correct" timestamp is somewhere in between the last time the LeanbackWorker was called and the current date/time, but the current behaviour never leads to any incorrect ordering in accordance to the Android TV guidelines.

I have been daily driving my branch for more than one week and it always leads to the desired behaviour, which is also in line with the guidelines. This means that upon finishing an episode, the new WATCH_TYPE_WATCH_NEXT episode will be added to the front of the watch next row, not the very back.

@hdweiss
Copy link
Contributor Author

hdweiss commented Dec 25, 2024

I did change the WATCH_NEXT_TYPE_NEW to current date/time, which is incorrect. This should be the release date of the episode. I have pushed a change that fixes that to the PR branch.

So the new ordering in the PR should be:
WATCH_NEXT_TYPE_NEW: creation timestamp (old behaviour)
WATCH_NEXT_TYPE_CONTINUE: engagement timestamp (old behaviour)
WATCH_TYPE_WATCH_NEXT: current date/time when element is inserted into the row (new behaviour)

@jellyfin-bot jellyfin-bot added the merge conflict Conflicts prevent merging label Dec 28, 2024
@jellyfin-bot jellyfin-bot removed the merge conflict Conflicts prevent merging label Jan 6, 2025
@hdweiss
Copy link
Contributor Author

hdweiss commented Jan 6, 2025

@nielsvanvelzen could you please take a look at what I wrote above?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants