Add WebP support #233

llenck · 2021-06-14T07:27:26Z

Add support for encoding as WebP, for reasons like in #190. I figured since WebP can be lossy or lossless, it'd be good to do lossy for quality settings 0-9 and lossless for quality '10'.

I tested building/running this on arch linux with the PKGBUILD from your maim-git package with 'libwebp' added to the dependency list, and it seems to work, and produce smaller file sizes than png/jpg (and better quality compared to jpg).

Please let me know if there are any design choices or technical issues that prevent this from getting merged, so I can try to fix them ;)

foxpy · 2021-06-14T08:48:01Z

CMakeLists.txt

@@ -44,6 +45,9 @@ set_property(TARGET ${BIN_TARGET} PROPERTY CXX_STANDARD_REQUIRED ON)
 set_property(TARGET ${BIN_TARGET} PROPERTY CXX_STANDARD 11)
 set_property(TARGET ${BIN_TARGET} PROPERTY CXX_EXTENSIONS OFF)

+message(STATUS "webp include dir = ${WEBP_INCLUDE_DIR}")
+message(STATUS "webp lib = ${WEBP_LIBRARY}")


Are these debug prints really needed?

No, I forgot to take those out, sorry. Will do another commit for this.

foxpy · 2021-06-14T08:49:50Z

modules/FindWebP.cmake

