fix: apply EXIF orientation to filmstrip thumbnails#136
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
Walkthroughhandler.goにEXIF Orientationを反映する回転処理を追加し、EXIFサムネイル返却経路と高解像度からのサムネイル生成経路に適用した。rotate_test.goで回転結果のサイズと色を検証するテストも追加した。 Changesサムネイル回転処理
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant handleThumb
participant rotateImage
participant JPEGEncoder
participant fileOpenSem
participant thumbProcessSem
Client->>handleThumb: サムネイル取得リクエスト
handleThumb->>handleThumb: EXIF Orientation取得
alt JpegThumbnailあり
alt Orientationが3/6/8
handleThumb->>rotateImage: 回転画像を生成
rotateImage-->>handleThumb: 回転後画像
handleThumb->>JPEGEncoder: JPEGエンコード
JPEGEncoder-->>handleThumb: 回転済みバイト列
handleThumb-->>Client: 回転済みサムネイル
else それ以外
handleThumb-->>Client: 元のEXIFサムネイル
end
else JpegThumbnailなし
handleThumb->>thumbProcessSem: 生成処理の許可を取得
handleThumb->>fileOpenSem: ファイルオープンの許可を取得
handleThumb->>rotateImage: 生成画像を回転
rotateImage-->>handleThumb: 回転後画像
handleThumb->>JPEGEncoder: JPEGエンコード
JPEGEncoder-->>handleThumb: サムネイルバイト列
handleThumb-->>Client: サムネイル
end
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR addresses Issue #135 by applying EXIF Orientation when serving filmstrip thumbnails via /api/thumb, so portrait images no longer appear sideways. It introduces a small rotation helper and adds a unit test validating the rotation logic.
Changes:
- Add
rotateImagehelper to apply common EXIF rotations (3/6/8). - Read EXIF Orientation in
handleThumband rotate both embedded EXIF thumbnails and generated thumbnails accordingly. - Add unit tests for the rotation helper.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| handler.go | Reads EXIF Orientation and rotates thumbnails (embedded and generated) using a new rotateImage helper. |
| rotate_test.go | Adds unit test coverage for rotateImage across several orientation values. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if bounds.Dx() != tt.wantW || bounds.Dy() != tt.wantH { | ||
| t.Errorf("rotateImage() bounds = %dx%d, want %dx%d", bounds.Dx(), bounds.Dy(), tt.wantW, tt.wantH) | ||
| } | ||
|
|
||
| r, g, b, a := rotated.At(0, 0).RGBA() |
| thumbProcessSem <- struct{}{} | ||
| defer func() { <-thumbProcessSem }() | ||
|
|
||
| if thumbImg, _, err := image.Decode(bytes.NewReader(pic)); err == nil { | ||
| rotatedThumb := rotateImage(thumbImg, orientation) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@handler.go`:
- Around line 190-232: Add integration coverage for the handleThumb EXIF
rotation path, since rotateImage is already unit-tested but the end-to-end
decode/rotate/re-encode flow is not. In handler_test.go, extend or add a helper
alongside createTestJPEG to generate JPEGs with EXIF Orientation metadata, then
add tests that exercise handleThumb with orientation != 1 and verify the rotated
thumbnail is returned correctly. Also add a case for the fallback path when
rotation or re-encoding fails, asserting the original unrotated thumbnail is
served.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c821397d-68d7-4fd2-95bf-a28f790eeb3a
📒 Files selected for processing (2)
handler.gorotate_test.go
| // Needs rotation, protect with semaphore to prevent CPU exhaustion | ||
| rotatedBytes := func() []byte { | ||
| thumbProcessSem <- struct{}{} | ||
| defer func() { <-thumbProcessSem }() | ||
|
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@handler.go`:
- Around line 201-234: The passthrough thumbnail path in the closure around
x.JpegThumbnail is writing the response with w.Write(pic) while fileOpenSem is
still held, unlike the EXIF-rotation path that defers writing until after the
semaphore is released. Move the passthrough write out of the file-open critical
section so both the non-rotated and rotated thumbnail flows return the bytes and
let the outer handler write the response after releasing fileOpenSem, preserving
the starvation-avoidance behavior without re-encoding.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9adc33b4-a841-4a4c-b8a7-4163237aba46
📒 Files selected for processing (2)
handler.gorotate_test.go
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
fixed #135
縦画像でもサムネイルが横を向く問題を修正
追加ロジックに対するユニットテストを追加