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

Automatically take action after X seconds (into credits) #195

Closed
wants to merge 4 commits into from

Conversation

behunin
Copy link

@behunin behunin commented Jun 30, 2020

Give an option to automatically play the next item or stop the player, with control over how many seconds before action is taken.

@im85288
Copy link
Owner

im85288 commented Jul 1, 2020

Thanks, I am going to see what @dagwieers thinks about this functionality before pulling in.

@dagwieers
Copy link
Collaborator

Sorry for keeping everyone waiting. When I saw this PR I wasn't sure what this adds as we already have a default action (play next or stop playback). So a bit of information on the use-case is welcome.

@dagwieers dagwieers added enhancement New feature or request question Further information is requested labels Jul 13, 2020
@behunin
Copy link
Author

behunin commented Jul 13, 2020

I use this addon mostly with the netflix addon and wanted to be able to have the next episode play within a few seconds of the end credits, much in the same way netflix does with the player on their site. I decided it would be nice to have control over how many seconds before action was taken and added that as well.

Before this PR I would see the progress bar go to 0% and then see the countdown start with no automatic action taken.

I am on the latest commit of both kodi and this addon.

@dagwieers
Copy link
Collaborator

@behunin I am confused. You first see the progress bar go to 0%, and then the countdown start? That seems like a bug. Countdown and progress bar should be simultaneous. If you can reproduce this, please open a new issue for this.

The default action (autoPlayMode) is configured in the settings, and could be "Play next" or "Stop playing". It defaults to "0" (which is "Play next"). If it doesn't automatically play next after the countdown, then that is a bug. (#139 and #170 were reported before, and #198 proposes a fix)

Having a system-wide option to automatically play (i.e. not wait for a countdown counter) is something I don't think is very useful. It will fail for anything but Up Next. (local content or other VOD add-ons do not have an exact timing for the credits and therefore could cut off the episode prematurely).

So if anything, I think it should be an option to Netflix (or any add-on where this use-case makes sense) that we could support.

@CastagnaIT Is this something you have had requests for before? Does Netflix support such a feature already?

@CastagnaIT
Copy link
Contributor

Hi,
I think he wants to add an option to have a possibility to set a custom timeout time (for automatic behaviour) now is set to 1minute?

personally i do not use upnext but wait a few (customizable) seconds instead of 1 minute i think it's nice

much in the same way netflix does with the player on their site

he refer to website, the autoplay of next episode have a very short timeout (some seconds)

Before this PR I would see the progress bar go to 0% and then see the countdown start with no automatic action taken.

i do not understand this sentence, perhaps he means a bug somewhere?

He also add "Automatic Watch or Stop" option menu
but seem to add again an existing feature
if it is not, is better that @behunin explain the use case and what differs from
"Default action when nothing select" option menu

@behunin
Copy link
Author

behunin commented Jul 13, 2020

You first see the progress bar go to 0%, and then the countdown start?

Yes that is the observed behavior. I was confused by this as well.

This PR checks to see if the UpNext progress bar <= 0 and then takes action if the option I added is set to true.

My intention was to recreate the same functionality as netflix has on its player so maybe this needs to be added to the netflix repo. What are your thoughts on that @CastagnaIT?

I will investigate further today and gather screenshots for a new issue as requested.

@CastagnaIT
Copy link
Contributor

I don't understand anything about what you want to do...
netflix add-on integrates upnext like any other VOD/Streaming add-on
so nothing is specific

@im85288
Copy link
Owner

im85288 commented Jul 13, 2020

Yes I am also confused here @behunin perhaps some screenshots will tell the story better. @CastagnaIT - the last time I used the Netflix addon the popup for upnext was not occurring when the credits started to show. The time before that though it was working fine in that as soon as the show ended the popup came...do you know if this functionality (i.e. calling the show notification time) has changed?

@CastagnaIT
Copy link
Contributor

the last time I used the Netflix addon the popup for upnext was not occurring when the credits started to show.

upnext could not works only in this case:
Kodi 19 + play nf videos from Kodi library

Because the JSON RPC Player.GetProperties in some cases is broken and provide broken data that cause unexpected behaviuours of the add-on service, but usually when happens is shown a popup error notification.

if this is not the case, you'll have to give me a case with steps to replicate it by opening an Issue

@im85288
Copy link
Owner

im85288 commented Jul 13, 2020

Thanks it may have been Kodi 19, I've been messing with all sorts so not sure. I'll check and create an issue if it's not Kodi 19.

@behunin
Copy link
Author

behunin commented Jul 14, 2020

