2015-01-10, 06:55
For those who dont know there is a pull request from Glenn-1990 on github to add core support for Series Timers in Kodi 15 I*****.
Wonderful to see!
After working on pvr.wmc to support this new "Advanced Timers" Kodi implementation I had numerous items to discuss and feedback etc with Glenn and other PVR developers, hence creating this thread.
The reason i make all these suggestions is that once this change is merged, all PVR addons need to make changes anyway, and those with active development that actually want to support series recording need to make more than just trivial changes too. So while we are all in the process of doing that, let's get it RIGHT and make it user intuitive etc... eg by having more sensible concepts around Priority and Lifetime, making user's lives easy by being able to specify their preferred defaults for these options and so on
So here are the different comments/suggestions Id like us to discuss
Existing Addons
given that with this change all existing addons now fail to compile, how should we handle it? The 2 reasons are that the DeleteTimer function now takes an extra bool parameter and the AddonCapabilities structure no longer has a SupportsTimers member. One way would be to update all addons to have the new DeleteTimer prototype but not do anything with the extra boolean, or potentially even return the NOT_SUPPORTED return code if bDeleteSchedule is true. And on the capabilities side obviously remove SupportsTimers and then perhaps even add a default state to the new bitmask flagging that they at least support the non series timer types. The intention being to at least hopefully maintain the way addons are currently functioning. Im not sure whether all addons have acive developers on them or not so obviously something has to be done to get builds to compile in the first place, and ideally even preserving however those addons worked in Helix
Timer Priority Field
Recording Priority being a numeric value from 1 to 99 seems a bit odd. Im not sure what other backends do but this doesnt gel that well for our backend (WMC) which has a very large LONG value for priority but when ueed normally actually allows users to see all pending recordings and arrange them up/down in the list. I cant imagine a user wants to actually change a numeric value between 1 and 99 with a spinner control anyway so its not really intuitive. I realise this is how it already was and you're just re-using the edit timer dialog, but I propose we change the Priority concept for recordings to something more sensible like "Low, Normal, High" options, or even "Lowest, Low, Normal, High, Highest" if people preferred that. I think this is much easier for users and also should be easy for most addons to map into whatever priority concept the backend uses (much more so than a numeric 1-99 value)
Weekday/Weekend Timer Type
The recording types all indicate whether they are for AnyChannel or ThisChannel, except the Weekday and Weekend ones. Shouldnt there really be AnyChannelWeekday/ThisChannelWeekDay and AnyChannelWeekends/ThisChannelWeekends? If not what is the expected value for AnyChannel/ThisChannel when using those weekday/weekend options? In our specific backend we actually have a bitflags enum allowing any day/s of the week to be selected. Obviuolsy we can map weekday/weekend back to these but I dont know whether users would like to have individual days able to be selected or not etc. Im ok with the current list that just includes weekday/weekend (not that id use them personally) but I do say as ive just mentioned that for completeness it does seem like we need the AnyChannel/ThisChannel versions of each
Timer Lifetime Field
The lifetime options could be better and more intuitive - instead of just a number of days I know our backend and I assume others also support things like "Until Watched" "Until Space Needed" "Forever" etc. Perhaps this could be handled just like the recording types by having a larger list of supported values in Kodi, and a bitmask for addons to indicate which options they can support and thus which options can be shown in the list. Another option would be an addon callback to provide the items for these fields, meaning addons could have whatever they wanted here. And then it would be the addons responsibility to have hte strings for these options localised and so on. So in other words our backend might have 1 day, 5 days, Until Watched and Forever, whilst another addon might have 1 day/1 month/etc. I assume the latter is harder to do, but i think the first suggestion would be easy (as easy as the recording types this PR implements anyway) ie come up with a more conclusive hardcoded list and a bitmask in the addon capabilities to indicate which ones the given addon can support
Default Values
If a user always likes to have their AdvancedRecordings on AnyTimeThisChannel, NewOnly, Priority 73, Pre/Post padding 5 minutes etc, we should ideally let them indicate that rather than have to adjust all those parameters every time. Our previous custom rolled series timer support allowed users to specify their preferred defaults for type, pre padding/post padding etc. This could be done in Kodi settings of course, but thats potentially a bunch more settings plus for those with multiple clients they have to configure them all individually... so instead i propose a better way would be a new addon callback GetAdvancedTimerDefaults(type, bNewOnly, prePadding, postPadding) that addons could implement if they want. - with these coming from the backend it means for those users who have multiple clients they wouldnt have to set the settings on all of them
I hope to have some good discussion here that hopefully results in modifying the PR a little bit, and then we addon developers can get cracking on implementing the series support (and in a few cases ripping out the custom rolled one). I think I***** will be great for PVR users experience if we can just get this PR as close to perfect as possible!
Wonderful to see!
After working on pvr.wmc to support this new "Advanced Timers" Kodi implementation I had numerous items to discuss and feedback etc with Glenn and other PVR developers, hence creating this thread.
The reason i make all these suggestions is that once this change is merged, all PVR addons need to make changes anyway, and those with active development that actually want to support series recording need to make more than just trivial changes too. So while we are all in the process of doing that, let's get it RIGHT and make it user intuitive etc... eg by having more sensible concepts around Priority and Lifetime, making user's lives easy by being able to specify their preferred defaults for these options and so on
So here are the different comments/suggestions Id like us to discuss
Existing Addons
given that with this change all existing addons now fail to compile, how should we handle it? The 2 reasons are that the DeleteTimer function now takes an extra bool parameter and the AddonCapabilities structure no longer has a SupportsTimers member. One way would be to update all addons to have the new DeleteTimer prototype but not do anything with the extra boolean, or potentially even return the NOT_SUPPORTED return code if bDeleteSchedule is true. And on the capabilities side obviously remove SupportsTimers and then perhaps even add a default state to the new bitmask flagging that they at least support the non series timer types. The intention being to at least hopefully maintain the way addons are currently functioning. Im not sure whether all addons have acive developers on them or not so obviously something has to be done to get builds to compile in the first place, and ideally even preserving however those addons worked in Helix
Timer Priority Field
Recording Priority being a numeric value from 1 to 99 seems a bit odd. Im not sure what other backends do but this doesnt gel that well for our backend (WMC) which has a very large LONG value for priority but when ueed normally actually allows users to see all pending recordings and arrange them up/down in the list. I cant imagine a user wants to actually change a numeric value between 1 and 99 with a spinner control anyway so its not really intuitive. I realise this is how it already was and you're just re-using the edit timer dialog, but I propose we change the Priority concept for recordings to something more sensible like "Low, Normal, High" options, or even "Lowest, Low, Normal, High, Highest" if people preferred that. I think this is much easier for users and also should be easy for most addons to map into whatever priority concept the backend uses (much more so than a numeric 1-99 value)
Weekday/Weekend Timer Type
The recording types all indicate whether they are for AnyChannel or ThisChannel, except the Weekday and Weekend ones. Shouldnt there really be AnyChannelWeekday/ThisChannelWeekDay and AnyChannelWeekends/ThisChannelWeekends? If not what is the expected value for AnyChannel/ThisChannel when using those weekday/weekend options? In our specific backend we actually have a bitflags enum allowing any day/s of the week to be selected. Obviuolsy we can map weekday/weekend back to these but I dont know whether users would like to have individual days able to be selected or not etc. Im ok with the current list that just includes weekday/weekend (not that id use them personally) but I do say as ive just mentioned that for completeness it does seem like we need the AnyChannel/ThisChannel versions of each
Timer Lifetime Field
The lifetime options could be better and more intuitive - instead of just a number of days I know our backend and I assume others also support things like "Until Watched" "Until Space Needed" "Forever" etc. Perhaps this could be handled just like the recording types by having a larger list of supported values in Kodi, and a bitmask for addons to indicate which options they can support and thus which options can be shown in the list. Another option would be an addon callback to provide the items for these fields, meaning addons could have whatever they wanted here. And then it would be the addons responsibility to have hte strings for these options localised and so on. So in other words our backend might have 1 day, 5 days, Until Watched and Forever, whilst another addon might have 1 day/1 month/etc. I assume the latter is harder to do, but i think the first suggestion would be easy (as easy as the recording types this PR implements anyway) ie come up with a more conclusive hardcoded list and a bitmask in the addon capabilities to indicate which ones the given addon can support
Default Values
If a user always likes to have their AdvancedRecordings on AnyTimeThisChannel, NewOnly, Priority 73, Pre/Post padding 5 minutes etc, we should ideally let them indicate that rather than have to adjust all those parameters every time. Our previous custom rolled series timer support allowed users to specify their preferred defaults for type, pre padding/post padding etc. This could be done in Kodi settings of course, but thats potentially a bunch more settings plus for those with multiple clients they have to configure them all individually... so instead i propose a better way would be a new addon callback GetAdvancedTimerDefaults(type, bNewOnly, prePadding, postPadding) that addons could implement if they want. - with these coming from the backend it means for those users who have multiple clients they wouldnt have to set the settings on all of them
I hope to have some good discussion here that hopefully results in modifying the PR a little bit, and then we addon developers can get cracking on implementing the series support (and in a few cases ripping out the custom rolled one). I think I***** will be great for PVR users experience if we can just get this PR as close to perfect as possible!