-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
[PVR] PVR client cleanup and improvement #13040
Conversation
5d2c7cf
to
5b54914
Compare
jenkins build this please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks sane imo. but I think this is going to be obsolete after we split pvr and streaming api. not sure about the status of @AlwinEsch 's work
I don't think this will be obsolete, as it makes sense as long as you have a PVR API with more than one function, which will be still the case, even after we rip off inputstream functions. |
I'm not aware that Alwin is working on the API split, btw. |
@ksooo before the API can be split the PVR addon has to be migrated to the new design. That I hoped @AlwinEsch was working on. |
Ah, okay. |
First commit is a larger cleanup and routes all calls to the client addon through a single proxy method. So, common tasks before after the client addon call can be held in one place . The diff is large, thus not easy to read. Take a look at a good example snippet, to get the idea: 2a62ed2#diff-d63034db2a0e41f8c4bf14542b168cd4L569
Second commit improves pvr manager shutdown performance. I've seen cases before where the client addon could not reach the backend (network problems, misconfiguration, ...) and pvr manager shutdown took ages, because it had to wait for the timeouts for a bunch of backend calls ongoing while pvr manager should be shut down - pvr gui info thread updating backend data cache and calling several client addon methods in a row is one of the candidates causing this.
This changes are runtime-tested on macOS, latest Kodi master.
@FernetMenta for review?