Skip to content

ipa: rpi: agc: Clamp digital gain to avoid saturation issues#340

Closed
davidplowman wants to merge 1 commit into
nextfrom
saturation-fix
Closed

ipa: rpi: agc: Clamp digital gain to avoid saturation issues#340
davidplowman wants to merge 1 commit into
nextfrom
saturation-fix

Conversation

@davidplowman
Copy link
Copy Markdown
Collaborator

Digital gain is used to adjust overall image exposure when the target exposure cannot be achieved (for example, cannot go high enough). But when the target exposure is lower than we can achieve, the digital gain was being set to values less than unity, causing saturation problems.

The fix is simply to clamp the digital gain at the bottom to 1.0, resulting in images that, while "too bright", saturate correctly.

Fixes: 17f9912 ("ipa: rpi: agc: Calculate digital gain in process()")

Reviewed-by: Alen Karnil alen.karnil@ideasonboard.com
Tested-by: Alen Karnil alen.karnil@ideasonboard.com
Reviewed-by: Kieran Bingham kieran.bingham@ideasonboard.com

Digital gain is used to adjust overall image exposure when the target
exposure cannot be achieved (for example, cannot go high enough). But
when the target exposure is lower than we can achieve, the digital
gain was being set to values less than unity, causing saturation
problems.

The fix is simply to clamp the digital gain at the bottom to 1.0,
resulting in images that, while "too bright", saturate correctly.

Fixes: 17f9912 ("ipa: rpi: agc: Calculate digital gain in process()")
Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Alen Karnil <alen.karnil@ideasonboard.com>
Tested-by: Alen Karnil <alen.karnil@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
@naushir
Copy link
Copy Markdown
Collaborator

naushir commented May 22, 2026

Looks reasonable. Should we merge?

@davidplowman
Copy link
Copy Markdown
Collaborator Author

I think so, it's in upstream libcamera already. Also perhaps this one, it's also been merged upstream?

@naushir
Copy link
Copy Markdown
Collaborator

naushir commented May 22, 2026

Perhaps I should merge the upstream tree in one-shot. Then we can close these.

@davidplowman
Copy link
Copy Markdown
Collaborator Author

Could do. We might also want to get Nick's fix, for the crash when camera_timeout_value_ms is non-zero.

@naushir
Copy link
Copy Markdown
Collaborator

naushir commented May 22, 2026

I don't see a PR for that, but I can add it later.

@naushir
Copy link
Copy Markdown
Collaborator

naushir commented May 22, 2026

Just running through some tests now on https://github.com/raspberrypi/libcamera/tree/next_merge

@naushir
Copy link
Copy Markdown
Collaborator

naushir commented May 22, 2026

I've now merged upstream to next, which includes this change.

@naushir naushir closed this May 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants