Feature/webp target size #3950
Feature/webp target size #3950
Conversation
|
An additional note: I'll have to verify, but I believe the target_size parameter doesn't limit the size, it just tries to make it "close" depending on the number of passes. I'll do some smoke tests. Edit: For my case, I think that's fine. Cloudflare Images has a hard limit of 10MB, but honestly a 10MB webp has way more fidelity that I really need. I'll likely set |
|
I wanted to make sure that I pick a sane default for Here's some python code I wrote to exercise this branch: TARGET_SIZE = 1_000_000
with open("myfile.jpg", "rb") as f:
buffer = f.read()
image = pyvips.Image.new_from_buffer(buffer, "")
print("Default encoding")
with Bench("webpsave_buffer"):
buff = image.webpsave_buffer()
print(f"Size: {len(buff)}")
print()
print()
for i in range(1, 11):
with Bench(f"target_size={TARGET_SIZE} & passes={i}"):
buff = image.webpsave_buffer(target_size=TARGET_SIZE, passes=i)
print(
f"Size with passes={i}: {len(buff)} ({100 * len(buff) / TARGET_SIZE:.2f}%)"
)
print()And here's the output A few conclusions:
|
john-parton
force-pushed
the
feature/webp_target_size
branch
from
bf64131
to
97d58df
Compare
|
I think this is ready for another review.
I think I need help with code style. Is there an auto-formatter / linter that I can run locally? |
|
Ooop, "approved when changes are made", I meant. Thank you for doing this work! |
Yes, run |
john-parton
force-pushed
the
feature/webp_target_size
branch
from
d5184d9
to
d4dc563
Compare
|
I think this is ready for review again. I have a comment above, but I'll re-iterate it here. It's important that |
|
I might need a little help documenting "passes". So basically, it seems to only do something positive/beneficial when If you turn on passes without either target_size or target_PSNR, it just dramatically increases encoding time, while leaving both the size and PSNR of the resulting image essentially untouched. Output: Exercise script: with open("test.jpg", "rb") as f:
buffer = f.read()
image = pyvips.Image.new_from_buffer(buffer, "")
for i in range(1, 11):
with Bench(f"passes={i},Q={Q}"):
buff = image.webpsave_buffer(passes=i, Q=Q)
with open(f"out_{i}.webp", "wb") as f:
f.write(buff)
print(f"Size with passes={i}: {len(buff)}")
print()
img_og = cv2.imread("test.jpg")
for i in range(1, 11):
img = cv2.imread(f"out_{i}.webp")
psnr = cv2.PSNR(img_og, img)
print(f"PSNR with passes={i}: {psnr}")
print()I didn't implement Not sure how to document this to basically say "this is a foot-gun if you don't have target_size set." |
john-parton
force-pushed
the
feature/webp_target_size
branch
from
641ac8e
to
6789323
Compare
|
I think this if ready for a final review. I updated the docs to caution users against setting |
|
@kleisauke Would you mind taking another look? I think this is pretty close. |
|
Ah I jumped the gun, there seem to be some settings issues. With this branch, I see: The tests above are failing with: |
|
Use |
|
Commit 8de3704 should fix that. |
|
Ah, sorry, that only resolves # target_size should reasonably work, +/- 2% is fine
x = pyvips.Image.new_from_file(WEBP_FILE)
buf_size = len(x.webpsave_buffer(target_size=10_000))
> assert 9800 < buf_size < 10200
E assert 18684 < 10200 |
|
Commit bedd618 should fix the Python test suite by increasing the I also passed |
john-parton
force-pushed
the
feature/webp_target_size
branch
from
bedd618
to
fc7b4b0
Compare
Change default number of passes when target size is set to 3. Exercise target_size. Add note in changelog. Manually cherry-picked fixes by kleisauke.
john-parton
force-pushed
the
feature/webp_target_size
branch
from
fc7b4b0
to
834ff08
Compare
|
Thanks for being so helpful and patient. This is my first time using meson/ninja and the first time I've touched C/C++ seriously in a while. I merged in the fixes you wrote, squashed it down to one commit, and rebased on master to fix a merge conflict with the changelog. Ran both |
|
Great! This will be in 8.16, due in a few months. |
This is the initial work to close #3824
It works, but there's one "gotcha." The
target_sizeparameter doesn't do anything unlesspassis 2 or greater. The more passes, the closer it will get totarget_size. In my non-scientific tests, 4 to 6 seem to be reasonable numbers.I did a basic 'smoke test' with the CLI tool:
and python
Note that 'pass' is a keyword in python, so I have to pass it by unpacking a dictionary. Very not fun.
Does this general approach seem reasonable?