Isotr0py / pillow-jpegxl-plugin

Pillow plugin for JPEG-XL, using Rust for bindings.
GNU General Public License v3.0
17 stars 5 forks source link

Quality behaving differently from reference encoder #53

Closed jojje closed 1 month ago

jojje commented 1 month ago

In the later releases (and latest: v1.2.5) the interpretation of quality seems to have changed. As far as I recall, earlier releases followed the reference encoder's interpretation more or less.

When looking at the jpegxl-rs documentation it seems that library is using distance rather than quality, but calling the variable quality still. Those two concepts are completely distinct. The latter requires a very specific transformation to the former. Could this explain the divergent behavior from the reference implementation that I observe below for your plugin?

Right now, with quality set to 90 for some random test image, I get the following encoding statistics for your plugin vs the reference encoder:

Quality: 90

Effort 1
  plugin    [size: 40267440, duration: 2.402]
  reference [size: 13734293, duration: 2.710]

Effort 2
  plugin    [size: 40267440, duration: 2.394]
  reference [size: 13734293, duration: 2.714]

Effort 3
  plugin    [size: 37676149, duration: 2.429]
  reference [size: 12893847, duration: 2.815]

Effort 4
  plugin    [size: 37803418, duration: 2.541]
  reference [size: 13350005, duration: 3.044]

Effort 5
  plugin    [size: 26061484, duration: 3.516]
  reference [size: 12424286, duration: 4.807]

Effort 6
  plugin    [size: 24412418, duration: 3.964]
  reference [size: 12201280, duration: 5.452]

Effort 7
  plugin    [size: 28384223, duration: 12.607]
  reference [size: 12206682, duration: 5.540]

The test script so you can reproduce the observation:

import subprocess
import shlex
import sys
from time import time
from io import BytesIO

from PIL import Image
import pillow_jxl

def encode_plugin(filename, quality, effort):
    bio = BytesIO()
    t = time()
    Image.open(filename).save(bio, format='jxl', quality=quality, effort=effort)
    d = time() - t
    return len(bio.getvalue()), d

def encode_cli(filename, quality, effort):
    cmd = f'cjxl -q {quality} -e {effort} "{filename}" -'
    t = time()
    p = subprocess.run(shlex.split(cmd), check=True, capture_output=True)
    d = time() - t
    return len(p.stdout), d

filename = sys.argv[1]
quality = 90

print('Quality:', quality)
for effort in range(1,8):
    size_plugin, time_plugin = encode_plugin(filename, quality, effort)
    size_refenc, time_refenc = encode_cli(filename, quality, effort)
    print(f'\nEffort {effort}')
    print(f'  plugin    [size: {size_plugin}, duration: {time_plugin:.3f}]')
    print(f'  reference [size: {size_refenc}, duration: {time_refenc:.3f}]')

Versions used:

Isotr0py commented 1 month ago

Yes, the quality used in jpegxl-rs encoder is the distance (which varies from 0..25) in fact. And we map jpeg quality to distance at python side in earlier version (#29)

However, jpegxl-rs updated a set_jpeg_quality method to map jpeg quality to distance through the libjxl C-API JxlEncoderDistanceFromQuality in v0.10.4. We migrate to this implementation in v1.2.5, and I think there may be something wrong in migration which causes the divergence.

I will have a check at the Rust side implementation for this.

Isotr0py commented 1 month ago

OK, I found what is the cause. The issue is the default value of use_container and use_original_profile, which we set it true in plugin by default, while cjxl set it false.

Plugin

https://github.com/Isotr0py/pillow-jpegxl-plugin/blob/d308b3d045baca5001b353fec24adcb40f220788/pillow_jxl/JpegXLImagePlugin.py#L95-L96

cjxl

 --container=0|1
    0 = Avoid the container format unless it is needed (default)
    1 = Force using the container format even if it is not needed.

Benchmarks after setting use_container=False:

Quality: 98

Effort 1
  plugin    [size: 4600335, duration: 1.322]
  reference [size: 4600228, duration: 1.364]

Effort 2
  plugin    [size: 4600335, duration: 1.391]
  reference [size: 4600228, duration: 1.363]

Effort 3
  plugin    [size: 2786269, duration: 1.264]
  reference [size: 2786162, duration: 1.277]

Effort 4
  plugin    [size: 2839899, duration: 1.317]
  reference [size: 2839792, duration: 1.578]

Effort 5
  plugin    [size: 3522876, duration: 1.736]
  reference [size: 3522769, duration: 1.953]

Effort 6
  plugin    [size: 3524057, duration: 1.928]
  reference [size: 3523950, duration: 2.041]

Effort 7
  plugin    [size: 3638554, duration: 1.927]
  reference [size: 3638447, duration: 2.088]

I will fix this and clarify the default behavior of the plugin.

jojje commented 1 month ago

Awesome. Will upgrade as soon as you've released a new version. Thanks for the swift investigation.

There was no urgency, since I could fall back on the reference encoder, but it's nice to not having to round-trip to a subprocess when using Pillow and numpy. Fewer moving parts :)

PS. Very strange that the container had such a large effect. Afaik the storage overhead for the container is just a few bytes unless something besides the bitstream is placed in the container; like gigantic metadata boxes. If you disable the container, keep in mind that you'll probably break the code you have that handles metadata transfer. That's why it's noted as "when not needed" in the reference implementation, since if anything other than the bitstream needs to be stored, a container is required as the bitstream itself has no place for such information.