@@ -59,8 +59,7 @@ class IPASoftSimple : public ipa::soft::IPASoftInterface, public Module
public:
IPASoftSimple()
: params_(nullptr), stats_(nullptr),
- context_({ {}, {}, { kMaxFrameContexts } }),
- ignoreUpdates_(0)
+ context_({ {}, {}, { kMaxFrameContexts } })
{
}
@@ -98,7 +97,6 @@ private:
int32_t exposure_;
double againMin_, againMax_, againMinStep_;
double again_;
- unsigned int ignoreUpdates_;
};
IPASoftSimple::~IPASoftSimple()
@@ -298,16 +296,6 @@ void IPASoftSimple::processStats(const uint32_t frame,
/* \todo Switch to the libipa/algorithm.h API someday. */
- /*
- * AE / AGC, use 2 frames delay to make sure that the exposure and
- * the gain set have applied to the camera sensor.
- * \todo This could be handled better with DelayedControls.
- */
- if (ignoreUpdates_ > 0) {
- --ignoreUpdates_;
- return;
- }
-
/*
* Calculate Mean Sample Value (MSV) according to formula from:
* https://www.araa.asn.au/acra/acra2007/papers/paper84final.pdf
@@ -356,8 +344,6 @@ void IPASoftSimple::processStats(const uint32_t frame,
ctrls.set(V4L2_CID_ANALOGUE_GAIN,
static_cast<int32_t>(camHelper_ ? camHelper_->gainCode(again_) : again_));
- ignoreUpdates_ = 2;
-
setSensorControls.emit(ctrls);
LOG(IPASoft, Debug) << "exposureMSV " << exposureMSV
@@ -32,6 +32,7 @@
#include "libcamera/internal/camera.h"
#include "libcamera/internal/camera_sensor.h"
#include "libcamera/internal/converter.h"
+#include "libcamera/internal/delayed_controls.h"
#include "libcamera/internal/device_enumerator.h"
#include "libcamera/internal/media_device.h"
#include "libcamera/internal/pipeline_handler.h"
@@ -278,6 +279,8 @@ public:
std::vector<Configuration> configs_;
std::map<PixelFormat, std::vector<const Configuration *>> formats_;
+ std::unique_ptr<DelayedControls> delayedCtrls_;
+
std::vector<std::unique_ptr<FrameBuffer>> conversionBuffers_;
std::queue<std::map<const Stream *, FrameBuffer *>> conversionQueue_;
bool useConversion_;
@@ -900,14 +903,13 @@ void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)
void SimpleCameraData::ispStatsReady(uint32_t frame, uint32_t bufferId)
{
- /* \todo Use the DelayedControls class */
swIsp_->processStats(frame, bufferId,
- sensor_->getControls({ V4L2_CID_ANALOGUE_GAIN,
- V4L2_CID_EXPOSURE }));
+ delayedCtrls_->get(frame));
}
void SimpleCameraData::setSensorControls(const ControlList &sensorControls)
{
+ delayedCtrls_->push(sensorControls);
ControlList ctrls(sensorControls);
sensor_->setControls(&ctrls);
}
@@ -1288,6 +1290,16 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
if (outputCfgs.empty())
return 0;
+ std::unordered_map<uint32_t, DelayedControls::ControlParams> params = {
+ { V4L2_CID_ANALOGUE_GAIN, { 2, false } },
+ { V4L2_CID_EXPOSURE, { 2, false } },
+ };
+ data->delayedCtrls_ =
+ std::make_unique<DelayedControls>(data->sensor_->device(),
+ params);
+ data->video_->frameStart.connect(data->delayedCtrls_.get(),
+ &DelayedControls::applyControls);
+
StreamConfiguration inputCfg;
inputCfg.pixelFormat = pipeConfig->captureFormat;
inputCfg.size = pipeConfig->captureSize;
@@ -209,35 +209,6 @@ At some point, yes.
---
-11. Improve handling the sensor controls which take effect with a delay
-
-> void IPASoftSimple::processStats(const ControlList &sensorControls)
-> {
-> ...
-> /*
-> * AE / AGC, use 2 frames delay to make sure that the exposure and
-> * the gain set have applied to the camera sensor.
-> */
-> if (ignore_updates_ > 0) {
-> --ignore_updates_;
-> return;
-> }
-
-This could be handled better with DelayedControls.
-
----
-
-12. Use DelayedControls class in ispStatsReady()
-
-> void SimpleCameraData::ispStatsReady()
-> {
-> swIsp_->processStats(sensor_->getControls({ V4L2_CID_ANALOGUE_GAIN,
-> V4L2_CID_EXPOSURE }));
-
-You should use the DelayedControls class.
-
----
-
13. Improve black level and colour gains application
I think the black level should eventually be moved before debayering, and
Use the standard libcamera mechanism to report the "current" controls rather than delaying updates by counting from the last update. MY SPECULATION -- valid or not?: A problem is that with software ISP we cannot be sure about the sensor delay. The original implementation simply skips exposure updates for 2 frames, which should be enough in most cases. After this change, we assume the delay being exactly 2 frames, which may or may not be correct and may work with outdated values if the delay is shorter. This patch also prepares moving exposure+gain to an algorithm module where the original delay mechanism would be a (possibly unnecessary) complication. Resolves software ISP TODO #11 + #12. Signed-off-by: Milan Zamazal <mzamazal@redhat.com> --- src/ipa/simple/soft_simple.cpp | 16 +------------ src/libcamera/pipeline/simple/simple.cpp | 18 ++++++++++++--- src/libcamera/software_isp/TODO | 29 ------------------------ 3 files changed, 16 insertions(+), 47 deletions(-)