+
+find_path( WEBP_INCLUDE_DIR
+           NAMES webp/encode.h
+           PATH_SUFFIXES /usr/include /include


Probably also a good idea to search for /usr/local/include? I am not sure though, just thinking.

I'm new to cmake, so I used the same paths as in the other Find*.cmake files. If we used /usr/local/ here, I feel like it'd also have to be changed for the other ones, which I think is a topic for another PR.

I'm not a cmake veteran either :D
If /local/ paths are not used in other Find*.cmake files, we should really not be bothered.

foxpy · 2021-06-14T08:50:17Z

modules/FindWebP.cmake

+
+find_library( WEBP_LIBRARY
+              NAMES libwebp.so
+              PATHS /usr/lib /lib


And same deal here with /usr/local/lib?

foxpy · 2021-06-14T08:52:22Z

maim.1

@@ -40,7 +40,7 @@ Sets the time in seconds to wait before taking a screenshot. Prints a simple mes
 By default maim super-imposes the cursor onto the image, you can disable that behavior with this flag.
 .TP
 .BR \-m ", " \-\-quality
-An integer from 1 to 10 that determines the compression quality. 1 is the highest (and lossiest) compression available for the provided format. For example a setting of `1` with png (a lossless format) would increase filesize and decrease decoding time. While a setting of `1` on a jpeg would create a pixel mush. No effect on bmp images.
+An integer from 1 to 10 that determines the compression quality. 1 is the highest (and lossiest) compression available for the provided format. For example a setting of `1` with png (a lossless format) would increase filesize and decrease decoding time. While a setting of `1` on a jpeg would create a pixel mush. No effect on bmp images. 0-9 set the quality of lossy webp, while 10 will encode as lossless.


"An integer from 1 to 10". I guess this whole line in manual should be rewritten to not be very misleading?

I don't know about the "An integer from 1 to 10" that you mentioned, but I agree that the rest of the line is weird, especially with the special case of png, where the correlation with file size is different. I'll try to rewrite that to be more clear.

Yeah I'm not sure what I was thinking when I wrote that. I probably got confused because both the jpg library and the PNG one refer to the setting as "quality". PNG being backwards in use didn't really raise any alarms to me in 2016 haha. I'm too afraid to fix it since a lot of people probably rely on it being wrong now.

I believe it should be rewritten from a single line to a list.
I see it like this:

--quality For bmp: no effect For jpeg: 1-10, higher is better quality and bigger file, always lossy For png: 1-10, higher is better compression, always lossless For webp: 0-10, higher is better quality and bigger file, lossy for values in range 0-9, lossless for 10

I don't know about the "An integer from 1 to 10" that you mentioned

I don't like how manual says first that the value should be in range from 1 to 10, but suddenly a value of 0 appears for webp in the end.

foxpy · 2021-06-14T08:53:12Z

src/main.cpp

@@ -315,6 +315,8 @@ OPTIONS
              vided format. For example a setting of `1` with  png  (a  lossless
              format)  would increase filesize and speed up encoding dramatical-
              ly. While a setting of `1` on a jpeg would create a pixel mush.
+              No effect on bmp images.  0-9 set the quality of lossy webp, while
+              while 10 will encode as lossless.


The word "while" is doubled here.

foxpy · 2021-06-14T08:53:55Z

src/main.cpp

    ("i,window", "Sets the desired window to capture, defaults to the root window. Allows for an integer, hex, or `root` for input.", cxxopts::value<std::string>())
    ("g,geometry", "Sets the region to capture, uses local coordinates from the given window. So -g10x30-5+0 would represent the rectangle wxh+x+y where w=10, h=30, x=-5, and y=0. x and y are the upper left location of this rectangle.", cxxopts::value<std::string>())
    ("w,parent", "By default, maim assumes the --geometry values are in respect to the provided --window (or root if not provided). This parameter overrides this behavior by making the geometry be in respect to whatever window you provide to --parent. Allows for an integer, hex, or `root` for input.", cxxopts::value<std::string>())
    ("B,capturebackground", "By default, when capturing a window, maim will ignore anything beneath the specified window. This parameter overrides this and also captures elements underneath the window.")
    ("d,delay", "Sets the time in seconds to wait  before taking a screenshot. Prints a simple message to show how many seconds are left before a screenshot is taken. See --quiet for muting this message.", cxxopts::value<float>()->implicit_value("5"))
    ("u,hidecursor", "By default maim super-imposes the cursor onto the image, you can disable that behavior with this flag.")
-    ("m,quality", "An integer from 1 to 10 that determines the compression quality. 1 is the highest (and lossiest) compression  available for the provided format. For example a setting of `1` with png (a loss‐ less format) would increase filesize and decrease decoding time. While a setting of `1` on a jpeg would create a pixel mush. No effect on bmp images.", cxxopts::value<int>())
+    ("m,quality", "An integer from 1 to 10 that determines the compression quality. 1 is the highest (and lossiest) compression  available for the provided format. For example a setting of `1` with png (a loss‐ less format) would increase filesize and decrease decoding time. While a setting of `1` on a jpeg would create a pixel mush. No effect on bmp images. 0-9 set the quality of lossy webp, while 10 will encode as lossless.", cxxopts::value<int>())


This help line is possibly as misleading as manual? I am not entirely sure.

naelstrof · 2021-06-14T10:20:19Z

Looks pretty good. I'll review it tomorrow and try to make a new release. Nice job!

foxpy · 2021-06-14T11:16:20Z

src/image.cpp

+    uint8_t* out;
+    if (quality == 10) {
+        // encode lossless at highest quality
+        size = WebPEncodeLosslessRGBA(data, width, height, width * 4, &out);


Actually, there is more advanced interface for lossless encoding:

struct WebPConfig { int lossless; // Lossless encoding (0=lossy(default), 1=lossless). float quality; // between 0 and 100. For lossy, 0 gives the smallest // size and 100 the largest. For lossless, this // parameter is the amount of effort put into the // compression: 0 is the fastest but gives larger // files compared to the slowest, but best, 100. int method; // quality/speed trade-off (0=fast, 6=slower-better)

It allows us to specify encoding speed for webp in lossless mode. I am not sure if we should use this or just stick to default prefubs.

I saw this as well, but I think we have no good reason to use other settings, unless the user can change them; but that would require more switches, which I think would introduce unnecessary complexity, which is also why I think re-using --quality for deciding between lossy/lossless encoding is good, compared to adding something like --lossless-webp.

naelstrof · 2021-06-15T02:47:13Z

I've merged this pull request, feel free to make another pull request if you feel like some other changes seem necessary. What I saw was well within my quality standards. Thanks so much llenck and foxpy!

llenck added 2 commits June 14, 2021 01:46

find and link with libwebp

33987d1

add webp support/doc

2b11d82

foxpy reviewed Jun 14, 2021

View reviewed changes

modules/FindWebP.cmake

find_library( WEBP_LIBRARY

NAMES libwebp.so

PATHS /usr/lib /lib

Copy link

Collaborator

foxpy Jun 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And same deal here with /usr/local/lib?

foxpy reviewed Jun 14, 2021

View reviewed changes

llenck added 2 commits June 14, 2021 12:21

remove leftover debug messages

262fd81

improve documentation for --quality

c92b037

foxpy reviewed Jun 14, 2021

View reviewed changes

naelstrof merged commit f105793 into naelstrof:master Jun 15, 2021

foxpy mentioned this pull request Jun 19, 2021

WebP Support #190

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add WebP support #233

Add WebP support #233

llenck commented Jun 14, 2021

foxpy Jun 14, 2021

llenck Jun 14, 2021

foxpy Jun 14, 2021

llenck Jun 14, 2021

foxpy Jun 14, 2021

foxpy Jun 14, 2021

foxpy Jun 14, 2021

llenck Jun 14, 2021

naelstrof Jun 14, 2021

foxpy Jun 14, 2021

foxpy Jun 14, 2021

foxpy Jun 14, 2021

foxpy Jun 14, 2021

naelstrof commented Jun 14, 2021

foxpy Jun 14, 2021

llenck Jun 14, 2021

naelstrof commented Jun 15, 2021

Add WebP support #233

Add WebP support #233

Conversation

llenck commented Jun 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naelstrof commented Jun 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naelstrof commented Jun 15, 2021