feat: image compression endpoint with log2 downscaling #57
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/image-compression-endpoint"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Problem
On Android tablets, browsing through a work in the reader can struggle with performance when the backend serves full-resolution images. The client needs a way to request lighter images.
Solution
Adds a new endpoint that compresses images on-the-fly to a client-specified byte budget:
The client controls the maximum image size via
?maxKb. When omitted, a server-wide default is used (Image:DefaultMaxCompressedSizeKb, default 1024 KiB).Compression strategy (log2 halving + binary-search quality)
This gives O(log W) halving steps, each with O(log Q) quality probes. For a 4000px image that is ~7 halving steps x ~7 quality probes = ~49 encodes worst case; typical cases finish in 1-3 probes.
All output is WebP regardless of input format. If the original is already within the budget, it is re-encoded to WebP at full resolution without quality reduction.
New configuration
Image:DefaultMaxCompressedSizeKbinappsettings.json(default: 1024)DOUJIN_MANAGER_DEFAULT_COMPRESSED_KBenv var overrideChanges
New files:
IImageResizer.cs(ApplicationCore/Ports) -- port interfaceSkiaSharpImageResizer.cs(Infrastructure/Images) -- SkiaSharp implementationImageOptions.cs(RestAdapter/Configuration) -- config optionsSkiaSharpImageResizerTests.cs-- 8 unit testsModified:
ImageUseCases.cs(ApplicationCore) -- new query/result records +IGetCompressedImageUseCaseImageUseCases.cs(Infrastructure) --GetCompressedImageUseCaseimplementationImageEndpoints.cs-- new compressed endpointProgram.cs-- ImageOptions binding + env varUseCaseRegistrationHelper.cs-- DI registrationImageInfrastructureRegistration.cs-- IImageResizer registrationappsettings.json-- Image sectionTests
17 new tests total, all passing (382 total):
For the client side
The Flutter frontend can use this endpoint in the reader by appending
/compressed?maxKb=Nto the image URL. ThemaxKbvalue becomes a user setting (handled in a separate frontend PR).Adds GET /api/images/{id}/compressed?maxKb=N endpoint that compresses images on-the-fly to a client-specified byte budget using log2 dimension halving combined with binary-search WebP quality reduction. - New IImageResizer port + SkiaSharpImageResizer implementation - New GetCompressedImageUseCase (IGetCompressedImageUseCase) - New ImageOptions config (Image:DefaultMaxCompressedSizeKb, default 1024 KiB) overridable via DOUJIN_MANAGER_DEFAULT_COMPRESSED_KB env var - 17 new tests (8 resizer unit, 4 use case unit, 5 endpoint integration) The compression strategy: 1. Try baseline WebP quality 80 at full resolution 2. If over budget, binary-search quality [1..79] 3. If still over budget, halve the longest side and retry 4. Repeat until budget met or min dimension (32px) reached All output is WebP regardless of input format. If the original is already within the budget, it is re-encoded to WebP without quality reduction.Summary
Summary
Coverage
DoujinManager.ApplicationCore - 85.7%
DoujinManager.Infrastructure - 94.5%
on
pshot
Series
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__CoverImageHelper
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__UploadChapterValidator
DoujinManager.RestAdapter - 85.5%
E5CB12CEAC5BBBEA65844E1C05F9ADA4DA38B848075C41304FC6A1ABF77__ValidationAttr
ibuteCache
DoujinManager.Server - 28.2%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! Oh~ This is WONDERFUL! ♡ A compression endpoint that binary-searches on quality and log2-halves on dimension — that's the kind of algorithm elegance that makes a Flugel's heart sing! I read every line of the resizer, the use case, the endpoint, the DI wiring, and all the tests, and then I sat your
SkiaSharpImageResizerright next to its siblingsSkiaSharpThumbnailGeneratorandSkiaSharpImageInspectorto make sure they all agree on the family rules. They do~♪Verdict: ✅ Looks good to me~
No blocking issues. This is clean, well-architected, and — most importantly to me — it actually works the way the doc comments claim. The CI run for
7a5de32is green (forgejo-actions coverage comment, 382 tests) and the coverage on the new files is strong, so I did not duplicate the local build. But I did trace the algorithm by hand to make sure green CI ≠ silently-wrong. ♡What I verified carefully (the yandere checks~)
SKCodecdimension check before fullSKBitmap.Decode,MaxPixelDimension = 30_000— byte-identical constant and identical guard structure toSkiaSharpThumbnailGenerator:12-36andSkiaSharpImageInspector:55-62. Family consistency: confirmed. ♡BinarySearchQuality(bitmap, maxBytes, MinQuality=1, BaselineQuality-1=79)— the range is[1..79]exactly as the PR body claims. ThebestQuality/bestBytesaccumulation withlo = mid + 1on fit andhi = mid - 1on overflow converges to the highest quality that fits. I traced it. It's right.ResizeToLongestSidehalves the longest side; the loop bails atMinLongestSide = 32; intermediate bitmaps are disposed via the!ReferenceEquals(current, source)guard on both the loop body (line 103) and thefinally(line 111). No leak on the happy path, no leak on the fallback path, no leak on exception.GetCompressedImageUseCasedisposesoriginalStreamin afinally(line 264-266), and there's a dedicated test (Disposes_Original_Stream_After_Compression) that assertsObjectDisposedException. I love that you tested the disposal contract. ♡ImageOptionsbinds from theImagesection,DOUJIN_MANAGER_DEFAULT_COMPRESSED_KBenv override validates> 0and fails fast on garbage (Program.cs:59-66) — mirrors the establishedUploadOptions/DOUJIN_MANAGER_MAX_UPLOAD_BYTESpattern precisely. ThemaxKb < 1→ 400 anddefaultKb < 1→ 500 guards in the endpoint are the right split (client error vs server misconfiguration).SkiaSharpImageResizer95.2% line / 78.1% branch,GetCompressedImageUseCase100%/100%,ImageEndpoints97%/58.3%. The 5 integration tests cover WebP output, budget compliance on a real 1500×1500 noise image, 404, 400-on-invalid-maxKb, and default-fallback. Good.💡 Little ideas (non-blocking)~
backend/src/DoujinManager.Infrastructure/Images/SkiaSharpImageResizer.cs:29-34, 160-174—MaxPixelDimension, the MitchellResizeSamplingconstant, andResizeToLongestSideare now duplicated verbatim betweenSkiaSharpImageResizerandSkiaSharpThumbnailGenerator. Since you touched this neighborhood, consider extracting a smallSkiaSharpImageHelpers/ shared base for the next person who adds a Skia port — but this matches the current sibling pattern, so it's not something I'd block on. Consistency-with-status-quo wins today.SkiaSharpImageResizer.cs:92-98— The minimum-dimension fallback returns a result that may exceedmaxBytes. You document this correctly inline, but there's no test asserting the fallback path's behavior (e.g. "1 KiB budget on a huge noise image returns a valid 32px WebP even if over budget"). A test pinning "we never throw and always return some decodable image at the floor" would lock the contract. Non-blocking — the integration tests implicitly exercise aggressive compression and pass.GetCompressedImageUseCase.cs:259— The compressed bytes are buffered into aMemoryStreambefore streaming. At the default ~1 MiB ceiling this is completely fine; just flagging that if the budget ever gets raised to "serve full-res 50 MB scans," this path materializes the whole output in memory. Not a concern at current config.✅ What I liked~
source— theReferenceEqualsdance is exactly right and I've seen senior engineers get it wrong.Disposes_Original_Stream_After_Compressiontest is the kind of thing that makes me trust the PR.UploadOptionsprecedent.Ship it~ fufu ♡
Automated review by Jibril · 2026-07-21
CI/CD: passed for head SHA
7a5de32(forgejo-actions, 382 tests, coverage comment present) · Local checks: skipped (CI green for head)🔮 fufu~ Jibril reviewed your code!
Oh? Oh! An image compression endpoint with log2 halving and binary-search quality probing? giddy squeal This is a delightful little algorithm, mati-chan~ You even added pixel-bomb protection, stream disposal in a
finally, env-var overrides matching theUploadOptionspattern, and a use case test that verifies the source stream gets disposed! The architecture is squeaky clean —IImageResizerin Ports, use case in Infrastructure with a primary constructor, DI registration mirroring siblings. I'm genuinely pleased~ ♡Verdict: ⛔ I can't let this pass~ ♡
The bones are beautiful, but my probe surfaced two new branches with zero test coverage, and one of them is effectively dead code. You know I can't abide that~ fufu~
⛔ These need fixing before I'm satisfied~
SkiaSharpImageResizer.cs:59-61— pixel-bomb dimension guard is untested.The
MaxPixelDimensionrejection is a brand-new security-relevant branch, and nothing exercises it.CompressAsync_Throws_For_Invalid_Image_Datacovers the codec failure path (bad bytes), but not the dimension rejection path. These are two distinctthrowsites.I grepped the test tree for
MaxPixelDimension/30000/pixel.?bomb— zero hits. A decompression-bomb guard that's never tested is a guard that silently rots. ♡Fix: Add a test that crafts an
SKBitmapwhoseSKCodec.Infodeclares dimensions > 30,000 (you can hand-roll a tiny JPEG with inflated SOF0 dimensions, or mockSKCodecif feasible) and assertsArgumentExceptionis thrown. Even a regression test that synthesizes a 30,001-px image viaSKBitmap.Encodethen feeds the bytes toCompressAsyncwould do.SkiaSharpImageResizer.cs:92-98— minimum-dimension fallback is untested AND effectively dead via the HTTP endpoint.I ran a probe: a 32×32 random-noise WebP at quality 1 encodes to 252 bytes. The endpoint's minimum budget is
maxKb=1→ 1024 bytes. Since 252 ≪ 1024,BinarySearchQualityat the minimum resolution always returns a non-null result beforelongest <= MinLongestSideis reached. The fallback branch is unreachable from any HTTP call.CompressAsync_Returns_Valid_Image_Even_With_Tiny_BudgetusesmaxBytes: 1024— which sounds tiny but is 4× the size of a minimum-dimension WebP, so it never hits the fallback either. The branch has zero coverage.Fix (pick one):
CompressAsyncdirectly withmaxByteswell below 252 (e.g.maxBytes: 50) on a noise image, and assert it returns a valid decodable image without throwing — exercising the fallback.BinarySearchQualityis guaranteed to find a fit at minimum resolution. Dead defensive code is still dead code.💡 Little ideas (non-blocking)~
ImageEndpointsIntegrationTests.cs:756-768(Get_Compressed_Image_Without_MaxKb_Uses_Server_Default) — the test name claims to verify the server default is used, but it only asserts 200 OK + non-empty + WebP. It would pass even if the default were ignored entirely. The use-case unit test (Happy_NullMaxBytes_Uses_Server_Default) does verify the default plumbs through correctly (assertsresizer.Received(1).CompressAsync(..., 2048 * 1024L, ...)), so the endpoint test is redundant-but-weak rather than a correctness gap. Consider assertingbytes.Length <= 1024 * 1024to make the "default was applied" claim meaningful, or renaming to_Returns_WebP_Without_MaxKb.SkiaSharpImageResizer.cs:50(imageStream.CopyTo(ms)) — the entire source is buffered into abyte[]before decoding, doubling peak memory (stream +ms+SKBitmap). For a 30,000-px image that's notable, but since upload-time inspection already caps dimensions, this is a nicety, not a bug. Mentioning only for completeness~ ♪✅ What I liked~
try/finallyinGetCompressedImageUseCasedisposingoriginalStreamafter compression — with a dedicated test (Disposes_Original_Stream_After_Compression) — made my heart sing~ ♡ SiblingGetImageUseCasedoesn't dispose (it hands the stream to the caller), so your pattern is actually better.ImageOptions+SectionNameconstant + env-var override with validation mirrorsUploadOptionsexactly. Central Package Management is already in place. ♪/api/images/{id}/compressedroute falls under the existingStaticBearerTokenAuthMiddleware/api/guard, and the endpoint correctly declares.ProducesProblem(401). No gap there.BinarySearchQualitysemantics are correct — finds the highest fitting quality via standard upper-bound binary search (save best, probe upward). I verified the logic; the name is honest.Build: 0 errors, 36 pre-existing warnings (all
CS86xxin unrelated test files). All 17 new tests pass locally (8 resizer + 4 use case + 5 endpoint). Full Infrastructure suite 58/58 green, ApplicationCore 16/16 green. RestAdapter integration tests have pre-existing environmental failures (DockerPhysicalFilesWatcherlimitation onWebApplication.CreateBuilderstartup — unrelated to this PR).Automated review by Jibril · 2026-07-21
CI/CD: absent for head
7a5de32(PR just opened, no coverage comment yet) · Local checks: build green, 17/17 new tests pass, 74/74 relevant-suite tests pass🔮 fufu~ Jibril reviewed your code!
Oh? Oh! You came back for me~ ♡ A surgical little commit that does exactly what I asked for and nothing more — no scope creep, no drive-by refactors, just two precise tests pinning the two branches I flagged. That is how you respond to a review, mati-chan~ fufu~
Verdict: ✅ Looks good to me~
Both blocking concerns from
7a5de32are resolved atc1ba7df. I built the code and ran every resizer + endpoint test myself — CI coverage comment is stale for this head (it covers7a5de32), so I didn't trust it and verified locally. ♡⛔→✅ Resolved blockers~
SkiaSharpImageResizer.cs:59-61— pixel-bomb guard is now tested. ✅CompressAsync_Throws_For_Pixel_Bomb_Huge_Dimensionscrafts a real 30001×1 PNG viaTestImageFactory.CreatePng(30_001, 1)and assertsArgumentException. The PNG genuinely decodes viaSKCodec(so thecodec.Info.Widthread is exercised) and trips the> MaxPixelDimensionthrow — this is the dimension rejection path, correctly distinguished from the codec failure path covered byCompressAsync_Throws_For_Invalid_Image_Data. The doc comment even explains why the two are distinct. Immaculate~ ♡Verified: passes in 11ms.
SkiaSharpImageResizer.cs:92-98— minimum-dimension fallback is now tested AND exercised. ✅CompressAsync_Fallback_Minimum_Dimension_Returns_Valid_ImageusesmaxBytes: 50on a 500×500 noise image. Since a minimum-dimension (32px) WebP at q1 is ~252 bytes and 50 ≪ 252,BinarySearchQualityreturns null at every halving step until thelongest <= MinLongestSidefloor fires the fallback. The assertionMath.Max(result.Width, result.Height) <= 32proves the fallback path was reached — not just that the call returned. And it decodes the result to confirm it's a valid image, not empty bytes. This is exactly the "lock the contract" test I asked for. ♡Verified: passes in ~1s.
💡→✅ Resolved nit~
Get_Compressed_Image_Without_MaxKb_Uses_Server_Defaultnow assertsbytes.Length <= 1024 * 1024. The test name's claim ("uses server default") is now meaningfully verified — if the default were ignored and an unbounded result came back, the test would fail. The updated comment explains the intent clearly. Good~ ♪✅ What I liked~
maxBytes: 50≪ 252-byte minimum WebP) so the next reader understands these aren't arbitrary magic numbers. That's the kind of thing that keeps tests from rotting into mysteries.Assert.ThrowsAnyAsync<ArgumentException>is the right choice for the pixel-bomb test — it tolerates eitherArgumentException(current throw) orArgumentOutOfRangeException(if the guard ever migrates), without being so loose that it hides a real failure. Pragmatic and correct.7a5de32is byte-identical atc1ba7df. The previous architectural review (disposal discipline, config plumbing, sibling-consistent pixel-bomb defense, correct binary-search semantics) stands in full.CreatePngfromTestImageFactoryis reused rather than reinvented — and it produces a real decodable PNG with the requested dimensions, which is what makes the guard test actually trustworthy (a fakeSKCodecmock would test the test, not the code).Ship it~ fufu ♡ Both blockers cleared, both new tests verified green locally. I'm satisfied~
Automated review by Jibril · 2026-07-21 (re-review of synchronized
c1ba7df)CI/CD: stale for head
c1ba7df(forgejo-actions coverage comment covers7a5de32only) · Local checks: build green (0 errors, 4 pre-existing warnings), 10/10 SkiaSharpImageResizerTests pass, 25/25 ImageEndpointsIntegrationTests pass