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

feat: Upnext addon integration #45

Open
wants to merge 7 commits into
base: nexus-staging
Choose a base branch
from

Conversation

lumiru
Copy link

@lumiru lumiru commented Feb 11, 2024

No description provided.

@smirgol smirgol self-requested a review February 11, 2024 16:51
resources/lib/model.py Outdated Show resolved Hide resolved
@smirgol
Copy link
Owner

smirgol commented Feb 11, 2024

Thank you very much for your work. At a first glance it looks good to me. I'll merge it locally first and play around with it a bit to see if I can break it. :)

@smirgol
Copy link
Owner

smirgol commented Feb 12, 2024

I did some cleanup and bug fixing, but overall it seems to work, good job. I also like that the commits are much much cleaner this time ;) I'll push my changes once I'm done with them.

Just one thing: Things completely go haywire if the option "skip outro" is enabled. I have no good idea how to solve this, yet. Also it might be worth investigating if the UpNext notification could be made a bit less obtrusive - it adds a dark layer on the video and effectively prevents you from watching the credits and/or the preview of the next episode. Not to speak that it prevents skipping the outro - and if that would work, it probably break the UpNext timer in return.

The preview of the next episode, that is a very common thing with animes, is the main reason I've not actively pursued to implement this feature, as these two things collide. That is very different with e.g. Netflix episodes, where there are only tons of credits at the end and nothing you'd want to watch (usually), so the UpNext totally makes sense there.

@lumiru
Copy link
Author

lumiru commented Feb 19, 2024

I agree with you, it could be annoying, depending on your theme, or if you enabled skip outro, if outro/preview was not found from API (default UpNext configuration make it pop 40s before the end of the video). In other cases, it depends on the following algorithm. Please note I consider preview is not a part of the episode so we can ignore it.

  • If outro ends when preview starts, show UpNext at the outro beginning,
  • else if preview start is set, show UpNext at the preview beginning,
  • else (since preview is not set) if outro is set and ends in the last 20 seconds (consider it will be an untagged preview), show UpNext at the outro beginning,
  • else use default UpNext behaviour.

We could add a bunch of settings:

  • UpNext integration mode (default: Disabled?):
    • End detection when available (current behaviour)
    • Default UpNext behaviour
    • Disabled
  • Override default UpNext duration for notification to be shown (default: 15)

@rjousse18
Copy link

Hi ! Any progress on this feature ?

@smirgol
Copy link
Owner

smirgol commented Apr 24, 2024

Nah not really. It was giving me headaches in combination with the skip outro functionality and thus requires some fiddling, but, in all honesty, I could not motivate myself to work on it yet.

@lumiru
Copy link
Author

lumiru commented Apr 26, 2024

There is a new bug since the next API update with this addon because we cannot start a video play while another one is playing. I plan to work on it on the next two weeks and try to do what I listed in my last comment at once.

@smirgol
Copy link
Owner

smirgol commented Apr 26, 2024

If you are talking about the restriction from crunchyroll, that is limited to 5 concurrent streams, so I think. There has been an issue with the addon when switching streams fast, because I did not release the previous stream before starting a new one.
But meanwhile I've added that, so it should not hit that 5 "parallel" streams limit anymore:

video_player.clear_active_stream()

But I have to admit that I have not tested my implementation. :)

@lumiru
Copy link
Author

lumiru commented May 13, 2024

If you are talking about the restriction from crunchyroll, that is limited to 5 concurrent streams, so I think. There has been an issue with the addon when switching streams fast, because I did not release the previous stream before starting a new one. But meanwhile I've added that, so it should not hit that 5 "parallel" streams limit anymore:

video_player.clear_active_stream()

But I have to admit that I have not tested my implementation. :)

Not sure this is the fix, but I do not met this bug since I rebase everything.

resources/language/resource.language.fr_fr/strings.po Outdated Show resolved Hide resolved
resources/lib/videoplayer.py Show resolved Hide resolved
resources/lib/videoplayer.py Outdated Show resolved Hide resolved
@smirgol
Copy link
Owner

smirgol commented May 13, 2024

If you are talking about the restriction from crunchyroll, that is limited to 5 concurrent streams, so I think. There has been an issue with the addon when switching streams fast, because I did not release the previous stream before starting a new one. But meanwhile I've added that, so it should not hit that 5 "parallel" streams limit anymore:

video_player.clear_active_stream()

But I have to admit that I have not tested my implementation. :)

Not sure this is the fix, but I do not met this bug since I rebase everything.

I've checked recently and it seems to work.

resources/settings.xml Outdated Show resolved Hide resolved
@smirgol
Copy link
Owner

smirgol commented May 13, 2024

Thank you for updating the branch, I'll give it a whirl the next days and see how it goes. :)

@smirgol
Copy link
Owner

smirgol commented May 13, 2024

There's another thing: When calculating when to show the UpNext, you refer to the preview skip data, but we don't support/fetch that:
https://github.com/lumiru/plugin.video.crunchyroll/blob/fd0dd73d8aec9cf35564609986064ab8655a108f/resources/lib/videostream.py#L358

@smirgol
Copy link
Owner

smirgol commented May 14, 2024

Hm okay, so, after fixing a few type errors that threw exceptions, I was able to test it a little bit. UpNext didn't show up, although it should have. Not sure why, but it was pretty late yesterday, so I could only do a few tests.

Then, the main issue that is giving me headaches isn't solved and I still don't know how to solve it:
If skip_credits is enabled, when to actually show the UpNext?

