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

Build Shared Lib #1049

Merged
merged 14 commits into from Jun 7, 2012

Conversation

Projects
None yet
5 participants
@theuni
Member

theuni commented Jun 6, 2012

This is a continuation of #890

This PR is purely foundational. No new behavior is introduced here, except for the ability to build a libxbmc.so in linux/osx, but the lib isn't good for much in its current form.

I'm submitting this now because I have several things up that depend on it. Because it's a nightmare to rebase, it would be a big help to get this part in, and continue contributing to it after that.

There are 2 primary things going on here:

  1. Building as a shared lib. Gives us the ability to do unit testing with all of xbmc's features at our disposal. Also allows for stand-alone programs to use our functionality, eg. scrapers. Eventually we may want to come up with an API that allows for individual services to be started up.
  2. Refactor startup to allow us to run without a window. When combined with #1, this could lead to headless, lightweight xbmc instances. There is still lots of work that remains here. Many functions still rely on a window to exist, so it's quite crash-heavy. As a result, I didn't hook up any way to run this way.

@davilla This likely needs some osx love. For OSX I made an attempt at the link-line, but I'm unfamiliar with the syntax there.

@wiso For win32 I didn't attempt any of the shared lib stuff, only fixed up the startup sequence to match current behavior. I have no way to test, but I think it should be ok.

@jmarshallnz

This comment has been minimized.

Show comment
Hide comment
@jmarshallnz

jmarshallnz Jun 6, 2012

Member

Looks good to me on the whole. I'll check if win32 builds + runs.

Member

jmarshallnz commented Jun 6, 2012

Looks good to me on the whole. I'll check if win32 builds + runs.

@ghost ghost assigned theuni Jun 6, 2012

@davilla

This comment has been minimized.

Show comment
Hide comment
@davilla

davilla Jun 7, 2012

Contributor

osx builds/runs with a simple addition of main.cpp, I've made a patch and passed it on to TheUni. IOS/ATV2 builds should be fine, they don't use SDL at all and don't even compile/link in xbmc.cpp.

OSX/IOS sign off.

Contributor

davilla commented Jun 7, 2012

osx builds/runs with a simple addition of main.cpp, I've made a patch and passed it on to TheUni. IOS/ATV2 builds should be fine, they don't use SDL at all and don't even compile/link in xbmc.cpp.

OSX/IOS sign off.

@theuni

This comment has been minimized.

Show comment
Hide comment
@theuni

theuni Jun 7, 2012

Member

Thanks. Win32/OSX patches included. Squashed down and ready to go.

Member

theuni commented Jun 7, 2012

Thanks. Win32/OSX patches included. Squashed down and ready to go.

theuni added a commit that referenced this pull request Jun 7, 2012

@theuni theuni merged commit 66520ff into xbmc:master Jun 7, 2012

@Memphiz

This comment has been minimized.

Show comment
Hide comment
@Memphiz

Memphiz Jun 7, 2012

Member

IOS + ATV2 not needed?

Member

Memphiz commented on fd1f11a Jun 7, 2012

IOS + ATV2 not needed?

This comment has been minimized.

Show comment
Hide comment
@Memphiz

Memphiz Jun 7, 2012

Member

ahh nvm read the comment on the PR - nothing to see here.

Member

Memphiz replied Jun 7, 2012

ahh nvm read the comment on the PR - nothing to see here.

This comment has been minimized.

Show comment
Hide comment
@davilla

davilla Jun 7, 2012

Contributor

we might need a look/see here on ios. forum reports of black screen, sounds like something not setup right.

Contributor

davilla replied Jun 7, 2012

we might need a look/see here on ios. forum reports of black screen, sounds like something not setup right.

@Montellese

This comment has been minimized.

Show comment
Hide comment
@Montellese

Montellese Jun 9, 2012

Member