I did an uninstall of kodi and then installed kodi 19 @ 7a65d55, then installed both netflix and upnext using kodi's addon manager.

The strange behavior I mentioned was no longer present so that was my bad. Sorry...

What I did observer is that with "Customize autoplay duration by episode length" set to true and all lengths set to 5. There was no change in the length of time upnext played the next episode. This must be what brought me to make this change and the confusion on my part.

This PR does work as I intended so I would like to refactor it with the settings I added to be auto play for any length. Then have each setting for auto play time disable the other so that only one can be set true.
So either auto play for any length or auto play for custom lengths can be true, not both.

What do you gentlemen think about that idea?

@dagwieers
Copy link
Collaborator

dagwieers commented Jul 14, 2020

What I did observer is that with "Customize autoplay duration by episode length" set to true and all lengths set to 5. There was no change in the length of time upnext played the next episode. This must be what brought me to make this change and the confusion on my part.

The "Customize autoplay duration by episode length" influences the amount of seconds before the end of the video the pop-up will appear, if and only if the video does not already provides this information. Since Netflix already provides detailed information on when the pop-up needs to be shown (i.e. when the credits start) this has no effect on Netflix. It affects local media and other VOD add-ons only. (Maybe we should make this more clear from the settings?)

A generic option that automatically performs the default action (before the video ends) is not foreseen, and I am not sure if Netflix does this currently? If we add this, this would only makes sense for VOD providers that provide detailed information on when the credits start (i.e. Netflix). But I wonder if this would not be something people would configure on a per-addon basis instead.

I am open to the suggestion, but would need to understand how this works in Netflix before making a call. (cc @CastagnaIT)

@dagwieers dagwieers changed the title Automatic Watch or Stop Automatically take action after X seconds (into credits) Jul 14, 2020
@dagwieers
Copy link
Collaborator

I modified the issue subject hoping this is what you intended, if not, feel free to clarify it further :-)

@CastagnaIT
Copy link
Contributor

there is no UpNext customization on netflix addon
the start of the next video, is done according to the UpNext behaviour nothing more...

@im85288
Copy link
Owner

im85288 commented Jul 14, 2020

Just to clarify, yes there is no customisation but the Netflix addon is the only one (as far as I am aware) that uses the facility to show the Popup at the exact time the credits start.

@dagwieers
Copy link
Collaborator

dagwieers commented Jul 14, 2020

@CastagnaIT I understand. But if this functionality is offered by Netflix today, we could make Up Next behave similarly. We have two options for add-ons that have detailed information on credits:

  • New option in Up Next settings: Automatically perform default action after X seconds into credits
  • A new key in the data structure send from the add-on that specifies this value

We could also implement both, a setting in Up Next that can be overridden by the add-on.

My concern is that these options only make sense if we have detailed information on credits, and this is not obvious to the user. So we probably need to reorganize the Up Next settings a bit to make this more clear (no idea yet on how to do this best).

(As usual, good working defaults are the best cure for complex settings)

@CastagnaIT
Copy link
Contributor

This is the NF website behaviour:

when credits time is fired will show these buttons
image
NF let to user only about 6 seconds to choose what doing
if the user not do a choice automatically fire "Next episode" button

Now i see your problem,
if the credits offset value is not available, this new behaviour option not have much sense

maybe yes, a solution is let an add-on the choice to enable a window time for the default action
but only with a boolean option parameter,
and so let user set the window time length in UpNext settings
but you should warn the user that it only works with add-ons that support this feature

@im85288
Copy link
Owner

im85288 commented Jul 14, 2020

Don't we already support this feature? The user can choose if the default action is to 'Play Next' the default and the time in seconds for the notification to be shown (5, 10, 15, 20, 25, 30) etc etc

@dagwieers
Copy link
Collaborator

Netflix doesn't even offer the option to stop watching, it either is "Continue watching credits" (which in our case is the same as escape/back to dismiss the pop-up, or not doing anything) or "Playing next episode". Obviously you can always stop watching in the next episode, so this is convenient (if you have the credits offset).

So we could implement this behaviour, both visually as well as functionally: Show a "Watch credits" and "Next episode" buttons, but the current settings we have would be even more confusing if we don't clearly differentiate the settings by whether we have a credits-offset or not.

@CastagnaIT I will let you decide because you are the only consumer of this functionality if it were available.
Is this something you would like to offer? And would you make it configurable in Up Next, or set by the add-on?

@dagwieers
Copy link
Collaborator

dagwieers commented Jul 14, 2020

@im85288 Yes, we offer a choice, but we don't automatically continue with the default action after 6 seconds, the user either has to grab for the remote, or wait until the end of the credits. It hasn't bothered me, but I can see how this would accommodate the lazy watcher 😉

