OpenCV 2.4.7 porting to PNaCl

OpenCV 2.4.7 porting to PNaCl

Patches and cmake toolchain file for compilation with PNaCl clang.

Started off from box2d, file "diff"s come from it, if any.

BUG=<not yet>

[mcasas, 2013-11-19 10:09:40]

Hi sbc@, I wonder if you could take a coarse first pass, or redirect to
another reviewer as needed? Thanks!

Context: for some chromium WebRTC experiments, I was planning to use
OpenCV and nothing better than to port it to PNaCl ! Once that's done I think
is best to upstream it for the wider community since there are scattered 
efforts (groups q's etc) for this. This is my first commit of this kind, please
be very vigilant! :)

OpenCV porting itself was relatively easy, with some minor patches to files
themselves.

[Sam Clegg, 2013-11-19 17:03:26]

Great!  Thanks for the contribution.

Can you add this new port to the top level makefile?

Once this is done can you make sure all the configurations
build by running "./make_all.sh opencv".

Also, I think the folder name should probably be all
lowercase given that the upstream repo/folder is
lowercase.

https://codereview.chromium.org/76153002/diff/50001/ports/OpenCV/nacl.patch
File ports/OpenCV/nacl.patch (right):

https://codereview.chromium.org/76153002/diff/50001/ports/OpenCV/nacl.patch#n...
ports/OpenCV/nacl.patch:42: -            }
Can you instead wrap these chunks in #ifndef __native_client__ and includes a
comment about why they were removed.  This makes the patch
more useful for upstream and future maintainers.

Same for all the other changes in this patch.

https://codereview.chromium.org/76153002/diff/50001/ports/OpenCV/pkg_info
File ports/OpenCV/pkg_info (right):

https://codereview.chromium.org/76153002/diff/50001/ports/OpenCV/pkg_info#new...
ports/OpenCV/pkg_info:2:
URL=https://github.com/Itseez/opencv/archive/2.4.7.tar.gz
I think this will end up downloading a file called '2.4.7.tar.gz'.

I think you can use "/archive/2.4.7/opencv-2.4.7.tar.gz" to get the filename you
want.

I will upload this file to our mirror on commondatastorage so we don't hammer
github (the build scripts will look here first by default).

[Sam Clegg, 2013-11-19 17:54:59]

I've added opencv-2.4.7.tar.gz to the mirror on commondatastorage.

https://codereview.chromium.org/76153002/diff/50001/ports/OpenCV/OpenCV_v2.4....
File ports/OpenCV/OpenCV_v2.4.7.sha1 (right):

https://codereview.chromium.org/76153002/diff/50001/ports/OpenCV/OpenCV_v2.4....
ports/OpenCV/OpenCV_v2.4.7.sha1:1: 67ec788943894d4be3dff6efd7720591108feb29 
*opencv-2.4.7.tar.gz
The name of this sha1 file should be opencv-2.4.7.sha1 to match the archive
filename.

Once I changed that, and change the URL in pkg_info
I was able to build this port.

However the TestStep in build.sh doesn't run as it
is looking for a binary called HelloWorld.

[mcasas, 2013-11-19 21:10:26]

Hi sbc@ BIG thanks for the review and your time, as you possibly noticed
I hadn't used the naclports make_all.sh, which must have cause you 
trouble compiling (silly naming etc). Apologies!

- Removed the TestStep in build.sh, I copy-kept it because I thought it
was part of the infra.
- In build.sh, I overrode BuildStep because opencv does not compile with 
make -jX 
- Rewrote the patch with your indications.
- I didn't know that both newlib and glibc  are used sequentially. I made 
a patch for both (is just one line that's different). I made my builds in
 Mac+pepper 31; but was mostly interested in PNaCl.
- Apologies for the silly naming mistake in .sha1 , must have wasted your
time quite a bit...
- Added MIRROR_URL to pkg_info.

[Sam Clegg, 2013-11-19 21:13:38]

lgtm

https://codereview.chromium.org/76153002/diff/170001/Makefile
File Makefile (right):

https://codereview.chromium.org/76153002/diff/170001/Makefile#newcode422
Makefile:422: # as modeline string.
Can you add an alias for opencv here?

https://codereview.chromium.org/76153002/diff/170001/ports/opencv/pkg_info
File ports/opencv/pkg_info (right):

https://codereview.chromium.org/76153002/diff/170001/ports/opencv/pkg_info#ne...
ports/opencv/pkg_info:3:
MIRROR_URL=http://commondatastorage.googleapis.com/nativeclient-mirror/nacl/opencv-2.4.7.tar.gz
You don't need MIRROR_URL here.. it should be implicit.

[Sam Clegg, 2013-11-19 21:14:51]

On 2013/11/19 21:13:38, Sam Clegg wrote:
> lgtm
> 
> https://codereview.chromium.org/76153002/diff/170001/Makefile
> File Makefile (right):
> 
> https://codereview.chromium.org/76153002/diff/170001/Makefile#newcode422
> Makefile:422: # as modeline string.
> Can you add an alias for opencv here?
> 
> https://codereview.chromium.org/76153002/diff/170001/ports/opencv/pkg_info
> File ports/opencv/pkg_info (right):
> 
>
https://codereview.chromium.org/76153002/diff/170001/ports/opencv/pkg_info#ne...
> ports/opencv/pkg_info:3:
>
MIRROR_URL=http://commondatastorage.googleapis.com/nativeclient-mirror/nacl/opencv-2.4.7.tar.gz
> You don't need MIRROR_URL here.. it should be implicit.

Would you like to be added to the contributors for naclports so that you can can
commit this?

[mcasasG, 2013-11-19 21:34:00]

Addition -> yes, please add me there :)

I'll commit as soon as I take in you changes and add another line to the
patch (that I forgot, and only shows with a certain toolchain).


On Tue, Nov 19, 2013 at 10:14 PM, <sbc@chromium.org> wrote:

> On 2013/11/19 21:13:38, Sam Clegg wrote:
>
>> lgtm
>>
>
>  https://codereview.chromium.org/76153002/diff/170001/Makefile
>> File Makefile (right):
>>
>
>  https://codereview.chromium.org/76153002/diff/170001/Makefile#newcode422
>> Makefile:422: # as modeline string.
>> Can you add an alias for opencv here?
>>
>
>  https://codereview.chromium.org/76153002/diff/170001/
>> ports/opencv/pkg_info
>> File ports/opencv/pkg_info (right):
>>
>
>
> https://codereview.chromium.org/76153002/diff/170001/
> ports/opencv/pkg_info#newcode3
>
>> ports/opencv/pkg_info:3:
>>
>
> MIRROR_URL=http://commondatastorage.googleapis.
> com/nativeclient-mirror/nacl/opencv-2.4.7.tar.gz
>
>> You don't need MIRROR_URL here.. it should be implicit.
>>
>
> Would you like to be added to the contributors for naclports so that you
> can can
> commit this?
>
> https://codereview.chromium.org/76153002/
>



-- 

Miguel Casas-Sanchez | Software Engineer | mcasas@google.com | +46 (0)70
322 0306

-- 
You received this message because you are subscribed to the Google Groups
"Native-Client-Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to native-client-reviews+unsubscribe@googlegroups.com.
To post to this group, send email to native-client-reviews@googlegroups.com.
Visit this group at http://groups.google.com/group/native-client-reviews.
For more options, visit https://groups.google.com/groups/opt_out.

[Sam Clegg, 2013-11-21 02:14:17]

Any idea why this issue didn't get closed when you
committed?

[Sam Clegg, 2014-03-19 16:23:39]

On 2013/11/21 02:14:17, Sam Clegg wrote:
> Any idea why this issue didn't get closed when you
> committed?

Committed as r993.