Say there are credits but no preview:

  • Show at start of credits (as of now): the UpNext Dialogue will interfere with the Skip Dialogue, messing things up
  • Show after the Skip Dialogue (after 10 seconds of credits) - user might have skipped the credits, thus video is now over

Say there are credits and a preview:

  • show at end of credits: preview might only be a very few seconds (very common) so time is too short for UpNext to show
  • show at end of video - fixed time: we likely end up in the credits, which the user might skip, so no UpNext

Say there are no credits, but a preview or no credits and no preview

  • Piece of cake: Show at end of video - fixed

If skip_credits is disabled, things are easy, but they always were, that's not what is giving me headaches. :)

Then, I'm not sure how UpNext works if I tell it to pop up at e.g. 1400 seconds and by skip actions I directly skip from 1350 to 1450, thus bypassing the 1400 mark - will UpNext still pop up, or will it never show up?

It's really giving me headaches and I can't find a proper solution to that. Best things would be to show a custom dialogue after the video has played, asking the user if he directly wants to go to the next episode. As an intermediate between the video and the listings of the addon, if you know what I mean. So not showing anything while the video is actually still playing, but afterwards. That'd would solve the issue with skipping and is the best I can come up with at this time. It would make UpNext obsolete, though.

As said, things are much easier for e.g. Netflix, as at the end of a video there are most of the time only credits (with rare exceptions), which you cannot skip, so you don't have that issue there.

sighs

@lumiru
Copy link
Author

lumiru commented May 14, 2024

Then, the main issue that is giving me headaches isn't solved and I still don't know how to solve it: If skip_credits is enabled, when to actually show the UpNext?

Say there are credits but no preview:

  • Show at start of credits (as of now): the UpNext Dialogue will interfere with the Skip Dialogue, messing things up
  • Show after the Skip Dialogue (after 10 seconds of credits) - user might have skipped the credits, thus video is now over

If there is nothing after credits, maybe the skip dialog is not useful. Indeed, if you want to skip credits, it is to see what is after, and this is the typical case UpNext was created ! This is the same as Netflix.

Say there are credits and a preview:

  • show at end of credits: preview might only be a very few seconds (very common) so time is too short for UpNext to show
  • show at end of video - fixed time: we likely end up in the credits, which the user might skip, so no UpNext

In this case, if the user skip credits, UpNext dialog is shown immediatly. The default UpNext behaviour is to run the next video, but it depends on its own UpNext configuration.

Then, I'm not sure how UpNext works if I tell it to pop up at e.g. 1400 seconds and by skip actions I directly skip from 1350 to 1450, thus bypassing the 1400 mark - will UpNext still pop up, or will it never show up?

(as I previoulsy wrote UpNext dialog is shown immediatly)

There are several ways to see the issues you reveal.

I suggest this one. Consider all cases I met:

  • Video with credits at the very end (e.g. Our Last Crusade or the Rise of a New World):
    • No need about skip button since it is the end of the episode, show UpNext dialog instead.
  • Video with credits, end scenes and preview (e.g. Shangrii La Frontier):
    • Skip button should be shown since we want to see the end scenes, and UpNext will be shown with the preview.
  • Video with credits and short preview (e.g. Dragon Raja):
    • No need about skip button since preview is considered the end of the episode too, show UpNext dialog instead.
  • Video with credits and long preview (e.g. One Piece):
    • No need about skip button since preview is considered the end of the episode too, show UpNext dialog instead.
  • Video without anything tagged from the skip API (a lot 😢, e.g. Sorcerous Orphen Stabber):
    • No skip button will be shown since there is no data about credits, UpNext dialog will be shown in the last 15s.

With all cases, we should not forget to show the skip button if no next episode was found!

resources/lib/videoplayer.py Outdated Show resolved Hide resolved
* add missing translations
* fix typing errors in upnext.py
* send call to UpNext only if enabled

msgctxt "#30091"
msgid "At credits start, if nothing after"
msgstr "Zu Beginn der Credits, wenn danach keine Vorschau kommt"
Copy link
Owner

Choose a reason for hiding this comment

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

Probably need to refine this, I wasn't sure what the english text was trying to tell me. :D

Copy link
Author

Choose a reason for hiding this comment

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

As I wrote in a previous comment, I change it to something more understandable.

In fact, it may be named like that (because, this is what it does):

"Show UpNext dialog at":

  • "credits start"
  • "preview start"
  • "fixed time before the end"
  • "never (disabled)"

smirgol and others added 2 commits May 14, 2024 21:33
* make return value of _compute_when_episode_ends optional
_compute_when_episode_ends was moved to videostream.py since it not need the player context to work and only need to expose data about video stream.
'best' upnext_mode was renamed to 'credits' (because 'best' does not mean anything).
translations was updated with easier to understand wording.
@lumiru
Copy link
Author

lumiru commented May 15, 2024

I applied my last suggest and fixed issues that was listed here. Feel free to share it if you have any doubts or suggests about the way it was done or what I tried to explain before.

@smirgol
Copy link
Owner

smirgol commented Jun 19, 2024

I have not forgotten you, it's just that I barely can keep up with watching the current season animes, so I rarely get to watch older animes where there actually are next episodes to test with. :)
I remember that I once had the chance to do a quick test weeks ago and again ran into the issue with overlapping UpNext and Skip outro. Need to look closer at it, but it's difficult to find the spare time to do so at the moment. :|

@lumiru
Copy link
Author

lumiru commented Jun 19, 2024

Oh, I believed I fixed this issue with my last commit. I never met it since this one with "skip outro" enabled and "credit start" option selected.

Could you share me your settings and maybe the episode where you met this issue? I could take a look on it.

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