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

Implement PVR API 5.1.0 #206

Merged
merged 4 commits into from Mar 11, 2016
Merged

Implement PVR API 5.1.0 #206

merged 4 commits into from Mar 11, 2016

Conversation

ksooo
Copy link
Member

@ksooo ksooo commented Mar 9, 2016

Refer to xbmc/xbmc#9290 and xbmc/xbmc#9295 for details.

@FernetMenta demuxer changes good to go?
@Jalle19 good to go?

@ksooo
Copy link
Member Author

ksooo commented Mar 9, 2016

@MilhouseVH could you do a runtime test?

@FernetMenta
Copy link
Contributor

@ksooo demuxer changes look ok but runtime test is more important in this case.

@Jalle19 Jalle19 mentioned this pull request Mar 9, 2016
@ksooo
Copy link
Member Author

ksooo commented Mar 9, 2016

Hmm. Sounds like a general problem, not only pvr.hts related. Whatever you guys say makes sense.

I only would like to avoid another API bump right now. Regardless of the version number it's always PITA and I really would like somebody else to do the next monkey work, then.

@ksooo
Copy link
Member Author

ksooo commented Mar 9, 2016

Wouldn't proper memset before reassigning stream data in ParseSubsriptionStartsolve the problem @Jalle19 mentioned?

@Jalle19
Copy link
Contributor

Jalle19 commented Mar 9, 2016

Yes a memset() would fix work, I just don't like polluting the code with C-isms just because we have a C API.

@ksooo
Copy link
Member Author

ksooo commented Mar 9, 2016

Yes a memset() would fix work, I just don't like polluting the code with C-isms just because we have a C API.

Fair enough. For now I would like to go with the memset approach. Okay?

@MilhouseVH
Copy link

@MilhouseVH could you do a runtime test?

To be honest I don't personally use any PVR functionality so runtime testing is difficult. I can build test it, that's about it - I leave runtime testing to the test users. :)

@ksooo
Copy link
Member Author

ksooo commented Mar 9, 2016

... now with memset.

@ksooo
Copy link
Member Author

ksooo commented Mar 10, 2016

Runtime tested. Works to some extend. One can watch tv, but on some channels audio stops after 3-5 seconds. Don't think this is an add-on problem.

@Jalle19 I suggest to merge this PR as it is now, so we have at least a compatible add-on again. Okay?

@ksooo
Copy link
Member Author

ksooo commented Mar 10, 2016

I just don't like polluting the code with C-isms just because we have a C API.

Lets fix this with the next API bump, which I'm pretty sure will come up before Krypton release anyway.

@Jalle19 okay to merge?

@Jalle19
Copy link
Contributor

Jalle19 commented Mar 11, 2016

Yeah go ahead, no point in having a broken addon.

ksooo added a commit that referenced this pull request Mar 11, 2016
@ksooo ksooo merged commit 1ec8554 into kodi-pvr:master Mar 11, 2016
@ksooo ksooo deleted the pvr-api-5-1-0 branch August 23, 2017 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants