-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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 system improvements and video reading fix for bin/tld.c #38
base: unstable
Are you sure you want to change the base?
Conversation
The undeclared constants are defined in zlib.h. Previously, png.h included zlib.h, but recent versions of libpng (version >= 1.5) do not include zlib.h anymore. This commit will solve the following errors: lib/io/_ccv_io_libpng.c: In function ‘_ccv_write_png_fd’: lib/io/_ccv_io_libpng.c:55:23: error: ‘MAX_MEM_LEVEL’ undeclared (first use in this function) lib/io/_ccv_io_libpng.c:55:23: note: each undeclared identifier is reported only once for each function it appears in lib/io/_ccv_io_libpng.c:63:38: error: ‘Z_BEST_SPEED’ undeclared (first use in this function) lib/io/_ccv_io_libpng.c:65:40: error: ‘Z_HUFFMAN_ONLY’ undeclared (first use in this function)
This commit makes it also possible to build all binaries and the ccv library in parallel using e.g. make -j4.
This looks good, I will do a manual merge tonight. BTW, do you know if this builds runs with clang's static analyzer now? I am trying to get it running on the old build sys, but has no luck. |
Please include these two commits as well: eb13376 and 33e1c65. Without the first patch, the build system does not look on the right location for the libccv.a file. The second commit is just some code cleanup. Is there a specific reason why you do not have -Wextra and -fPIC in CFLAGS? Using -fPIC allows us to create shared libraries, instead of compiling static builds. Currently, the size of each binary in bin/ is >2.0mb, which is rather large. I tried "scan-build make" but it says "scan-build: Removing directory '/tmp/scan-build-2012-11-13-1' because it contains no reports.". Did you have a similar error? It tried it with clang 2.9. If you have any questions about the new build system, feel free to ask of course. |
I don't think that this diff does the right thing. It invokes cc (alias to gcc) to compile, rather than the designated clang compiler, there are redundant rules that I cannot figure out why is not called. The ./configure shell-script doesn't read old config.mk and skip properly. Trying to fix these issues now. |
Need to spend more time to improve my GNU make skill, before that, there are a few issues in this diff makes me uncomfortable to merge yet (the main makefile's %.o: %.c rule never get called, a few %.o: %c rules in suffix.mk is executed, no libccv.a object get emitted, maybe a make version problem?: GNU Make 3.81) Also, the ffmpeg part is tricky too, it cannot now execute on my mpg file. Note that my tld.c is known to have memory issues and such, I need to fix these, meanwhile, the tld implementation itself is separately tested with valgrind to make sure no memory leak in the implementation. Thanks for the diff, I just need more time to make sure everything I understands ;-( a night is just not enough. |
also, to answer the question about why not a shared object, because ccv uses some global data, and it would be troublesome to make it a shared object. Personally, for any serious project, ccv should be just included in the project and use link-time-optimization to remove all these unused code if you care about binary size. |
I fixed the issue with libccv.a and integrated test/ in the new central build system. Both "make test" as well as "make" work as expected. I also ran make through clang's static analyser (use "scan-build make STATIC_ANALYZER=1") and it produced an error report. Make sure that you first remove /config.mk and generate it again using "./configure". Otherwise, config.mk will not have the proper content. There is a null pointer dereference issue in lib/ccv_swt.c on line 730: 730: for (i = 0; i < all_words->rnum; i++) where all_words is null. There are a couple of checks above involving scale_invariant and when scale_invariant is false, all_words is set. However, when scale_invariant is true, all_words remains null and that will cause a null pointer dereference on line 730. See clang's static analyser report (use "scan-view /tmp/scan-build-2012-11-14-1" or a similar path) for more details. |
thanks for the pointer, I am running trunk version of scan-build with clang now, quite a few warnings, no surprisingly. |
This build system will enable parallelized builds and tracks changes in header files properly. Changing an header file will result in a rebuild of the library and/or binaries.
The first commit fixes the video reading part of bin/tld.c. Due to incorrect usage of the libav API, it was not possible for me to use the demo. Symptoms were complaining about incorrect nalsizes and not flipping "got_picture" to true. The first commit should resolve that issue.