| Message ID | 20221006131744.5179-3-naush@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Naush, Thank you for the patch. On Thu, Oct 06, 2022 at 02:17:36PM +0100, Naushir Patuck via libcamera-devel wrote: > Add fields for minimum and maximum line length duration to the CameraMode > structure. This replaces the existing lineLength field. > > Any use of the existing lineLength field is replaced by the new minLineLength > field, as logically we always want to use the fastest sensor readout by default. > > As a drive-by cosmetic change, split all fields in the CameraMode structure into > separate lines. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > Tested-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > --- > src/ipa/raspberrypi/cam_helper.cpp | 8 +++---- > src/ipa/raspberrypi/controller/camera_mode.h | 23 +++++++++++++------- > src/ipa/raspberrypi/raspberrypi.cpp | 13 ++++++----- > 3 files changed, 26 insertions(+), 18 deletions(-) > > diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp > index cac8f39ee763..42251ba29682 100644 > --- a/src/ipa/raspberrypi/cam_helper.cpp > +++ b/src/ipa/raspberrypi/cam_helper.cpp > @@ -64,13 +64,13 @@ void CamHelper::process([[maybe_unused]] StatisticsPtr &stats, > uint32_t CamHelper::exposureLines(const Duration exposure) const > { > assert(initialized_); > - return exposure / mode_.lineLength; > + return exposure / mode_.minLineLength; > } > > Duration CamHelper::exposure(uint32_t exposureLines) const > { > assert(initialized_); > - return exposureLines * mode_.lineLength; > + return exposureLines * mode_.minLineLength; > } > > uint32_t CamHelper::getVBlanking(Duration &exposure, > @@ -86,8 +86,8 @@ uint32_t CamHelper::getVBlanking(Duration &exposure, > * minFrameDuration and maxFrameDuration are clamped by the caller > * based on the limits for the active sensor mode. > */ > - frameLengthMin = minFrameDuration / mode_.lineLength; > - frameLengthMax = maxFrameDuration / mode_.lineLength; > + frameLengthMin = minFrameDuration / mode_.minLineLength; > + frameLengthMax = maxFrameDuration / mode_.minLineLength; > > /* > * Limit the exposure to the maximum frame duration requested, and > diff --git a/src/ipa/raspberrypi/controller/camera_mode.h b/src/ipa/raspberrypi/controller/camera_mode.h > index a6ccf8c1c600..6bc35b771946 100644 > --- a/src/ipa/raspberrypi/controller/camera_mode.h > +++ b/src/ipa/raspberrypi/controller/camera_mode.h > @@ -20,23 +20,30 @@ struct CameraMode { > /* bit depth of the raw camera output */ > uint32_t bitdepth; > /* size in pixels of frames in this mode */ > - uint16_t width, height; > + uint16_t width; > + uint16_t height; > /* size of full resolution uncropped frame ("sensor frame") */ > - uint16_t sensorWidth, sensorHeight; > + uint16_t sensorWidth; > + uint16_t sensorHeight; > /* binning factor (1 = no binning, 2 = 2-pixel binning etc.) */ > - uint8_t binX, binY; > + uint8_t binX; > + uint8_t binY; > /* location of top left pixel in the sensor frame */ > - uint16_t cropX, cropY; > + uint16_t cropX; > + uint16_t cropY; > /* scaling factor (so if uncropped, width*scaleX is sensorWidth) */ > - double scaleX, scaleY; > + double scaleX; > + double scaleY; > /* scaling of the noise compared to the native sensor mode */ > double noiseFactor; > - /* line time */ > - libcamera::utils::Duration lineLength; > + /* minimum and maximum line time */ > + libcamera::utils::Duration minLineLength; > + libcamera::utils::Duration maxLineLength; > /* any camera transform *not* reflected already in the camera tuning */ > libcamera::Transform transform; > /* minimum and maximum fame lengths in units of lines */ s/fame/frame/ as another drive-by fix. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> No need to resubmit for just this. > - uint32_t minFrameLength, maxFrameLength; > + uint32_t minFrameLength; > + uint32_t maxFrameLength; > /* sensitivity of this mode */ > double sensitivity; > }; > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp > index 358a119da222..67326bcf4a14 100644 > --- a/src/ipa/raspberrypi/raspberrypi.cpp > +++ b/src/ipa/raspberrypi/raspberrypi.cpp > @@ -314,7 +314,7 @@ void IPARPi::start(const ControlList &controls, StartConfig *startConfig) > } > > startConfig->dropFrameCount = dropFrameCount_; > - const Duration maxSensorFrameDuration = mode_.maxFrameLength * mode_.lineLength; > + const Duration maxSensorFrameDuration = mode_.maxFrameLength * mode_.minLineLength; > startConfig->maxSensorFrameLengthMs = maxSensorFrameDuration.get<std::milli>(); > > firstStart_ = false; > @@ -356,7 +356,8 @@ void IPARPi::setMode(const IPACameraSensorInfo &sensorInfo) > * Calculate the line length as the ratio between the line length in > * pixels and the pixel rate. > */ > - mode_.lineLength = sensorInfo.minLineLength * (1.0s / sensorInfo.pixelRate); > + mode_.minLineLength = sensorInfo.minLineLength * (1.0s / sensorInfo.pixelRate); > + mode_.maxLineLength = sensorInfo.maxLineLength * (1.0s / sensorInfo.pixelRate); > > /* > * Set the frame length limits for the mode to ensure exposure and > @@ -458,8 +459,8 @@ int IPARPi::configure(const IPACameraSensorInfo &sensorInfo, > * based on the current sensor mode. > */ > ControlInfoMap::Map ctrlMap = ipaControls; > - const Duration minSensorFrameDuration = mode_.minFrameLength * mode_.lineLength; > - const Duration maxSensorFrameDuration = mode_.maxFrameLength * mode_.lineLength; > + const Duration minSensorFrameDuration = mode_.minFrameLength * mode_.minLineLength; > + const Duration maxSensorFrameDuration = mode_.maxFrameLength * mode_.minLineLength; > ctrlMap[&controls::FrameDurationLimits] = > ControlInfo(static_cast<int64_t>(minSensorFrameDuration.get<std::micro>()), > static_cast<int64_t>(maxSensorFrameDuration.get<std::micro>())); > @@ -1150,8 +1151,8 @@ void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls) > > void IPARPi::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDuration) > { > - const Duration minSensorFrameDuration = mode_.minFrameLength * mode_.lineLength; > - const Duration maxSensorFrameDuration = mode_.maxFrameLength * mode_.lineLength; > + const Duration minSensorFrameDuration = mode_.minFrameLength * mode_.minLineLength; > + const Duration maxSensorFrameDuration = mode_.maxFrameLength * mode_.minLineLength; > > /* > * This will only be applied once AGC recalculations occur.
diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp index cac8f39ee763..42251ba29682 100644 --- a/src/ipa/raspberrypi/cam_helper.cpp +++ b/src/ipa/raspberrypi/cam_helper.cpp @@ -64,13 +64,13 @@ void CamHelper::process([[maybe_unused]] StatisticsPtr &stats, uint32_t CamHelper::exposureLines(const Duration exposure) const { assert(initialized_); - return exposure / mode_.lineLength; + return exposure / mode_.minLineLength; } Duration CamHelper::exposure(uint32_t exposureLines) const { assert(initialized_); - return exposureLines * mode_.lineLength; + return exposureLines * mode_.minLineLength; } uint32_t CamHelper::getVBlanking(Duration &exposure, @@ -86,8 +86,8 @@ uint32_t CamHelper::getVBlanking(Duration &exposure, * minFrameDuration and maxFrameDuration are clamped by the caller * based on the limits for the active sensor mode. */ - frameLengthMin = minFrameDuration / mode_.lineLength; - frameLengthMax = maxFrameDuration / mode_.lineLength; + frameLengthMin = minFrameDuration / mode_.minLineLength; + frameLengthMax = maxFrameDuration / mode_.minLineLength; /* * Limit the exposure to the maximum frame duration requested, and diff --git a/src/ipa/raspberrypi/controller/camera_mode.h b/src/ipa/raspberrypi/controller/camera_mode.h index a6ccf8c1c600..6bc35b771946 100644 --- a/src/ipa/raspberrypi/controller/camera_mode.h +++ b/src/ipa/raspberrypi/controller/camera_mode.h @@ -20,23 +20,30 @@ struct CameraMode { /* bit depth of the raw camera output */ uint32_t bitdepth; /* size in pixels of frames in this mode */ - uint16_t width, height; + uint16_t width; + uint16_t height; /* size of full resolution uncropped frame ("sensor frame") */ - uint16_t sensorWidth, sensorHeight; + uint16_t sensorWidth; + uint16_t sensorHeight; /* binning factor (1 = no binning, 2 = 2-pixel binning etc.) */ - uint8_t binX, binY; + uint8_t binX; + uint8_t binY; /* location of top left pixel in the sensor frame */ - uint16_t cropX, cropY; + uint16_t cropX; + uint16_t cropY; /* scaling factor (so if uncropped, width*scaleX is sensorWidth) */ - double scaleX, scaleY; + double scaleX; + double scaleY; /* scaling of the noise compared to the native sensor mode */ double noiseFactor; - /* line time */ - libcamera::utils::Duration lineLength; + /* minimum and maximum line time */ + libcamera::utils::Duration minLineLength; + libcamera::utils::Duration maxLineLength; /* any camera transform *not* reflected already in the camera tuning */ libcamera::Transform transform; /* minimum and maximum fame lengths in units of lines */ - uint32_t minFrameLength, maxFrameLength; + uint32_t minFrameLength; + uint32_t maxFrameLength; /* sensitivity of this mode */ double sensitivity; }; diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index 358a119da222..67326bcf4a14 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -314,7 +314,7 @@ void IPARPi::start(const ControlList &controls, StartConfig *startConfig) } startConfig->dropFrameCount = dropFrameCount_; - const Duration maxSensorFrameDuration = mode_.maxFrameLength * mode_.lineLength; + const Duration maxSensorFrameDuration = mode_.maxFrameLength * mode_.minLineLength; startConfig->maxSensorFrameLengthMs = maxSensorFrameDuration.get<std::milli>(); firstStart_ = false; @@ -356,7 +356,8 @@ void IPARPi::setMode(const IPACameraSensorInfo &sensorInfo) * Calculate the line length as the ratio between the line length in * pixels and the pixel rate. */ - mode_.lineLength = sensorInfo.minLineLength * (1.0s / sensorInfo.pixelRate); + mode_.minLineLength = sensorInfo.minLineLength * (1.0s / sensorInfo.pixelRate); + mode_.maxLineLength = sensorInfo.maxLineLength * (1.0s / sensorInfo.pixelRate); /* * Set the frame length limits for the mode to ensure exposure and @@ -458,8 +459,8 @@ int IPARPi::configure(const IPACameraSensorInfo &sensorInfo, * based on the current sensor mode. */ ControlInfoMap::Map ctrlMap = ipaControls; - const Duration minSensorFrameDuration = mode_.minFrameLength * mode_.lineLength; - const Duration maxSensorFrameDuration = mode_.maxFrameLength * mode_.lineLength; + const Duration minSensorFrameDuration = mode_.minFrameLength * mode_.minLineLength; + const Duration maxSensorFrameDuration = mode_.maxFrameLength * mode_.minLineLength; ctrlMap[&controls::FrameDurationLimits] = ControlInfo(static_cast<int64_t>(minSensorFrameDuration.get<std::micro>()), static_cast<int64_t>(maxSensorFrameDuration.get<std::micro>())); @@ -1150,8 +1151,8 @@ void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls) void IPARPi::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDuration) { - const Duration minSensorFrameDuration = mode_.minFrameLength * mode_.lineLength; - const Duration maxSensorFrameDuration = mode_.maxFrameLength * mode_.lineLength; + const Duration minSensorFrameDuration = mode_.minFrameLength * mode_.minLineLength; + const Duration maxSensorFrameDuration = mode_.maxFrameLength * mode_.minLineLength; /* * This will only be applied once AGC recalculations occur.