[ksooo]

Picking up your post on github:

If you edit an 'Anytime' timer (startTime=endTime=0) to convert it to a specific time timer (e.g. 22:00 - 23:00), CGUIDialogPVRTimerSettings::Save() runs the following code snippet, which mangles things, setting startTime=endTime=now(). I guess this is because start time was initially set as 1/1/1970 22:00:

if (m_startLocalTime < now)
m_startLocalTime = now;

if (m_endLocalTime < m_startLocalTime)
{
CLog::Log(LOGWARNING, "CGUIDialogPVRTimerSettings::Save - end time is before start time! Setting to start time.");
m_endLocalTime = m_startLocalTime;
}

For any-time timers, start and end time are initially set to <date now><time now>, not <1/1/1970><time now> which you should see the moment you disable "Any time". Start day and end day fields should show the date from the moment you opened the settings dialog (yes, might be not "today" if you managed to have the dialog open across day boundary) and start time and end time should show the time at the moment you opened the timer settings dialog.

Cannot see how this warning could be triggered in the scenario you described, with the only exception that you managed to cross day boundary and start/end day actually pointing to "yesterday" when closing the dialog, because you only adjusted the time, not the dates. Sounds wild, but could this have happened? Asking beeing a night owl myself...

Could you please tell me what the functional misbehavior is you're experiencing from the warning above? ... and step-by-step instructions how to reproduce it, please.

A similar effect afflicts 'm_firstDayLocalTime' a couple of lines below, which will set all 'first days' in the past to 'now' on timer edit (not good for 'Weekly' rules where 1st day sets the day of the week...).

Forcing firstday to something at least "now" seems to be pointless. Will remove this. Thx for reporting.

[janbar]

Hi @ksooo, currently i can't sync my changes to pvr.mythtv repos since commit 1bcdc418d4bc25fa2f96ac85b9633ffb78a506fe 1 parent 0401335. So my question is when you will revert the commit or at least to support array size for 256 ?

[ksooo]

Sorry, I do not plan to revert anything.

I already tried I to explain that I see no need for > 64 array size. This is purely waste of memory - 256 means still 1.6 MB memory per GetTimerType call. OTOH I also told you that I consider a list with >200 priorities a really bad user experience. This is just tekky stuff with no real end user value, imo. Same holds for lifetime. Average users imo could easily get confused by the pure amount of choices.

I'm afraid that you will have to live with the 64 possible value you now can supply. :-) How about a user friendly mapping done I the add-on? Helps to save memory and makes life for average users easier.

[janbar]

This is purely waste of memory - 256 means still 1.6 MB memory per GetTimerType call

Do you really think it is waste of memory ? You have to know when user open a playback we create buffer for 1MB~2MB to store frame. It would be a good reason to forbid user to open playback too. For your information the memory is reserved only during the call, then it should be freed. Is there a leak somewhere.

OTOH I also told you that I consider a list with >200 priorities a really bad user experience. This is just tekky stuff with no real end user value, imo

Please I invit you to tell it to MythTV team. Sure they would be enjoy to answer you.

So, maybe i waste memory , but sure i wasted my time to develop on this feature. And now you have broken all my work, i don't want retry because in few weeks you will break again others things without really good reason. So my only solution is to remove these features requested by users, but it will take time and for instance i haven't more desire to maintain this addon and probably J**** will live without pvr.mythtv.

Finally i think you should fix the pvr code of this feature:
- file PVRClient.cpp, line 421:

Code:

PVR_TIMER_TYPE types_array[PVR_ADDON_TIMERTYPE_ARRAY_SIZE];

You should "new" that. Even decreasing the array size to 64, we have a risk of stack overflow. To resolve that you simply have to store it in the heap and finally freeing once you have transferred data.

tchao.

[ksooo]

@janbar calm down. Certainly not all of your work gets useless due to the decreased array size, right?

Maybe others do also have an opinion on this? Might help us to find the "right" answer.

[scarecrow420]

My opinion is it's crazy to have 200 values for something like these and I can't see how it could possibly be what a user would want or need. For wmc addon I am using 3 priorities and 6 lifetimes

What are the 200 values that myth tv uses and are you sure the user's really want to have all those options presented? They are using a remote control to input these options in most cases remember...

I do agree on "newing" the array to avoid stack overflow, I think that fix is already in a pending PR anyhow

[MikeB2013]

mythtvbackend allows a priority in the range -99 to +99 with 0 being default (which means let mythbackend scheduler sort out what to record using built-in rules). Adjusting the priority tells the mythtvbackend scheduler to either promote (+ve numbers) or demote (-ve numbers) a particular recording rule/timer within the built-in rules.

