Squashed from the PR, relevant commit messages follow below:
Added file size option to artresizer
- In line with comments on PR, adjusted the ArtResizer API to add
functionality to "resize to X bytes" through `max_filesize` arg
- Adjustment to changelog.rst to include max_filesize change to ArtResizer
and addition of new plugin.
Added explicit tests for PIL & Imagemagick Methods
- Checks new resizing functions do reduce the filesize of images
Expose max_filesize logic to fetchart plugin
- Add syspath escaping for OS cross compatibility
- Return smaller PIL image even if max filesize not reached.
- Test resize logic against known smaller filesize (//2)
- Pass integer (not float) quality argument to PIL
- Remove Pillow from dependencies
- Implement "max_filesize" fetchart option, including
logic to resize and rescale if maxwidth is also set.
Added tests & documentation for fetchart additions.
Tests now check that a target filesize is reached with a
higher initial quality (a difficult check to pass).
With a starting quality of 95% PIL takes 4 iterations to succeed
in lowering the example cover image to 90% its original size.
To cover all bases, the PIL loop has been changed to 5 iterations
in the worst case, and the documentation altered to reflect the
50% loss in quality this implies. This seems reasonable as users
concerned about performance would most likely be persuaded to
install ImageMagick, or remove the maximum filesize constraint.
The previous 30% figure was arbitrary.
In
766c8b190b from [1]
a slight hack was introduced that allowed to add new arguments to
plugin events without breaking legacy plugins that did not feature
those arguments in the signature of their handler functions.
When improving logging management for plugins in
327b62b610 from [2]
this hack was removed, to be restored with
7f34c101d7 from [3]
Now in addition to inspecting the function signature, an assumption is
made about the string message of the TypeError that occurs for legacy
handler functions (namely, that it start with func.__name__).
In Python 3.10, the TypeError uses func.__qualname__ [4], however, due
to the patch [5]. Thus, checking whether the TypeError is due to an
outdated function signature or something internal to the handler fais.
As a fix, instead of using __name__ or __qualname__ depending on Python
version, let's just revert to the old hack from [1], I'm not seeing a
significant advantage in [3]. The latter might be a bit faster since it
doesn't construct a new dict for each call, but only for legacy calls in
the excption handler. I doubt that really matters, and if it does, we
should try to think of a better fix than inspecting error messages with
no guarantees about their content in such a central place in the code.
[1] https://github.com/beetbox/beets/pull/652
[2] https://github.com/beetbox/beets/pull/1320
[3] https://github.com/beetbox/beets/pull/1359
[4] https://docs.python.org/3.10/glossary.html#term-qualified-name
[5] https://bugs.python.org/issue40679
We lost the ability to show skipped tests when transitioning to gh
actions. As it turns out, all of the replaygain tests were being
skipped, so as a start, try to install at least one backend.
I didn't actually benchmark this, but creating new FormattedItemMapping
views in each iteration of the loop appeared like a rather inefficient
way of doing things. In reality, this is probably completely irrelevant,
since one would usually not show huge numbers of changed Models at a
time.
