advanced webp output options - alpha quality, lossless and near lossless #685

rn4391 · 2017-01-17T06:14:05Z

No description provided.

lovell

Hello, thanks for this, I'll added some comments inline.

lovell · 2017-01-17T21:12:28Z

lib/constructor.js

@@ -145,6 +145,8 @@ const Sharp = function (input, options) {
    pngCompressionLevel: 6,
    pngAdaptiveFiltering: true,
    webpQuality: 80,
+    webpAlphaQuality: 100,
+    webpLossless: false,
    tiffQuality: 80,


Is this missing a webpNearLossless: false default? This might be the reason for the "Partial image extraction WebP" test failure.

Tests on my system passed without this as well. Still added it.

lovell · 2017-01-17T21:12:58Z

lib/output.js

@@ -168,6 +168,9 @@ const png = function png (options) {
 * Use these WebP options for output image.
 * @param {Object} [options] - output options
 * @param {Number} [options.quality=80] - quality, integer 1-100
+ * @param {Number} [options.alphaQuality=100] - quality of alpha layer, integer 0-100
+ * @param {Boolean} [options.lossless=false] - use lossless compression mode
+ * @param {Boolean} [options.nearLossless=false] - use near_lossless compression mode


Are lossless and nearLossless mutually exclusive?

nearLossless has to be used in conjunction with Q. nearLossless with Q=100 is equivalent to lossless . But with other quality settings, it will lead to a smaller file size than lossless. This does make lossless kind of redundant (https://groups.google.com/a/webmproject.org/forum/#!topic/webp-discuss/0GmxDmlexek) as a shorthand way of saying nearLossless, Q=100

Sorry, my question wasn't very clear. What happens when trying to combine the two?

.webp({ lossless: true, nearLossless: true })...

Is this valid?

lovell · 2017-01-17T21:13:39Z

src/pipeline.cc

@@ -1209,6 +1218,11 @@ NAN_METHOD(pipeline) {
  baton->pngCompressionLevel = AttrTo<uint32_t>(options, "pngCompressionLevel");
  baton->pngAdaptiveFiltering = AttrTo<bool>(options, "pngAdaptiveFiltering");
  baton->webpQuality = AttrTo<uint32_t>(options, "webpQuality");
+  baton->webpAlphaQuality = AttrTo<uint32_t>(options, "webpAlphaQuality");
+  baton->webpLossless = AttrTo<bool>(options, "webpLossless");
+  if(HasAttr(options, "webpNearLossless")) {


Does this attribute need the HasAttr check given there is (or should be) a default?

Removed the check since the default has been added

lovell · 2017-01-17T21:15:26Z

test/unit/io.js

+          if (err) throw err;
+          assert.strictEqual(true, data.length > 0);
+          assert.strictEqual('webp', info.format);
+          fixtures.assertSimilar(fixtures.expected('webp-alpha-80.webp'), data, function (err) {


The inline function can be rolled up to use done, something like:

fixtures.assertSimilar(fixtures.expected('webp-alpha-80.webp'), data, done);

Thanks for pointing this out

lovell · 2017-01-17T21:18:25Z

test/unit/io.js

@@ -735,6 +779,12 @@ describe('Input/output', function () {
    });
  });

+  it('Invalid WebP alpha quality throws error', function () {


Please can you add tests for invalid lossless and nearLossless attributes also.

Invalid lossless and nearLossless attributes are ignored and are set to default boolean false. Similar to what is done for jpegOvershootDeringing and other such params

rn4391 · 2017-01-18T07:35:05Z

If it looks good now, I can squash it into a single commit?

lovell · 2017-01-18T12:59:32Z

docs/api-operation.md

@@ -169,7 +169,7 @@ This operation will always occur after resizing and extraction, if any.
 // to the top, left and right edges and 20 to the bottom edge
 sharp(input)
  .resize(140)
-  .background({r: 0, g: 0, b: 0, a: 0})
+  .background({r: 0, g: 0, b: 0, alpha: 0})


This change was included in commit 4730554 so I think this file can be removed from the PR.

lovell · 2017-01-18T13:02:32Z

lib/output.js

@@ -168,6 +168,9 @@ const png = function png (options) {
 * Use these WebP options for output image.
 * @param {Object} [options] - output options
 * @param {Number} [options.quality=80] - quality, integer 1-100
+ * @param {Number} [options.alphaQuality=100] - quality of alpha layer, integer 0-100
+ * @param {Boolean} [options.lossless=false] - use lossless compression mode
+ * @param {Boolean} [options.nearLossless=false] - use near_lossless compression mode


Sorry, my question wasn't very clear. What happens when trying to combine the two?

.webp({ lossless: true, nearLossless: true })...

Is this valid?

rn4391 · 2017-01-19T10:19:49Z

Since this is valid vips copy x.jpg x.webp[lossless,near_lossless] and uses near_lossless with default Q setting, then I guess even

.webp({
  lossless: true,
  nearLossless: true
})

should be considered valid. I will add a test for the same, that when both lossless and nearLossless are provided, nearLossless should kick in.

coveralls · 2017-01-19T12:52:22Z

Coverage increased (+0.007%) to 99.497% when pulling 0ac8a6e on rnanwani:feature-advanced-webp-options into 815d076 on lovell:master.

lovell · 2017-01-19T13:45:57Z

Fantastic, thank you very much for this Rahul.

advanced webp output options - alpha quality, lossless and near lossless

4f17c16

lovell requested changes Jan 17, 2017

View reviewed changes

rn4391 added 2 commits January 18, 2017 12:29

advanced webp output options - alpha quality, lossless and near lossless

5e8e269

addressing code review

40aea9b

lovell requested changes Jan 18, 2017

View reviewed changes

rn4391 added 2 commits January 19, 2017 17:26

adding test for both lossless and near_lossless

60d2ced

fix test error

0ac8a6e

lovell approved these changes Jan 19, 2017

View reviewed changes

lovell merged commit a1b8efe into lovell:master Jan 19, 2017

lovell added this to the v0.17.2 milestone Jan 19, 2017

lovell mentioned this pull request Jan 19, 2017

WebP advanced output options #569

Closed

lovell added a commit that referenced this pull request Jan 22, 2017

Add changelog entry and credit for #685

d241efc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

advanced webp output options - alpha quality, lossless and near lossless #685

advanced webp output options - alpha quality, lossless and near lossless #685

rn4391 commented Jan 17, 2017

lovell left a comment

lovell Jan 17, 2017

rn4391 Jan 18, 2017

lovell Jan 17, 2017

rn4391 Jan 18, 2017 •

edited

Loading

lovell Jan 18, 2017

lovell Jan 17, 2017

rn4391 Jan 18, 2017

lovell Jan 17, 2017

rn4391 Jan 18, 2017

lovell Jan 17, 2017

rn4391 Jan 18, 2017

rn4391 commented Jan 18, 2017

lovell Jan 18, 2017

lovell Jan 18, 2017

rn4391 commented Jan 19, 2017 •

edited

Loading

coveralls commented Jan 19, 2017 •

edited

Loading

lovell commented Jan 19, 2017

advanced webp output options - alpha quality, lossless and near lossless #685

advanced webp output options - alpha quality, lossless and near lossless #685

Conversation

rn4391 commented Jan 17, 2017

lovell left a 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

rn4391 Jan 18, 2017 • edited Loading

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

rn4391 commented Jan 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rn4391 commented Jan 19, 2017 • edited Loading

coveralls commented Jan 19, 2017 • edited Loading

lovell commented Jan 19, 2017

rn4391 Jan 18, 2017 •

edited

Loading

rn4391 commented Jan 19, 2017 •

edited

Loading

coveralls commented Jan 19, 2017 •

edited

Loading