Skip to content

Commit

Permalink
fix: cannot access local variable 'quality' ... (#33)
Browse files Browse the repository at this point in the history
* fix: cannot access local variable 'quality' ...

Moves the qmin/qmax conversion logic from AvifImagePlugin to
_avif.c, where we know whether avif supports the quality encoder option.

This allows users to continue to pass qmin/qmax to save(), even if they
are using libavif >= 1.0.0

fixes #32

* fix qminmax quality test
  • Loading branch information
fdintino authored Oct 12, 2023
1 parent f046fa2 commit 63ecc8d
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 34 deletions.
22 changes: 5 additions & 17 deletions src/pillow_avif/AvifImagePlugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,23 +126,11 @@ def _save(im, fp, filename, save_all=False):

is_single_frame = total == 1

qmin = info.get("qmin")
qmax = info.get("qmax")

if qmin is None and qmax is None:
# The min and max quantizer settings in libavif range from 0 (best quality)
# to 63 (worst quality). If neither are explicitly specified, we use a 0-100
# quality scale (default 75) and calculate the qmin and qmax from that.
#
# - qmin is 0 for quality >= 64. Below that, qmin has an inverse linear
# relation to quality (i.e., quality 63 = qmin 1, quality 0 => qmin 63)
# - qmax is 0 for quality=100, then qmax increases linearly relative to
# quality decreasing, until it flattens out at quality=37.
quality = info.get("quality", 75)
if not isinstance(quality, int) or quality < 0 or quality > 100:
raise ValueError("Invalid quality setting")
qmin = max(0, min(64 - quality, 63))
qmax = max(0, min(100 - quality, 63))
qmin = info.get("qmin", -1)
qmax = info.get("qmax", -1)
quality = info.get("quality", 75)
if not isinstance(quality, int) or quality < 0 or quality > 100:
raise ValueError("Invalid quality setting")

duration = info.get("duration", 0)
subsampling = info.get("subsampling", "4:2:0")
Expand Down
21 changes: 18 additions & 3 deletions src/pillow_avif/_avif.c
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,18 @@ AvifEncoderNew(PyObject *self_, PyObject *args) {
return NULL;
}

enc_options.qmin = normalize_quantize_value(qmin);
enc_options.qmax = normalize_quantize_value(qmax);
if (qmin == -1 || qmax == -1) {
#if AVIF_VERSION >= 1000000
enc_options.qmin = -1;
enc_options.qmax = -1;
#else
enc_options.qmin = normalize_quantize_value(64 - quality);
enc_options.qmax = normalize_quantize_value(100 - quality);
#endif
} else {
enc_options.qmin = normalize_quantize_value(qmin);
enc_options.qmax = normalize_quantize_value(qmax);
}
enc_options.quality = quality;

if (speed < AVIF_SPEED_SLOWEST) {
Expand Down Expand Up @@ -326,7 +336,12 @@ AvifEncoderNew(PyObject *self_, PyObject *args) {

encoder->maxThreads = max_threads;
#if AVIF_VERSION >= 1000000
encoder->quality = enc_options.quality;
if (enc_options.qmin != -1 && enc_options.qmax != -1) {
encoder->minQuantizer = enc_options.qmin;
encoder->maxQuantizer = enc_options.qmax;
} else {
encoder->quality = enc_options.quality;
}
#else
encoder->minQuantizer = enc_options.qmin;
encoder->maxQuantizer = enc_options.qmax;
Expand Down
36 changes: 22 additions & 14 deletions tests/test_file_avif.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,6 @@
except ImportError:
from multiprocessing import cpu_count

try:
from unittest import mock
except ImportError:
import mock

import pytest

from PIL import Image
Expand Down Expand Up @@ -93,6 +88,17 @@ def skip_unless_avif_version_gte(version):
return pytest.mark.skipif(should_skip, reason=reason)


def skip_unless_avif_version_lt(version):
if not _avif:
reason = "AVIF unavailable"
should_skip = True
else:
version_str = ".".join([str(v) for v in version])
reason = "%s > %s" % (_avif.libavif_version, version_str)
should_skip = _avif.VERSION >= version
return pytest.mark.skipif(should_skip, reason=reason)


class TestUnsupportedAvif:
def test_unsupported(self):
AvifImagePlugin.SUPPORTED = False
Expand Down Expand Up @@ -494,6 +500,7 @@ def test_encoder_codec_available_cannot_decode(self):
def test_encoder_codec_available_invalid(self):
assert _avif.encoder_codec_available("foo") is False

@skip_unless_avif_version_lt((1, 0, 0))
@pytest.mark.parametrize(
"quality,expected_qminmax",
[
Expand All @@ -505,15 +512,16 @@ def test_encoder_codec_available_invalid(self):
],
)
def test_encoder_quality_qmin_qmax_map(self, tmp_path, quality, expected_qminmax):
MockEncoder = mock.Mock(wraps=_avif.AvifEncoder)
with mock.patch.object(_avif, "AvifEncoder", new=MockEncoder) as mock_encoder:
with Image.open("tests/images/hopper.avif") as im:
test_file = str(tmp_path / "temp.avif")
if quality is None:
im.save(test_file)
else:
im.save(test_file, quality=quality)
assert mock_encoder.call_args[0][3:5] == expected_qminmax
qmin, qmax = expected_qminmax
with Image.open("tests/images/hopper.avif") as im:
out_quality = BytesIO()
out_qminmax = BytesIO()
im.save(out_qminmax, "AVIF", qmin=qmin, qmax=qmax)
if quality is None:
im.save(out_quality, "AVIF")
else:
im.save(out_quality, "AVIF", quality=quality)
assert len(out_quality.getvalue()) == len(out_qminmax.getvalue())

def test_encoder_quality_valueerror(self, tmp_path):
with Image.open("tests/images/hopper.avif") as im:
Expand Down

0 comments on commit 63ecc8d

Please sign in to comment.