search

plexinc / plex-for-kodi

Offical Plex for Kodi add-on releases.
GNU General Public License v2.0
249 stars 124 forks source link

Add intro skipping #314

Closed pannal closed 4 years ago

pannal commented 4 years ago

GHI (If applicable): #

Description:

Adds the support for markers and thus intro skipping for PlexPass enabled users.

Checklist:

mirkster commented 4 years ago

So happy to see this. I am seeing Skip Intro on the Android and Roku clients but was missing it from Plex for Kodi.

dfekete commented 4 years ago

That was fast, well done! Hope the merge of this PR can be fast-tracked :)

ruuk commented 4 years ago

I missed one other thing. The button is only supposed to display for 10 seconds without the OSD. If 10 seconds has elapsed or the OSD has been show, then it will only show in the OSD until the end of the window.

jordankoehn commented 4 years ago

It seems that if you skip intro then rewind, skip intro doesn't appear again, only initially. Minor bug really, if it's a technical limitation it's not the end of the world

pannal commented 4 years ago

That's actually intentional and in line with the other clients. The button then only shows up when the OSD is activated. Right @ruuk?

jordankoehn commented 4 years ago

Plex on android you can skip intro, rewind 10 seconds, skip intro, repeat unlimited times

ruuk commented 4 years ago

The skip intro button should always be visible in the OSD during the intro window, but should only display over the video (i.e. without OSD) the first time.

ruuk commented 4 years ago

Thanks, @pannal!

Diniboy1123 commented 3 years ago

Sorry to bring this old closed PR thread off, but I have the issue described in #351 and I figured it's due to the hasPlexPass change to a one-liner in this PR:

return self.isPlexPass or (self.isManaged and self.adminHasPlexPass)

This won't return True if the user is a home user, but is not managed. So the skip intro button will only work on truly managed home users, but not on the home users with their own plex account. I understand that the one-liner looks better, but may I ask what was the motivation to change that line? @pannal

pannal commented 3 years ago

@ruuk I think we talked about changing that check to the one liner. I don't remember the reason for it, though.

ruuk commented 3 years ago

I believe the code is correct here, but the old code was definitely wrong. It was always returning managed users as having a plex pass, regardless of the managing account.

It looks like the app should actually be checking if the user has the intro-markers feature rather than if the user has a plex pass.

ruuk commented 3 years ago

A quick check of the features for a non-plex pass normal user in my home shows the intro-markers feature so checking the feature should get the behavior you're expecting.

Diniboy1123 commented 3 years ago

Thanks for double-checking! And my apologies for bringing this old PR thread up again... However may I know which endpoint returns that intro-markers feature? I would try to monkey patch it myself.