ilyakurdyukov / jpeg-quantsmooth

JPEG artifacts removal based on quantization coefficients.
GNU Lesser General Public License v2.1
254 stars 21 forks source link

macOS Support? #21

Closed JanJastrow closed 2 years ago

JanJastrow commented 2 years ago

Hi there,

is there any chance you can make this build under macOS? I was able to build it on my linux server just fine, but I would love to have this working on macOS too.

Anyway - thanks for this cool project!

ilyakurdyukov commented 2 years ago

I don't have any macOS devices.

I can improve the Makefile if you tell me what to add to it for macOS support. Probably something about file extensions.

ilyakurdyukov commented 2 years ago

Here's instructions of how to install libjpeg for sources using it on macOS. (You don't need libpng and ImageMagick.)

You can try it and then run make and see what happens.

JanJastrow commented 2 years ago

When I try to run make this happens:

cc -Wall -O2  -march=native -Wextra -pedantic -fopenmp  -DAPPNAME=jpegqs -o jpegqs quantsmooth.c -ljpeg  -Wl,--gc-sections -s -lm
clang: error: unsupported option '-fopenmp'
clang: error: unsupported option '-fopenmp'
make: *** [jpegqs] Error 1

(this is even before adding libjpeg)

zvezdochiot commented 2 years ago

Hi @JanJastrow .

https://github.com/ImageProcessing-ElectronicPublications/libjpegqs

I remove the -fopenmp from CFLAGS the Makefile and domake on Linux. The result is no different. Can you repeat it on macOS?

JanJastrow commented 2 years ago

Hi @zvezdochiot I can confirm - when I clone your fork (?) and remove the -fopenmpcflag I have some minor warnings, but it builds :)

uname: illegal option -- -
usage: uname [-amnprsv]
cc -Wall -O2 -DAPPNAME=jpegqs   -c -o src/idct.o src/idct.c
cc -Wall -O2 -DAPPNAME=jpegqs   -c -o src/libjpegqs.o src/libjpegqs.c
src/libjpegqs.c:24:24: warning: unused variable 'p1' [-Wunused-variable]
    int i, l0, l1, p0, p1;
                       ^
src/libjpegqs.c:24:20: warning: unused variable 'p0' [-Wunused-variable]
    int i, l0, l1, p0, p1;
                   ^
2 warnings generated.
ar rcs libjpegqs.a src/idct.o src/libjpegqs.o
cc -Wall -O2 -DAPPNAME=jpegqs -shared src/idct.o src/libjpegqs.o -o libjpegqs.so.0 -ljpeg -lm -s
ld: warning: option -s is obsolete and being ignored
cc -Wall -O2 -DAPPNAME=jpegqs   -c -o src/jpegqs.o src/jpegqs.c
cc -Wall -O2 -DAPPNAME=jpegqs src/jpegqs.o libjpegqs.so.0 -o jpegqs -ljpeg -lm -s
ld: warning: option -s is obsolete and being ignored
cc -Wall -O2 -DAPPNAME=jpegqs src/jpegqs.c libjpegqs.a -o jpegqs-static -ljpeg -lm -s
ld: warning: option -s is obsolete and being ignored

Thank you very much.

zvezdochiot commented 2 years ago

Hi @JanJastrow .

Thanks for the answer. I will take note.

No. This is not a fork. This is a library implementation this code from which most of the hardware optimizations have been removed.

We are waiting for the test results.

ilyakurdyukov commented 2 years ago

@JanJastrow, it's best not to use the degraded version from @zvezdochiot as it will run about ten times or even slower.

OpenMP support should have been in Clang (the default compiler on macOS) for years.

make MTOPTS= should build it without OpenMP but with SIMD optimizations.

zvezdochiot commented 2 years ago

Hi @ilyakurdyukov .

Yes. My goal was not speed, but library (embeddability). A bench is required. It's on your conscience.

JanJastrow commented 2 years ago

Running make MTOPTS= gives the following error:

cc -Wall -O2  -march=native -Wextra -pedantic   -DAPPNAME=jpegqs -o jpegqs quantsmooth.c -ljpeg  -Wl,--gc-sections -s -lm
ld: warning: option -s is obsolete and being ignored
ld: unknown option: --gc-sections
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [jpegqs] Error 1

Where should a add -v in the makefile to have a more verbose error message?

zvezdochiot commented 2 years ago

Hi @JanJastrow .

About libjpegqs. Prompt the output of the uname --kernel-name and uname --operating-system commands on your system.

PS: The absence of -fopenmp makes the library single-threaded. The execution time is increased by *(number of cores).

https://github.com/ImageProcessing-ElectronicPublications/libjpegqs/releases/tag/0.20211223

JanJastrow commented 2 years ago

I'm sorry I never mentioned that I'm not testing on the latest macOS release. All the above have been run on a Hackintosh running macOS 10.15.7 (a 2 year old release). But I also have an original MacBook here with the latest release, so here is some additional info:

libjpegqs

I have pulled the latest changes on both machines. Just running make fails:

macOS 10.15:

uname: illegal option -- -
usage: uname [-amnprsv]
cc -Wall -O2 -fopenmp -DAPPNAME=jpegqs -fPIC   -c -o src/libjpegqs.o src/libjpegqs.c
clang: error: unsupported option '-fopenmp'
make: *** [src/libjpegqs.o] Error 1

macOS 12.1:

uname: illegal option -- -
usage: uname [-amnprsv]
cc -Wall -O2 -fopenmp -DAPPNAME=jpegqs -fPIC   -c -o src/idct.o src/idct.c
clang: error: unsupported option '-fopenmp'
make: *** [src/idct.o] Error 1

running make MPFLAGS= builds on both machines but with different warnings:

macOS 10.15:

uname: illegal option -- -
usage: uname [-amnprsv]
cc -Wall -O2  -DAPPNAME=jpegqs -fPIC   -c -o src/libjpegqs.o src/libjpegqs.c
ar rcs libjpegqs.a src/idct.o src/libjpegqs.o
cc -Wall -O2  -DAPPNAME=jpegqs -fPIC -shared src/idct.o src/libjpegqs.o -o libjpegqs.so.0 -ljpeg -lm -s
ld: warning: option -s is obsolete and being ignored
cc -Wall -O2  -DAPPNAME=jpegqs -fPIC src/jpegqs.o libjpegqs.so.0 -o jpegqs -ljpeg -lm -s
ld: warning: option -s is obsolete and being ignored
cc -Wall -O2  -DAPPNAME=jpegqs -fPIC src/jpegqs.c libjpegqs.a -o jpegqs-static -ljpeg -lm -s
ld: warning: option -s is obsolete and being ignored

macOS 12.1

uname: illegal option -- -
usage: uname [-amnprsv]
cc -Wall -O2  -DAPPNAME=jpegqs -fPIC   -c -o src/idct.o src/idct.c
cc -Wall -O2  -DAPPNAME=jpegqs -fPIC   -c -o src/libjpegqs.o src/libjpegqs.c
ar rcs libjpegqs.a src/idct.o src/libjpegqs.o
cc -Wall -O2  -DAPPNAME=jpegqs -fPIC -shared src/idct.o src/libjpegqs.o -o libjpegqs.so.0 -ljpeg -lm -s
ld: warning: option -s is obsolete and being ignored
cc -Wall -O2  -DAPPNAME=jpegqs -fPIC   -c -o src/jpegqs.o src/jpegqs.c
cc -Wall -O2  -DAPPNAME=jpegqs -fPIC src/jpegqs.o libjpegqs.so.0 -o jpegqs -ljpeg -lm -s
ld: warning: option -s is obsolete and being ignored
cc -Wall -O2  -DAPPNAME=jpegqs -fPIC src/jpegqs.c libjpegqs.a -o jpegqs-static -ljpeg -lm -s
ld: warning: option -s is obsolete and being ignored

uname

uname behaves differently on MacOS, so --kernel-name and --operating-system do not work. But here is some relevant output:

macOS 10.15:

uname -r (print the operating system release.)
19.6.0

uname -s (print the operating system name.)
Darwin

uname -v (print the operating system version.)
Darwin Kernel Version 19.6.0: Sun Nov 14 19:58:51 PST 2021; root:xnu-6153.141.50~1/RELEASE_X86_64

macOS 12.1:

uname -r (print the operating system release.)
21.2.0

uname -s (print the operating system name.)
Darwin

uname -v (print the operating system version.)
Darwin Kernel Version 21.2.0: Sun Nov 28 20:28:54 PST 2021; root:xnu-8019.61.5~1/RELEASE_X86_64

jpeg-quantsmooth

Just FYI - the main project also does not build on my original, up-to-date MacBook with the same error. So nothing different here.

zvezdochiot commented 2 years ago

Hi @JanJastrow .

Thanks for the detailed answer. I'll take note of the difference in the uname options. You have provided the required information with uname -s ==Darwin. Thank you. Does make MPFLAGS = work fine without editing the Makefile?

JanJastrow commented 2 years ago

Hi @zvezdochiot yes, it builds without manual changes to the Makefile.

zvezdochiot commented 2 years ago

Hi @JanJastrow .

Have you already tested the library on images?

