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

v7.1.0: Change default value for 'recordings lifetime' setting #460

Merged
merged 4 commits into from Jul 1, 2020

Conversation

ksooo
Copy link
Member

@ksooo ksooo commented Jun 28, 2020

  • Add new setting value for recordings lifetime of timers created by Kodi: 'Use backend setting"
  • Change default value for 'lifetime' setting from '3 months' to 'Use backend setting' to avoid unintended loss of data.
  • Enforce reset of default value for lifetime' to 'Use backend setting' to avoid unintended loss of data for existing addon configurations

As discussed here: #458

@m4tt075 mind doing a runtime-test?

@m4tt075
Copy link
Contributor

m4tt075 commented Jun 28, 2020

@ksooo Certainly. Could you please create a Windows build for me to test it?

@ksooo
Copy link
Member Author

ksooo commented Jun 28, 2020

@ksooo
Copy link
Member Author

ksooo commented Jun 30, 2020

@m4tt075 any feedback?

@m4tt075
Copy link
Contributor

m4tt075 commented Jun 30, 2020

@ksooo Yes, here where I am:

Testenvironment

  • Kodi Build 19.0-ALPHA1 (18.9.701), Nightly from 2020-06-27 on Windows
  • The addon build you referenced
  • TVH backend 4.3-1886~g51a4c5bec (dockered)

Test results

Backend setting Addon timer setting Retention value in recording log Removal value in recording log
Forever Use backend setting 0 0
1 week Use backend setting 0 0
1 week 2 weeks 0 14
6 months 2 weeks 0 14
6 months Use backend setting 0 0
Forever Use backend setting 0 0

The way I read this, specified timer settings in the addon are interpreted correctly by the backend, but the "Use backend setting" sets retention to forever instead of respecting the actual backend setting. I have to say that I did not restart the backend after changing the backend settings though, as I don't think that that should be required. Anyways, please let me know if I should test differently...

@ksooo
Copy link
Member Author

ksooo commented Jun 30, 2020

> but the "Use backend setting" sets retention to forever instead of respecting the actual backend setting

Then, you have to open an issue for tvheadend. This is clearly a backend bug.

EDIT: Not sure you are reading the backend log correctly. A "0" there imo means "use value specified in the corresponding dvr backend config". 0 does NOT mean 'forever'. 'Forever' is MAX_INT (thus a very big number). For me, the log reads like everything should work as expectd now.

@m4tt075
Copy link
Contributor

m4tt075 commented Jun 30, 2020

Not sure either, but I would have expected different backend retention values to be reflected in the dvr/log files. It would also mean that changing the rention time in the backend was applied retrospectively to old recordings, which seems counter-intuitive to me. I have not been able to find other, related parameters in the log files either, though... Hmh.

@ksooo
Copy link
Member Author

ksooo commented Jun 30, 2020

I'm afraid you have to open an issue for tvheaded if things do not work as expected. Nothing the addon can do about that. I just use an offcial tvheadend API.

@ksooo
Copy link
Member Author

ksooo commented Jun 30, 2020

How about, instead of trying to "decrypt" the log files, just doing a real runtime test?

@ksooo
Copy link
Member Author

ksooo commented Jun 30, 2020

Regarding difference between retention and removal: this addon only modifies removal. removal is about when the file (incl. db entry) gets removed, retention is when the db entry is removed without removing the recording file. A mess, if you ask me and very confusing.

So, I'm not astonished that your test reveals that retention value is always zero. The addon does not touch this value, so it always is 0 (which means "current DVR config").

@ksooo
Copy link
Member Author

ksooo commented Jun 30, 2020

To conclude: I think neither tvheadeend is buggy nor the implementation done with this PR. You simply misread the log files.

@m4tt075
Copy link
Contributor

m4tt075 commented Jun 30, 2020

Might well be the case, @ksooo. I'm in no position to judge. Just testing and reporting... :-)
If you want me to test or report something else, please let me know. If you are reassured, please just move on...
I appreciate you took this on. If it works correctly, this is a clear improvement IMHO. Much appreciated...

@m4tt075
Copy link
Contributor

m4tt075 commented Jun 30, 2020

I learned something more from reading old forum posts: The "retention" value refers to the dvr LOG retention period and has nothing to do with the FILE retention. The FILE retention indeed is supposed to be stored in the "removal" variable. And I've found some old, unanswered reports that the backend settings are not always operating correctly. Maybe this IS a backend problem after all. I'll open a ticket on tvheadend.org. Maybe "Flole" knows more...

@ksooo ksooo merged commit 761b376 into kodi-pvr:Matrix Jul 1, 2020
@ksooo ksooo deleted the retention-default branch July 1, 2020 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants