fix(exposure): avoid degenerate EV compensation when images lack exposure metadata - #2150
fix(exposure): avoid degenerate EV compensation when images lack exposure metadata#2150skachm wants to merge 3 commits into
Conversation
…sure metadata PrepareDenseScene writes AliceVision:EVComp = medianExposure / cameraExposure into each undistorted EXR, but getExposure() returns a -1 "no metadata" sentinel for views without shutter/fnumber (e.g. ffmpeg-extracted video frames). The division trusts that sentinel, so every such view gets a degenerate EVComp; with correctEV on, Texturing multiplies every texel by it and the atlas comes out black. When all views lack metadata there is an additional issue: the candidate list in getMedianCameraExposureSetting() is empty and cameraExposureList[size()/2] is read out of bounds, so the median is garbage. That makes the result nondeterministic (-inf vs 1 depending on heap contents), which with PrepareDenseScene's per-chunk parallelism produces the characteristic per-block split. Guard all three layers: - PrepareDenseScene: only divide when median and camera exposure are both strictly positive; otherwise write a neutral 1.0. - SfMData: return a default ExposureSetting when no view has usable metadata, instead of reading past the end of an empty vector. - fileIO loadImage: reject a non-finite/non-positive EVComp at read time and fall back to 1.0, like the existing "missing metadata" branch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces safety checks to handle missing or invalid exposure metadata, preventing out-of-bounds reads when the camera exposure list is empty, guarding against division by non-positive exposure values during compensation calculation, and resetting degenerate compensation factors to a neutral 1.0f. The review feedback highlights two important issues: first, in fileIO.cpp, skipping the exposure compensation block due to invalid values also skips the colorspace conversion, leaving the image in LINEAR colorspace instead of the requested one; second, in main_prepareDenseScene.cpp, the calculation of ev remains unguarded against non-positive cameraExposure values, which can result in NaN being written to the image metadata.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Before: After: |
…data Address review feedback: - fileIO loadImage: move colorconvert out of the branch chain so the missing and invalid EVComp paths also return the image in the requested colorspace, instead of leaking a LINEAR image to the cache. - main_prepareDenseScene: guard the EV computation as well, so a missing-exposure -1 sentinel no longer writes NaN (log2 of a negative) into AliceVision:EV. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ata sentinel Complements the per-image EV guard: when no view has exposure metadata the median exposure is the -1 sentinel, so log2(1/median) printed -nan(ind) in the "Median EV" info line. Guard it the same way with a 0.0 fallback. Log-only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
Problem
PrepareDenseScenewritesAliceVision:EVComp = medianExposure / cameraExposureinto each undistorted EXR, but
getExposure()returns a-1sentinel for viewswith no shutter/fnumber (e.g. video frames extracted with ffmpeg). The division
trusts that sentinel, so every such view gets a degenerate
EVComp. WithcorrectEVenabled,Texturingmultiplies every texel by it and the atlas comesout black.
When all views lack metadata there is an additional issue: the candidate list in
getMedianCameraExposureSetting()is empty andcameraExposureList[size()/2]isread out of bounds, so the median is garbage. That makes the result
nondeterministic —
EVComplands on-infor1depending on heap contents,which with PrepareDenseScene's per-chunk parallelism produces the characteristic
per-block split.
Fix (defense in depth)
main_prepareDenseScene.cpp: only compute the ratio when median and cameraexposure are both strictly positive; otherwise write a neutral
1.0.SfMData::getMedianCameraExposureSetting(): return a defaultExposureSettingwhen no view has usable metadata, instead of indexing an empty vector.
fileIO.cpp(loadImage): reject a non-finite / non-positiveEVCompat readtime and fall back to
1.0, mirroring the existing "missing metadata" branch.