- Maniphest Tasks
- T27611: Support optimized WebP thumbnails as alternative to JPEG, PNG
fgiunchedi • Imarlier
- rTHMBREXTbd557b6c12c1: WebP output support
- Patch without arc
- git checkout -b D1016 && curl -L https://phabricator.wikimedia.org/D1016?download=true | git apply
Upstream might be interested in adding such a mode (output to stdout), if possible?
It seems to me run_operators is convert-specific, meaning I'm expecting it to only call/interact with convert. I think having webp conversion in a separate function would make the code more clear and easier to test, unless there's something I'm missing?
This change doesn't seem related to webp?
Ditto, not related to this change?
The issue is with the dumb IM delegate, which is extremely limited in what can be passed to the cwebp command. To have all webp parameters available to IM, we could need to compile IM with WebP support (the Debian version doesn't have it).
The actual cwebp command does support output to stdout, which is what I'm doing in the latest version.
Yeah I'm going to refactor things in my next pass, I was waiting to have all the parts working. The way webp supported is injected right now doesn't please me.
It's actually related, one of the WebP output files in the tests hits that execution path when its mime gets sniffed by this engine.
It is related. Without storing the content-type explicitly, when reading the object back Swift can't figure out that it's webp, and will serve it as octet stream, making the browser download the image instead of displaying it.
And in the end as you'll see below, I need access to a whole bunch of parameters on the cwebp command, not just the ability to output to stdout.
Without calling the cwebp command directly, I wouldn't be able to use lossless mode, to keep the metadata, etc.
- Split tests into smaller focused files
- Make EXIF tests only about EXIF
- Add WebP output test coverage for every test outputting JPG or PNG
- Switch reference visual thumbnail from MediaWiki output to perfect lossless reference thumbnail
- Make test size assertions explicit instead of derived from reference thumbnails
- Shrink with VIPS to a larger size, to avoid height rounding errors
- Make jpeg:size hint target a larger size, to avoid height rounding errors
- Make rounding strategy explicit to avoid python's default rounding to the nearest odd number on halves
- Make IM resize parameter more strict to avoid off-by-one-pixel widths and ensure that the requested width is always respected
- Remove dead code for reading/writing buffer data, which was used for an older version of the conditional sharpen filter
Where does this file path come from? Another test?
.format() is preferred at this point. (Best to be consistent, though, if there's any string formatting elsewhere that uses the old style.)
I'm guessing there's a check elsewhere that errors in case both are zero?
That's just something that made troubleshooting easier during the refactoring, I'll get rid of it.
I've used that style consistently (unaware that it's "old") in thumbor plugins code, because it was the most common style seen in Thumbor itself (64 hits vs 18 for format).
Thumbor tends to be oldschool as I believe it's not python3-compatible yet.
This can't happen because hitting resize(), which then calls this, implies that at least one target dimension is specified.
I'll add a test case for when no dimension is defined (no resize) for completion's sake, even though this can't happen at all in the Wikimedia setup.