To test a dynamic link library on Linux: LD_LIBRARY_PATH=. ./jpegqs you.jpg you.jqs.jpg.
To test a static link library on Linux: ./jpegqs-static you.jpg you.jqs.jpg.

JanJastrow commented 2 years ago

Yes, I can confirm that it works as expected. The results are the same as on the WASM tool.

(I just run ./jpegqs --optimize 1.jpg 2.jpg from the build-directory)

ilyakurdyukov commented 2 years ago

Running make MTOPTS= gives the following error:

Can you try this? make MTOPTS= LDFLAGS="-Wl,-dead_strip"

What is your compiler and what version is it? Should be printed by cc -v.

JanJastrow commented 2 years ago

Hi @ilyakurdyukov

Nice, now it builds without any warnings! Thank you very much for your support.

Here is what the output of cc -v:

Apple clang version 13.0.0 (clang-1300.0.29.30)
Target: x86_64-apple-darwin21.2.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
ilyakurdyukov commented 2 years ago

@JanJastrow, can you try changes from this branch?

https://github.com/ilyakurdyukov/jpeg-quantsmooth/tree/macos_makefile

Just run make without parameters, there should be a warning on the command line:

Makefile:47: Your compiler has no OpenMP support!

But it will compile anyway.

You can enable (you can read here and here) OpenMP in macOS if you want, but as I see it's a bit complicated.

JanJastrow commented 2 years ago

Hi, I can confirm that this branch builds fine by running make with the exact warning you said it will/should have.

I did some additional changes to the makefile (as suggested by this stackoverflow answer): (I have no idea how makefiles work and there is surely a more elegant way then this, but it worked)

Line53:

CFLAGS := -Wall -O2 -Xpreprocessor -fopenmp -lomp

it gave me some warning, but build otherwise (both on macOS 10.15 and 12.1)

Makefile:47: Your compiler has no OpenMP support!
cc -Wall -O2 -Xpreprocessor -fopenmp -lomp  -march=native -Wextra -pedantic   -DAPPNAME=jpegqs -o jpegqs quantsmooth.c -ljpeg  -Wl,-dead_strip -lm
In file included from quantsmooth.c:89:
In file included from ./quantsmooth.h:28:
/usr/local/include/omp.h:53:9: warning: ISO C restricts enumerator values to range of 'int' (2147483648 is too large) [-Wpedantic]
        omp_sched_monotonic = 0x80000000
        ^                     ~~~~~~~~~~
/usr/local/include/omp.h:406:7: warning: ISO C restricts enumerator values to range of 'int' (18446744073709551615 is too large)
      [-Wpedantic]
      KMP_ALLOCATOR_MAX_HANDLE = UINTPTR_MAX
      ^                          ~~~~~~~~~~~
/usr/local/include/omp.h:423:7: warning: ISO C restricts enumerator values to range of 'int' (18446744073709551615 is too large)
      [-Wpedantic]
      KMP_MEMSPACE_MAX_HANDLE = UINTPTR_MAX
      ^                         ~~~~~~~~~~~
/usr/local/include/omp.h:458:39: warning: ISO C restricts enumerator values to range of 'int' (18446744073709551615 is too large)
      [-Wpedantic]
    typedef enum omp_event_handle_t { KMP_EVENT_MAX_HANDLE = UINTPTR_MAX } omp_event_handle_t;
                                      ^                      ~~~~~~~~~~~
4 warnings generated.

the new binary was 85kb (on macOS 10.15) and 67kb (on macOS 12.1) instead of 66kb (on both machines) and in a quick benchmark also run faster (hence multicore):

$ time ./jpegqs --optimize test.jpg test1.jpg
[…]
quantsmooth: 109.228ms
./jpegqs --optimize test.jpg test1.jpg  1.83s user 0.07s system 931% cpu 0.204 total

$ time ./jpegqs_without --optimize test.jpg test2.jpg
[…]
quantsmooth: 747.682ms
./jpegqs_without --optimize test.jpg test2.jpg  0.81s user 0.02s system 99% cpu 0.830 total

(jpegqs_without is the "orignal" makefile)

zvezdochiot commented 2 years ago

Hi all.

Maybe add in Makefile?:

LDFLAGS = -ljpeg -lm
ifeq ($(shell uname -s), Darwin)
CFLAGS += -Xpreprocessor
LDFLAGS += -lomp
endif
ilyakurdyukov commented 2 years ago

@JanJastrow, I have updated the branch, can you try once more?

https://github.com/ilyakurdyukov/jpeg-quantsmooth/tree/macos_makefile

@zvezdochiot, -Xpreprocessor is paired with the next option.

-Xpreprocessor option

    Pass option as an option to the preprocessor. You can use this to supply system-specific
 preprocessor options that GCC does not recognize.

    If you want to pass an option that takes an argument, you must use -Xpreprocessor twice,
 once for the option and once for the argument.
zvezdochiot commented 2 years ago

Hi @JanJastrow .

Maybe test?:

make MPFLAGS="-Xpreprocessor -fopenmp -lomp"

https://github.com/ImageProcessing-ElectronicPublications/libjpegqs

JanJastrow commented 2 years ago

@ilyakurdyukov: I just ran the latest commit - builds just fine (with the same warnings I mentioned before).


@zvezdochiot: also builds, but still with the uname warning:

uname: illegal option -- -
usage: uname [-amnprsv]
cc -Wall -O2 -Xpreprocessor -fopenmp -lomp -DAPPNAME=jpegqs -fPIC   -c -o src/idct.o src/idct.c
clang: warning: -lomp: 'linker' input unused [-Wunused-command-line-argument]
cc -Wall -O2 -Xpreprocessor -fopenmp -lomp -DAPPNAME=jpegqs -fPIC   -c -o src/libjpegqs.o src/libjpegqs.c
clang: warning: -lomp: 'linker' input unused [-Wunused-command-line-argument]
ar rcs libjpegqs.a src/idct.o src/libjpegqs.o
cc -Wall -O2 -Xpreprocessor -fopenmp -lomp -DAPPNAME=jpegqs -fPIC -shared src/idct.o src/libjpegqs.o -o libjpegqs.so.0 -ljpeg -lm -s
ld: warning: option -s is obsolete and being ignored
cc -Wall -O2 -Xpreprocessor -fopenmp -lomp -DAPPNAME=jpegqs -fPIC   -c -o src/jpegqs.o src/jpegqs.c
clang: warning: -lomp: 'linker' input unused [-Wunused-command-line-argument]
cc -Wall -O2 -Xpreprocessor -fopenmp -lomp -DAPPNAME=jpegqs -fPIC src/jpegqs.o libjpegqs.so.0 -o jpegqs -ljpeg -lm -s
ld: warning: option -s is obsolete and being ignored
cc -Wall -O2 -Xpreprocessor -fopenmp -lomp -DAPPNAME=jpegqs -fPIC src/jpegqs.c libjpegqs.a -o jpegqs-static -ljpeg -lm -s
ld: warning: option -s is obsolete and being ignored

I ran both binaries and they were both running as expected with multithreading

ilyakurdyukov commented 2 years ago

I just ran the latest commit - builds just fine (with the same warnings I mentioned before).

So I committed the same for the main branch. These warnings come from the "omp.h" header because of the "-pedantic" compiler option. It seems like a problem with macOS/Clang, I'll leave it as it is. No such thing with GCC or Clang on Linux.

Also mentioned here: https://lists.llvm.org/pipermail/openmp-dev/2020-June/003494.html

JanJastrow commented 2 years ago

Perfect. Thanks for your support :)

ilyakurdyukov commented 2 years ago

@zvezdochiot: don't use long options to uname, use "-m" instead of "--machine", "-lomp" should not be in compiler flags, it should be in linker flags.

zvezdochiot commented 2 years ago

Hi @ilyakurdyukov .

Да знаю я, что это не очень "красиво". Просто городить огород совсем не хочется. Действует же. В РИДМИ добавлю и хорош.

https://github.com/ImageProcessing-ElectronicPublications/libjpegqs/releases/tag/0.20211225

JanJastrow commented 2 years ago

Hi again, I just wanted to give an update to this. I got myself an Apple Silicon Mac (MacBook Air M1) two days ago and wanted to compile this tool again.

I ran into some problems compiling, but I think they are all just depending on my enviroment (installed libraries are not found). Still, with some tinkering (and compiling jpeg-9e myself) I was able to build the project sucessfully. (make JPEGSRC=jpeg-9e MTOPTS= LDFLAGS="-Wl,-dead_strip,-L/opt/homebrew/opt/llvm/lib")


Here are my benchmarks, just for fun:

$time ./jpegqs_single 1.jpg single.jpg
[…]
quantsmooth: 482.526ms
./jpegqs_single 1.jpg single.jpg  0.52s user 0.01s system 14% cpu 3.575 total

$time ./jpegqs_openmp 1.jpg openmp.jpg
[…]
quantsmooth: 471.358ms
./jpegqs_openmp 1.jpg openmp.jpg  0.49s user 0.01s system 86% cpu 0.583 total

So, if you have anything else to test, let me know :)

zvezdochiot commented 2 years ago

Hi @JanJastrow .

Test more #22 .