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

Move screenshot code into it's own file. Move surface capturing code in... #1511

Merged
merged 1 commit into from Oct 4, 2012

Conversation

Projects
None yet
2 participants
Contributor

hydra commented Sep 30, 2012

...to it's own class so it can be reused. Add message to Util.cpp as it appears to be a big dumping ground for disparate code.

Re-created pull-request. Previous one was from the wrong branch.

Member

jmarshallnz commented Sep 30, 2012

Looks good. Needs build support on Linux (simply add to the Makefile) and OSX (trickier - one of the devs here will take care of it once it goes in).

Contributor

hydra commented Sep 30, 2012

Can you do the makefile as part of the merge; I don't have a linux build environment setup here yet. I'll be adding AmbiPi support (which was the driver for this refactor) to OSX and Linux so will setup OSX and Linux build environments in due couse but not ready yet.

ok?

@jmarshallnz jmarshallnz commented on an outdated diff Oct 2, 2012

xbmc/Screenshot.h
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This Program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with XBMC; see the file COPYING. If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ */
+
+class CScreenshotSurface {
+
@jmarshallnz

jmarshallnz Oct 2, 2012

Member

Please put the brackets on separate lines. Also, you need at least #include "utils/StdString.h" here.

@jmarshallnz jmarshallnz commented on an outdated diff Oct 2, 2012

xbmc/Screenshot.h
+ * You should have received a copy of the GNU General Public License
+ * along with XBMC; see the file COPYING. If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ */
+
+class CScreenshotSurface {
+
+public:
+ int m_width;
+ int m_height;
+ int m_stride;
+ unsigned char* m_buffer;
+
+ CScreenshotSurface(void);
+ boolean capture( void );
@jmarshallnz

jmarshallnz Oct 2, 2012

Member

This should be bool, not boolean

@jmarshallnz jmarshallnz commented on an outdated diff Oct 2, 2012

xbmc/Screenshot.cpp
+#include "utils/JobManager.h"
+#include "utils/URIUtils.h"
+#include "utils/log.h"
+
+using namespace std;
+using namespace XFILE;
+
+CScreenshotSurface::CScreenshotSurface()
+{
+ m_width = 0;
+ m_height = 0;
+ m_stride = 0;
+ m_buffer = NULL;
+}
+
+boolean CScreenshotSurface::capture()
@jmarshallnz

jmarshallnz Oct 2, 2012

Member

bool, not boolean

Member

jmarshallnz commented Oct 2, 2012

Once those last ones are fixed, if you squash down the fixes, I'll merge and fix up the OSX and Makefile builds.

@ghost ghost assigned jmarshallnz Oct 2, 2012

Contributor

hydra commented Oct 2, 2012

Addressed two comments from yesterday and squashed commits (you did mean 'git reset'..., 'git commit', 'git push origin relocate-screenshot-code2 --force' right?)

I wanted to keep my first pull request simple so I could figure out the process and start communication. I'm sure i'll be making additional pull requests in the future to help out when I get time.

Thanks again!

Contributor

hydra commented Oct 2, 2012

Also, once this is done let me know so I can make a pull request for this branch:

https://github.com/hydra/xbmc/commits/refactor-screenshot-code1

Member

jmarshallnz commented Oct 2, 2012

There's two commits here still. You need to squash them together down into one commit (via an interactive rebase or similar) then force push back so there's a single commit.

Contributor

hydra commented Oct 2, 2012

Sure, i'll do that tomorrow.

Contributor

hydra commented Oct 4, 2012

Done

Contributor

hydra commented Oct 4, 2012

Also added missing #prama once

Member

jmarshallnz commented Oct 4, 2012

Looks good - thanks!

@jmarshallnz jmarshallnz added a commit that referenced this pull request Oct 4, 2012

@jmarshallnz jmarshallnz Merge pull request #1511 from hydra/relocate-screenshot-code2
Move screenshot code into it's own file.  Move surface capturing code in...
00545ea

@jmarshallnz jmarshallnz merged commit 00545ea into xbmc:master Oct 4, 2012

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