I have no idea how many existing recording rules/timers on mythtvbackends use large values of priority.
Initially most timers will have been created outside of Kodi so we need to handle the full range.

Mike

[metaron]

sorry @janbar, I have to agree with @ksooo on this one.
My opinion is if you have to scroll a list to get to to the setting you want, you have too many settings on the page.

According to the database schema mythtv supports sizeof(int(10))) for priority and sizeof(int(11)) for number of recordings to keep.
Mythfrontend simply displays a current value for priority (default 0) and allows you to increase or decrease it using a spinner.

For Lifetime there are three separate variables:

• A spinner: Max episodes to keep. Starts at 0 (No episode limit).
  
• A boolean: 'Delete oldest if this would exceed the max episodes' or 'Don't record if this would exceed the max episodes'. This is only enabled when Max episodes to keep != 0.
  
• Allow recordings to expire. This is independent of the other values.
  

The issue occurs because 'Lifetime' and 'Priority' aren't spinners in kodi, plus we are trying to squeeze one and a half extra booleans into the Lifetime list.

Probably +/- 5 would be enough for priority with excessive values shown in the list as >5 or <-5 (and the backend rule value not changed if these values are selected - my working version of the timer code could do this relatively easily).

I already have a proposal for lifetime which uses far less than 64 settings (https://github.com/janbar/pvr.mythtv/pull/5) although I accept that solution isn't perfect.

Yes, it will require us to diverge from mythfrontend philosophy a little, and we may not be able to support all the options, but I think with careful consideration and experimentation we can come up with something which works well and is 'simple' to use.
(Don't worry, where we can't I will help you pester Ksooo and friends to change the API so we can.)

Unfortunately I suspect it will require quite a bit of iteration which I am very willing to help you with.
The good news is, as you pointed out before, we have about 4 months to get this right before Jxxx hits the streets.

[janbar]

All rules outside the permitted range will be "Unhandled" by kodi. This is the reason why we cannot be satisfied by any solution without range provided by backend. @metaron I don't want provide a way which will break rules of users: If a user set priority -82, the addon cannot override value with a ball shit. About series expiration the only way is to remove the feature. And finally about recordings group, you are limited to 64 folders (LiveTV , Default are included) which is probably to short: same here i prefer remove the feature and propose instead the text field to enter manually the folder.

EDIT: I missed a last thing, there hasn't problem with remote to scroll hundred of items. All remote have repeat keys or page-up, page-down. If you need a reference ask me, i have one for 10 euros.

[ksooo]

Although I really do not like the tone of @janbar and cannot completely get his points, for the pure sake of freedom I will revert the buffer size decrease. Size will be 512 again, like it was initially.

[janbar]

Thanks @ksooo. Sorry for my tone, my english is not good and i explain my point of view as i can. In a same time i will make a PR to load timer types as the same way of recordings.

[ksooo]

@janbar no need for another PR. I will take care of the timer type allocation stuff.

[metaron]

Cannot see how this warning could be triggered in the scenario you described, with the only exception that you managed to cross day boundary and start/end day actually pointing to "yesterday" when closing the dialog, because you only adjusted the time, not the dates. Sounds wild, but could this have happened? Asking beeing a night owl myself...

Could you please tell me what the functional misbehavior is you're experiencing from the warning above? ... and step-by-step instructions how to reproduce it, please.

I can't seem to reproduce it at present using your pr7079-followup-2 branch. I've also found some errors in my working version of pvr.mythv which need to be fixed before I can be sure. I am continuing to investigate.
I can say that I was editing an existing timer rather than creating a new one and it happened several times, but the night owl comment my very well apply. Doing something at 23:59:59 on 31st December is always a good test case .

EDIT: I just noticed you said 'only adjusted the time, not the dates'. This is definitely the case as the dates aren't shown for this timer type only the times.

[scarecrow420]

Hey guys, while working on my pvr.wmc implementation I noticed that if you have "child" timers that are created by the parent repeating timer, when you click on the parent in the GUI and are viewing the child timers, there is no ".." item at the top of the list to take you back to the main timer screen. If you have a parent with no children, you do get a ".." option when inside the parent

Of course you can use the "back" button on the remote but just wondering if this was by design or should be fixed, as it's not consistent with other areas in Kodi where there is always the ".." option to go up a level.

Other than that I don't have much to report except that the API changes are allowing me to do almost everything we would want so great job ksooo Glenn and others once I've finished my first cut I'll publish it for peer and user review and comment, hopefully sometime this week

[ksooo]

I know about the ".." problem, but have not found a solution yet. I'm not that familiar with the GUI code. If someone knows how to fix this I would be happy about a hint or even a PR.