@im85288
Copy link
Owner

im85288 commented Jul 14, 2020

At yes I get it now, so for all the other addons where we show the popup X seconds from the end it works fine..but yeah I agree for Netflix where sometimes you have to wait 2 minutes or even more this would be a handy feature to build in. In fact I think if we can work it out it should just work that way by default..eg. Does the play next after the countdown finishes or whatever the user has selected as the default Action.

@CastagnaIT
Copy link
Contributor

CastagnaIT commented Jul 14, 2020

if you want to implement the window time customization for automatic default action it's fine to me
i will activate the functionality when it is ready

I would prefer just to add a boolean property key to the data to send in order to enable it

if you want you can automatically enable it when "notification_time" or "notification_offset" key value exists (and avoid add a new property key)

@dagwieers
Copy link
Collaborator

@CastagnaIT I was more thinking of a time window in seconds (e.g. autoplay_delay) that would be provided. For Netflix that would then be 6 (seconds). If it would not be provided, we would not act on it. (So not add this as a setting in Up Next even)

But if this would be set, we could also modify the pop-up to only have "Watch credits" and "Play next episode" and maybe even make the "Play next episode" the default action. It would mean that some Up Next settings would no longer affect Netflix (and other add-ons providing credits offset)

I am not 100% sure if we want this (i.e. change the look and feel) for Netflix. It could be confusing for people that do not care if something was played from Netflix or another means.

@dagwieers dagwieers removed the question Further information is requested label Jul 14, 2020
@dagwieers dagwieers added this to the v1.2.0 milestone Jul 14, 2020
@CastagnaIT
Copy link
Contributor

CastagnaIT commented Jul 14, 2020

I don't know who proposed changing the interface to UpNext, but that's not my intention

I'm just saying the time window parameter for "auto play?" or "auto select action" it should be an UpNext settings
not should be handled from add-ons that integrate upnext

I'm not in favor of adding a setting parameter in my add-on settings for something that should be handled "externally"
I don't see the point...(i means a setting param to set the time window)

@dagwieers
Copy link
Collaborator

That's clear, I will not propose to make it more Netflix-alike.

However you proposed here to make it add-on controlled, but I now understand you don't see a need for that. No problem.

@CastagnaIT
Copy link
Contributor

CastagnaIT commented Jul 14, 2020

perhaps i explained it wrong way (cleaned)

@behunin
Copy link
Author

behunin commented Jul 15, 2020

Instead of adding any length option, an option for action after x seconds on any addon that uses upnext with the notification_offset set. Then make it clearer that the other timing options are for addons that don't use that option.

Yea?

@CastagnaIT
Copy link
Contributor

CastagnaIT commented Jul 15, 2020

I personally have no intention of maintaining external add-on features inside the add-on, nf the project is already large enough

@dagwieers
Copy link
Collaborator

@behunin I think your implementation is fine in general. I still need to think on how we can present what options work for which cases to not make things more confusing for our users. Maybe we ought to split the options between: Having detailed credits offset and not having them, but I can't find a good descriptive title for this.

@CastagnaIT
Copy link
Contributor

Can you explain what means this option? that i do not understand
image

@Dnkhatri
Copy link

Can you explain what means this option? that i do not understand
image

That setting is for local files and starts upnext widget according to the length of the video file. For netflix addon as it is already known where the credits start it is ignored.

@CastagnaIT
Copy link
Contributor

CastagnaIT commented Jul 15, 2020

i can suggest something like this:

"-" == submenu

[group-------------Automatic button action-------------]
Autoselect action (when nothing selected) [Play next/Stop]
Enable custom autoselect time [true/false]
-For calculated credits position [slider/seconds]
-For precise credits position [slider/seconds]

That setting is for local files and starts upnext widget according to the length of the video file. For netflix addon as it is already known where the credits start it is ignored

i think it its better add to wiki what means set this functionality

@behunin
Copy link
Author

behunin commented Jul 15, 2020

Automatic Actions after X seconds

  1. Timing for local content, VOD addons, etc (bool)
    X seconds for Y length of episode... (sliders)
  2. Timing for addons that send detailed offset information. [Netflix] (bool)
    X seconds. (slider) (default 6 seconds)

Number one I end with an etc because I don't know all the situations this applies to.
Number two is my suggestion, with the array of addons that use this offset information growing when the time comes.

@dagwieers
Copy link
Collaborator

dagwieers commented Jul 15, 2020

IMO option 1 does not make any sense. We have an estimated/configurable time and we are going to use that as a basis for another estimated time. Chance are you are cutting off an episode prematurely. We cannot have that, it is the opposite of convenience 😉

@dagwieers
Copy link
Collaborator

dagwieers commented Jul 15, 2020

I was thinking of doing it like this.

Behaviour

Features
Default action when nothing selected
Include already watched episodes
Number of episodes before "Still there?" query

When credits offset is known (e.g. Netflix)
Duration before automatically performing default action

When credits offset is unknown
Duration in seconds for notification to be shown
Customize notification time by episode length (← I improved this slightly, and removed autoplay from the description)

When we have support for EDL (see #171) it would be added next to Netflix, same for HBO or Disney if they offers this too.

I am also considering to do away with the default notification time, and only allow for the customized option. People can still set the same notification time for different length videos if they like, but I think that is inferior.

@behunin
Copy link
Author

behunin commented Jul 15, 2020

So are you saying just X seconds for both? I think just having two timings would make it simpler, no boolean, just a slider for each timing. Default 6 seconds for both?

Edit: I see now, I like it.

@dagwieers

This comment has been minimized.

@im85288
Copy link
Owner

im85288 commented Jul 15, 2020

@dagwieers could this also work for items where the credits offset is not known (ie. anything but Netflix at the moment)..for example you want the popup to show 30 seconds from the end, but then you also want it to play the next episode automatically after 6 seconds of no activity..In other words perfect for lazy people?

@dagwieers
Copy link
Collaborator

@im85288 No, because we don't know if the credits will be at 30 - 6 (24 seconds). It might only be at 10 seconds before the end, and you cut off the video prematurely (see #195 (comment)). We already estimated the values to be before the credits start (because if the notification is shown after the credits are shown, that is not convenient either).

@im85288
Copy link
Owner

im85288 commented Jul 15, 2020

@dagwieers - I don't follow. I'm only referring to the path when the pop up is shown at the use defined setting. So if it is shown 30 seconds from the end we know there is 30 seconds until it would auto play. In that case we could allow auto play after a period shorter than the 30 seconds.

I may be missing something??

@dagwieers
Copy link
Collaborator

dagwieers commented Jul 15, 2020

@im85288 I am referring to that same path. So what if the credits only start 10 seconds before the end. Notification time is 30 and autoplay time is 6. You are cutting off the last 14 seconds of an episode.

If we don't know when the credits actually start, the following is important:

  • The notification time we propose should be before the actual credits start
  • The user needs to acknowledge explicitly when to start the next episode

We cannot offer automatic proceeding because we have no clue if/when the credits have started. And we cannot risk cutting of a video prematurely. With EDL support we could offer it for local files though, but IMO EDL support should be part of Kodi's metadata, not an Up Next feature.

@im85288
Copy link
Owner

im85288 commented Jul 15, 2020

Fair enough, I understand what you mean. It is probably safer as you suggest not to add this feature then for the occasion when we don't know exactly when the credits will start.

I agree about EDL being preferred as a native kodi feature. Plex has recently started offering the Skip Intro feature on their clients (Apple TV etc) and it works well. They do some sort of analysis on the files prior which is clever stuff.

@behunin
Copy link
Author

behunin commented Jul 15, 2020

@dagwieers @im85288 Please review revisions.

@dagwieers
Copy link
Collaborator

@behunin Will check later tonight!

@dagwieers
Copy link
Collaborator

dagwieers commented Jul 15, 2020

@behunin Next time you make a PR, please always make a separate branch and dont use your master branch. Because of this I cannot add the necessary commits to your branch and add the work as a single commit... I will have to make a new PR (#200) with your commits included.

@dagwieers dagwieers closed this Jul 15, 2020
@Dnkhatri
Copy link

Fair enough, I understand what you mean. It is probably safer as you suggest not to add this feature then for the occasion when we don't know exactly when the credits will start.

I agree about EDL being preferred as a native kodi feature. Plex has recently started offering the Skip Intro feature on their clients (Apple TV etc) and it works well. They do some sort of analysis on the files prior which is clever stuff.

Yeah it should be a Kodi feature but some of the edl functionality got broken after 17.6 release and has never been fixed till now. I don't think any of the current Kodi dev use this feature much so it has been neglected. As Kodi is a hobby project for most of them so I don't blame them as they would work on things that interest them but I have given up hope that it will get implemented. I waited for 6 months hoping it will get implemented in kodi 19 but with the Alpha to drop and no progress on it I thought I put in my request again.

@dagwieers dagwieers modified the milestones: v1.2.0, v1.1.3 Oct 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants