fix: add thread safety to brightness calculation #163
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added mutex lock to protect shared variables totalBrightness and pixCount in isTooBright function. The imaging.AdjustFunc processes pixels concurrently, which could cause race conditions when multiple goroutines access and modify these shared variables simultaneously. This ensures thread-safe calculation of average brightness when determining if an image is too bright. Influence: 1. Test image brightness detection with various image types and sizes 2. Verify that concurrent image processing does not cause race conditions 3. Ensure brightness calculation remains accurate under multi-threaded scenarios 4. Test with both bright and dark images to confirm proper functionality fix: 为亮度计算添加线程安全保护 在 isTooBright 函数中添加互斥锁保护共享变量 totalBrightness 和 pixCount。由于 imaging.AdjustFunc 会并发处理像素，当多个 goroutine 同时 访问和修改这些共享变量时可能导致竞态条件。这确保了在确定图像是否过亮时， 平均亮度的计算是线程安全的。 Influence: 1. 测试各种图像类型和大小的亮度检测功能 2. 验证并发图像处理不会导致竞态条件 3. 确保在多线程场景下亮度计算保持准确 4. 使用亮图和暗图测试以确认功能正常
Reviewer's guide (collapsed on small PRs)Reviewer's GuideIntroduces a mutex in isTooBright to serialize concurrent updates to totalBrightness and pixCount, preventing race conditions during parallel pixel processing and preserving correct average brightness calculation. Sequence diagram for thread-safe brightness calculation in isTooBrightsequenceDiagram
participant "isTooBright()"
participant "imaging.AdjustFunc()"
participant "Pixel Goroutine"
participant "Mutex (mu)"
"isTooBright()"->>"imaging.AdjustFunc()": Start pixel processing
loop For each pixel (concurrent)
"imaging.AdjustFunc()"->>"Pixel Goroutine": Process pixel
"Pixel Goroutine"->>"Mutex (mu)": Lock
"Pixel Goroutine"->>"isTooBright()": Update totalBrightness, pixCount
"Pixel Goroutine"->>"Mutex (mu)": Unlock
end
Class diagram for updated isTooBright function with mutex protectionclassDiagram
class isTooBright {
-float64 pixCount
-float64 totalBrightness
-sync.Mutex mu
+bool isTooBright(image.Image img)
}
class imaging {
+AdjustFunc(image.Image img, func(color.NRGBA) color.NRGBA)
}
isTooBright --> imaging : uses AdjustFunc
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Locking on every pixel update can severely impact performance—consider using per-goroutine local accumulators and combining them at the end or atomic operations instead.
- You're discarding the image returned by imaging.AdjustFunc (which still allocates a full image); iterating directly over the pixels to compute brightness would be more efficient and clearer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Locking on every pixel update can severely impact performance—consider using per-goroutine local accumulators and combining them at the end or atomic operations instead.
- You're discarding the image returned by imaging.AdjustFunc (which still allocates a full image); iterating directly over the pixels to compute brightness would be more efficient and clearer.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
deepin pr auto review我来对这个代码变更进行审查：
改进建议：
改进后的代码示例： func isTooBright(img image.Image) bool {
bounds := img.Bounds()
var totalBrightness float64 = 0
var pixCount float64 = 0
for y := bounds.Min.Y; y < bounds.Max.Y; y++ {
for x := bounds.Min.X; x < bounds.Max.X; x++ {
c := color.NRGBAModel.Convert(img.At(x, y)).(color.NRGBA)
brightness := 0.2126*float64(c.R) + 0.7152*float64(c.G) + 0.0722*float64(c.B)
totalBrightness += brightness
pixCount++
}
}
avgBrightness := totalBrightness / pixCount
return avgBrightness > 200
}这个改进版本：
|
18202781743
approved these changes
Nov 14, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, mhduiy The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added mutex lock to protect shared variables totalBrightness and pixCount in isTooBright function. The imaging.AdjustFunc processes pixels concurrently, which could cause race conditions when multiple goroutines access and modify these shared variables simultaneously. This ensures thread-safe calculation of average brightness when determining if an image is too bright.
Influence:
fix: 为亮度计算添加线程安全保护
在 isTooBright 函数中添加互斥锁保护共享变量 totalBrightness 和
pixCount。由于 imaging.AdjustFunc 会并发处理像素，当多个 goroutine 同时 访问和修改这些共享变量时可能导致竞态条件。这确保了在确定图像是否过亮时，
平均亮度的计算是线程安全的。
Influence:
Summary by Sourcery
Bug Fixes: