search

foliojs / brotli.js

A JavaScript port of the Brotli compression algorithm, as used in WOFF2
500 stars 51 forks source link

Is the quality option OK? #38

Open Joxter opened 2 years ago

Joxter commented 2 years ago

Hi, is the quality option in the compress function OK?

https://github.com/foliojs/brotli.js/blob/master/compress.js#L18=

  } else if (typeof opts === 'object') {
    quality = opts.quality || 11;
    mode = opts.mode || 0;
    lgwin = opts.lgwin || 22;
  }

README says quality could be 0, but then it should turn into 11. My tests confirm that 0 works like 11.

Where is a mistake? I am extremely sorry if I messed up something

persunde commented 1 year ago

My tests also showed that quality = 0 and quality = 11, results in the same compression quality.

As @Joxter pointed out, the source confirms my suspicion as the code uses a "fasly" check and that logic is incorrect when the quality value is 0. I am referring to this line: quality = opts.quality || 11;

Instead the code should use the nullish coalescing (??) operator to check if the value is defined or not. Like this: quality = opts.quality ?? 11;

I will submit a merge request soon when I have some extra time.