Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

RFC: [pvr] Fix problems with multiple PVR Addons running at the same time #1472

Merged
merged 4 commits into from Sep 30, 2012

Conversation

Projects
None yet
6 participants
Member

fetzerch commented Sep 24, 2012

Currently, running different PVR addons simultaneously leads into unspecified behavior.
For example recordings are loaded from all plugins, but you can only play some.

The issue is described here: http://trac.xbmc.org/ticket/13353
In short: The shared libraries that are loaded by the addons (libXBMC_addon, libXBMC_pvr, libXBMC_gui) use global variables (m_Handle, m_cb) that are overriden each time a plugin loads.

I started to work on one possible solution: Getting rid of those global variables, and passing them as function parameter in all exported functions.
The API that the addons call, doesn't not change because I store the handles as class members.

@opdenkamp: I'd like to get some feedback on this. Is this the proper approach or any easier fix that I just fail to see?
You were using void* as handles in the .h file. Is there a specific reason or is it better to include the header / forward declare the handles?

Member

elupus commented Sep 24, 2012

imho globals must go.. That goes for the addons as well. That will break
api rather badly though. But it's really a necessary change.

I didn't review this diff though.

Member

opdenkamp commented Sep 24, 2012

I don't have time to look into this until later this week, sorry. Not at home now. Blame @malard ;)

Joakim Plate notifications@github.com wrote:

imho globals must go.. That goes for the addons as well. That will break
api rather badly though. But it's really a necessary change.

I didn't review this diff though.


Reply to this email directly or view it on GitHub:
#1472 (comment)

Member

topfs2 commented Sep 25, 2012

Looks good to me, just reviewed it very quickly though.

I would suggest removing (or seperate) the vxcproj changes however.

Member

opdenkamp commented Sep 25, 2012

had a quick look at it and looking good. i'll need to go through this in more detail though as soon as i got time

Member

fetzerch commented Sep 25, 2012

Thanks so far! I've splitted the vxcproj changes and corrected the return type of XBMC_get_file_chunk_size (according to CFile and CAddonCallbacksAddon it should be int instead of int64_t)

Member

fetzerch commented Sep 28, 2012

@opdenkamp I'd like to add a few more API functions that are needed for the mythtv addon (FileExists, FileDelete, FileStat, CreateDirectory, DirectoryExists and DirectoryDelete). Is it okay to add them here as a separate commit? Cause they also depend on this rework here.

@ghost ghost assigned opdenkamp Sep 28, 2012

Member

opdenkamp commented Sep 28, 2012

@fetzerch go for it.

won't be around until sunday evening, so if someone else could verify this and merge in if ok before tagging the alpha, than that would be great.

[addons] Added FileExists, StatFile, DeleteFile, CreateDirectory, Dir…
…ectoryExists and RemoveDirectory to libXBMCaddon
Owner

Memphiz commented Sep 30, 2012

Reviewed the code change (which looks like search&replace most of the time) and compile/run tested on osx. Though since i can't estimate the influences because i'm not into that binary addon handling - someone else has to push the button.

Owner

MartijnKaijser commented Sep 30, 2012

compiled on win32 without problems. haven't tested functionality

opdenkamp pushed a commit that referenced this pull request Sep 30, 2012

Merge pull request #1472 from 'fetzerch/bugfix-multiple-addons'.
[pvr] Fix problems with multiple PVR Addons running at the same time

@opdenkamp opdenkamp merged commit 20bd8cd into xbmc:master Sep 30, 2012

Member

opdenkamp commented Sep 30, 2012

needed an interface version bump. added that before pushing. pvr add-on repos has been updated with this too.

LongChair added a commit to plexinc/plex-home-theater-public that referenced this pull request Feb 5, 2015

Go back one level up if MediaWindow update failed, fixes #1472
This allows to get out of the section if anything goes wrong with the update.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment