| Message ID | 20200729093154.12296-5-david.plowman@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi David, Thank you for the patch. On Wed, Jul 29, 2020 at 10:31:53AM +0100, David Plowman wrote: > This commit updates ALSC (Auto Lens Shading Correction) to handle the > transform now passed in the SwitchMode method. > > The transform is applied by the sensor, so the statistics we get > already incorporates it, and the adpative algorithm is agnostic to > it. That leaves only the calibrated tables (assumed measured without > any user transform) that need to be flipped, and resample_cal_table - > where we resample the calibrated table anyway - is an easy place to do > it. > --- > src/ipa/raspberrypi/controller/rpi/alsc.cpp | 22 ++++++++++++++------- > src/ipa/raspberrypi/controller/rpi/alsc.hpp | 3 ++- > 2 files changed, 17 insertions(+), 8 deletions(-) > > diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.cpp b/src/ipa/raspberrypi/controller/rpi/alsc.cpp > index 5fac9dd..33c1a88 100644 > --- a/src/ipa/raspberrypi/controller/rpi/alsc.cpp > +++ b/src/ipa/raspberrypi/controller/rpi/alsc.cpp > @@ -151,8 +151,9 @@ static void get_cal_table(double ct, > std::vector<AlscCalibration> const &calibrations, > double cal_table[XY]); > static void resample_cal_table(double const cal_table_in[XY], > - CameraMode const &camera_mode, > - double cal_table_out[XY]); > + CameraMode const &camera_mode, > + Transform transform, > + double cal_table_out[XY]); :set tabstop=8 but you may not be using vim :-) > static void compensate_lambdas_for_cal(double const cal_table[XY], > double const old_lambdas[XY], > double new_lambdas[XY]); > @@ -187,6 +188,7 @@ void Alsc::SwitchMode(CameraMode const &camera_mode, Transform transform, Metada > // change, any effects should be transient, and if they're not transient > // enough, we'll revisit the question then. > camera_mode_ = camera_mode; > + transform_ = transform; > if (first_time_) { > // On the first time, arrange for something sensible in the > // initial tables. Construct the tables for some default colour > @@ -194,9 +196,9 @@ void Alsc::SwitchMode(CameraMode const &camera_mode, Transform transform, Metada > // adaptive algorithm. > double cal_table_r[XY], cal_table_b[XY], cal_table_tmp[XY]; > get_cal_table(4000, config_.calibrations_Cr, cal_table_tmp); > - resample_cal_table(cal_table_tmp, camera_mode_, cal_table_r); > + resample_cal_table(cal_table_tmp, camera_mode_, transform_, cal_table_r); > get_cal_table(4000, config_.calibrations_Cb, cal_table_tmp); > - resample_cal_table(cal_table_tmp, camera_mode_, cal_table_b); > + resample_cal_table(cal_table_tmp, camera_mode_, transform_, cal_table_b); > compensate_lambdas_for_cal(cal_table_r, lambda_r_, > async_lambda_r_); > compensate_lambdas_for_cal(cal_table_b, lambda_b_, > @@ -379,7 +381,9 @@ void get_cal_table(double ct, std::vector<AlscCalibration> const &calibrations, > } > > void resample_cal_table(double const cal_table_in[XY], > - CameraMode const &camera_mode, double cal_table_out[XY]) > + CameraMode const &camera_mode, > + Transform transform, > + double cal_table_out[XY]) > { > // Precalculate and cache the x sampling locations and phases to save > // recomputing them on every row. > @@ -395,6 +399,8 @@ void resample_cal_table(double const cal_table_in[XY], > xf[i] = x - x_lo[i]; > x_hi[i] = std::min(x_lo[i] + 1, X - 1); > x_lo[i] = std::max(x_lo[i], 0); > + if (transform.contains(Transform::hflip())) > + x_lo[i] = X - 1 - x_lo[i], x_hi[i] = X - 1 - x_hi[i]; if (transform.contains(Transform::hflip())) { x_lo[i] = X - 1 - x_lo[i]; x_hi[i] = X - 1 - x_hi[i]; } No need to hide the code :-) > } > // Now march over the output table generating the new values. > double scale_y = camera_mode.sensor_height / > @@ -407,6 +413,8 @@ void resample_cal_table(double const cal_table_in[XY], > double yf = y - y_lo; > int y_hi = std::min(y_lo + 1, Y - 1); > y_lo = std::max(y_lo, 0); > + if (transform.contains(Transform::vflip())) > + y_lo = Y - 1 - y_lo, y_hi = Y - 1 - y_hi; Same here. > double const *row_above = cal_table_in + X * y_lo; > double const *row_below = cal_table_in + X * y_hi; > for (int i = 0; i < X; i++) { > @@ -671,9 +679,9 @@ void Alsc::doAlsc() > // Fetch the new calibrations (if any) for this CT. Resample them in > // case the camera mode is not full-frame. > get_cal_table(ct_, config_.calibrations_Cr, cal_table_tmp); > - resample_cal_table(cal_table_tmp, async_camera_mode_, cal_table_r); > + resample_cal_table(cal_table_tmp, async_camera_mode_, transform_, cal_table_r); > get_cal_table(ct_, config_.calibrations_Cb, cal_table_tmp); > - resample_cal_table(cal_table_tmp, async_camera_mode_, cal_table_b); > + resample_cal_table(cal_table_tmp, async_camera_mode_, transform_, cal_table_b); > // You could print out the cal tables for this image here, if you're > // tuning the algorithm... > (void)print_cal_table; > diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.hpp b/src/ipa/raspberrypi/controller/rpi/alsc.hpp > index 9b54578..9001e8a 100644 > --- a/src/ipa/raspberrypi/controller/rpi/alsc.hpp > +++ b/src/ipa/raspberrypi/controller/rpi/alsc.hpp > @@ -60,7 +60,8 @@ private: > // configuration is read-only, and available to both threads > AlscConfig config_; > bool first_time_; > - std::atomic<CameraMode> camera_mode_; > + CameraMode camera_mode_; This seems unrelated. If you don't want to split it to a separate patch, could you please mention it in the commit message with a rationale ? > + libcamera::Transform transform_; > std::thread async_thread_; > void asyncFunc(); // asynchronous thread function > std::mutex mutex_;
diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.cpp b/src/ipa/raspberrypi/controller/rpi/alsc.cpp index 5fac9dd..33c1a88 100644 --- a/src/ipa/raspberrypi/controller/rpi/alsc.cpp +++ b/src/ipa/raspberrypi/controller/rpi/alsc.cpp @@ -151,8 +151,9 @@ static void get_cal_table(double ct, std::vector<AlscCalibration> const &calibrations, double cal_table[XY]); static void resample_cal_table(double const cal_table_in[XY], - CameraMode const &camera_mode, - double cal_table_out[XY]); + CameraMode const &camera_mode, + Transform transform, + double cal_table_out[XY]); static void compensate_lambdas_for_cal(double const cal_table[XY], double const old_lambdas[XY], double new_lambdas[XY]); @@ -187,6 +188,7 @@ void Alsc::SwitchMode(CameraMode const &camera_mode, Transform transform, Metada // change, any effects should be transient, and if they're not transient // enough, we'll revisit the question then. camera_mode_ = camera_mode; + transform_ = transform; if (first_time_) { // On the first time, arrange for something sensible in the // initial tables. Construct the tables for some default colour @@ -194,9 +196,9 @@ void Alsc::SwitchMode(CameraMode const &camera_mode, Transform transform, Metada // adaptive algorithm. double cal_table_r[XY], cal_table_b[XY], cal_table_tmp[XY]; get_cal_table(4000, config_.calibrations_Cr, cal_table_tmp); - resample_cal_table(cal_table_tmp, camera_mode_, cal_table_r); + resample_cal_table(cal_table_tmp, camera_mode_, transform_, cal_table_r); get_cal_table(4000, config_.calibrations_Cb, cal_table_tmp); - resample_cal_table(cal_table_tmp, camera_mode_, cal_table_b); + resample_cal_table(cal_table_tmp, camera_mode_, transform_, cal_table_b); compensate_lambdas_for_cal(cal_table_r, lambda_r_, async_lambda_r_); compensate_lambdas_for_cal(cal_table_b, lambda_b_, @@ -379,7 +381,9 @@ void get_cal_table(double ct, std::vector<AlscCalibration> const &calibrations, } void resample_cal_table(double const cal_table_in[XY], - CameraMode const &camera_mode, double cal_table_out[XY]) + CameraMode const &camera_mode, + Transform transform, + double cal_table_out[XY]) { // Precalculate and cache the x sampling locations and phases to save // recomputing them on every row. @@ -395,6 +399,8 @@ void resample_cal_table(double const cal_table_in[XY], xf[i] = x - x_lo[i]; x_hi[i] = std::min(x_lo[i] + 1, X - 1); x_lo[i] = std::max(x_lo[i], 0); + if (transform.contains(Transform::hflip())) + x_lo[i] = X - 1 - x_lo[i], x_hi[i] = X - 1 - x_hi[i]; } // Now march over the output table generating the new values. double scale_y = camera_mode.sensor_height / @@ -407,6 +413,8 @@ void resample_cal_table(double const cal_table_in[XY], double yf = y - y_lo; int y_hi = std::min(y_lo + 1, Y - 1); y_lo = std::max(y_lo, 0); + if (transform.contains(Transform::vflip())) + y_lo = Y - 1 - y_lo, y_hi = Y - 1 - y_hi; double const *row_above = cal_table_in + X * y_lo; double const *row_below = cal_table_in + X * y_hi; for (int i = 0; i < X; i++) { @@ -671,9 +679,9 @@ void Alsc::doAlsc() // Fetch the new calibrations (if any) for this CT. Resample them in // case the camera mode is not full-frame. get_cal_table(ct_, config_.calibrations_Cr, cal_table_tmp); - resample_cal_table(cal_table_tmp, async_camera_mode_, cal_table_r); + resample_cal_table(cal_table_tmp, async_camera_mode_, transform_, cal_table_r); get_cal_table(ct_, config_.calibrations_Cb, cal_table_tmp); - resample_cal_table(cal_table_tmp, async_camera_mode_, cal_table_b); + resample_cal_table(cal_table_tmp, async_camera_mode_, transform_, cal_table_b); // You could print out the cal tables for this image here, if you're // tuning the algorithm... (void)print_cal_table; diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.hpp b/src/ipa/raspberrypi/controller/rpi/alsc.hpp index 9b54578..9001e8a 100644 --- a/src/ipa/raspberrypi/controller/rpi/alsc.hpp +++ b/src/ipa/raspberrypi/controller/rpi/alsc.hpp @@ -60,7 +60,8 @@ private: // configuration is read-only, and available to both threads AlscConfig config_; bool first_time_; - std::atomic<CameraMode> camera_mode_; + CameraMode camera_mode_; + libcamera::Transform transform_; std::thread async_thread_; void asyncFunc(); // asynchronous thread function std::mutex mutex_;