This causes a problem on win32 (haven't checked linux or osx) with multi-monitor support. The problem is that the GUI settings (specifically "videoscreen.screen") are created before calling CApplication::CreateGUI() which makes the call to InitWindowSystem which in turn calls the code to get the available monitor information on win32. Unfortunately we use g_Windowing.GetNumScreens() when we create the GUI setting "videoscreen.screen" but at that point it isn't properly initialized yet so g_Windowing.GetNumScreens() returns 0. This doesn't cause a problem for the first monitor because it has the index 0 but for any additional monitor with an index > 0 the GUI settings will just set "videoscreen.screen" to the maximum possible value which is 0.

Member

Montellese commented Jun 9, 2012

This causes a problem on win32 (haven't checked linux or osx) with multi-monitor support. The problem is that the GUI settings (specifically "videoscreen.screen") are created before calling CApplication::CreateGUI() which makes the call to InitWindowSystem which in turn calls the code to get the available monitor information on win32. Unfortunately we use g_Windowing.GetNumScreens() when we create the GUI setting "videoscreen.screen" but at that point it isn't properly initialized yet so g_Windowing.GetNumScreens() returns 0. This doesn't cause a problem for the first monitor because it has the index 0 but for any additional monitor with an index > 0 the GUI settings will just set "videoscreen.screen" to the maximum possible value which is 0.

@theuni

This comment has been minimized.

Show comment
Hide comment
@theuni

theuni Jun 9, 2012

Member

Thanks for the rundown, I'll have a look

Member

theuni commented Jun 9, 2012

Thanks for the rundown, I'll have a look

@theuni

This comment has been minimized.

Show comment
Hide comment
@theuni

theuni Jun 10, 2012

Member

From a quick grep, it appears as though that control is disabled and we're just using it to keep the current screen number. I don't see it being used as a ceiling anywhere. As a quick fix, it should be enough to change g_Windowing.GetNumScreens() to INT_MAX. It's a hack, but imo it's pretty hackish as it is now to be storing that kind of thing in a hidden control.

Member

theuni commented Jun 10, 2012

From a quick grep, it appears as though that control is disabled and we're just using it to keep the current screen number. I don't see it being used as a ceiling anywhere. As a quick fix, it should be enough to change g_Windowing.GetNumScreens() to INT_MAX. It's a hack, but imo it's pretty hackish as it is now to be storing that kind of thing in a hidden control.

@davilla

This comment has been minimized.

Show comment
Hide comment
@davilla

davilla Jun 10, 2012

Contributor

looks like numscreens also hits osx, there's a forum post regarding it

Contributor

davilla commented Jun 10, 2012

looks like numscreens also hits osx, there's a forum post regarding it

@Montellese

This comment has been minimized.

Show comment
Hide comment
@Montellese

Montellese Jun 10, 2012

Member

A hidden control? It's the control that is used in Settings -> System -> Video to switch between the different screens and windowed mode so it's anything but disabled. The ceiling is implemented in the control's internal logic. I can give INT_MAX a try.

@davilla Yeah it hits every platform as long as you have more than one monitor.

Member

Montellese commented Jun 10, 2012

A hidden control? It's the control that is used in Settings -> System -> Video to switch between the different screens and windowed mode so it's anything but disabled. The ceiling is implemented in the control's internal logic. I can give INT_MAX a try.

@davilla Yeah it hits every platform as long as you have more than one monitor.

@Montellese

This comment has been minimized.

Show comment
Hide comment
@Montellese

Montellese Jun 10, 2012

Member

ok INT_MAX seems to work but it's a very ugly hack IMO. We should consider that there might be similar effects in other places which just weren't uncovered yet.

Member

Montellese commented Jun 10, 2012

ok INT_MAX seems to work but it's a very ugly hack IMO. We should consider that there might be similar effects in other places which just weren't uncovered yet.

@theuni

This comment has been minimized.

Show comment
Hide comment
@theuni

theuni Jun 12, 2012

Member

Yea, had another look. Not sure what lead me to believe it was hidden. Though I still think that changing it to a big number is "safe", it's certainly no real solution.

I'll push a quck-fix tomorrow changing this to 32 or so so that nightly builds won't stay borked. Will come up with a proper fix and do a PR after that.

Member

theuni commented Jun 12, 2012

Yea, had another look. Not sure what lead me to believe it was hidden. Though I still think that changing it to a big number is "safe", it's certainly no real solution.

I'll push a quck-fix tomorrow changing this to 32 or so so that nightly builds won't stay borked. Will come up with a proper fix and do a PR after that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment