summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNaushir Patuck <naush@raspberrypi.com>2021-03-04 08:17:22 +0000
committerKieran Bingham <kieran.bingham@ideasonboard.com>2021-03-12 14:12:32 +0000
commit96c0eb338e5e9168147538736b688601c00b87f9 (patch)
treec14b4bbb4865f6c77e36c77195fad5d54f92d750
parent6795ffe67f3318e474ee14141e5b394b0e5d2d85 (diff)
libcamera: delayed_controls: Add notion of priority write
If an exposure time change adjusts the vblanking limits, and we set both VBLANK and EXPOSURE controls through the VIDIOC_S_EXT_CTRLS ioctl, the latter may fail if the value is outside of the limits calculated by the old VBLANK value. This is a limitation in V4L2 and cannot be fixed by setting VBLANK before EXPOSURE in a single VIDIOC_S_EXT_CTRLS ioctl. The workaround here is to have the DelayedControls object mark the VBLANK control as "priority write", which then write VBLANK separately from (and ahead of) any other controls. This way, the sensor driver will update the EXPOSURE control with new limits before the new values is presented, and will thus be seen as valid. To support this, a new struct DelayedControls::ControlParams is used in the constructor to provide the control delay value as well as the priority write flag. Signed-off-by: Naushir Patuck <naush@raspberrypi.com> Tested-by: David Plowman <david.plowman@raspberrypi.com> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> Tested-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> [Kieran: Fix up trivial comments, merge conflicts] Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
-rw-r--r--include/libcamera/internal/delayed_controls.h9
-rw-r--r--src/libcamera/delayed_controls.cpp54
-rw-r--r--src/libcamera/pipeline/ipu3/ipu3.cpp8
-rw-r--r--src/libcamera/pipeline/raspberrypi/raspberrypi.cpp13
-rw-r--r--src/libcamera/pipeline/rkisp1/rkisp1.cpp8
-rw-r--r--test/delayed_contols.cpp20
6 files changed, 68 insertions, 44 deletions
diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h
index dc447a88..564d9f2e 100644
--- a/include/libcamera/internal/delayed_controls.h
+++ b/include/libcamera/internal/delayed_controls.h
@@ -19,8 +19,13 @@ class V4L2Device;
class DelayedControls
{
public:
+ struct ControlParams {
+ unsigned int delay;
+ bool priorityWrite;
+ };
+
DelayedControls(V4L2Device *device,
- const std::unordered_map<uint32_t, unsigned int> &delays);
+ const std::unordered_map<uint32_t, ControlParams> &controlParams);
void reset();
@@ -64,7 +69,7 @@ private:
V4L2Device *device_;
/* \todo Evaluate if we should index on ControlId * or unsigned int */
- std::unordered_map<const ControlId *, unsigned int> delays_;
+ std::unordered_map<const ControlId *, ControlParams> controlParams_;
unsigned int maxDelay_;
bool running_;
diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp
index ab1d4005..fb9e6c42 100644
--- a/src/libcamera/delayed_controls.cpp
+++ b/src/libcamera/delayed_controls.cpp
@@ -40,15 +40,19 @@ LOG_DEFINE_CATEGORY(DelayedControls)
/**
* \brief Construct a DelayedControls instance
* \param[in] device The V4L2 device the controls have to be applied to
- * \param[in] delays Map of the numerical V4L2 control ids to their associated
- * delays (in frames)
+ * \param[in] controlParams Map of the numerical V4L2 control ids to their
+ * associated control parameters.
*
- * Only controls specified in \a delays are handled. If it's desired to mix
- * delayed controls and controls that take effect immediately the immediate
- * controls must be listed in the \a delays map with a delay value of 0.
+ * The control parameters comprise of delays (in frames) and a priority write
+ * flag. If this flag is set, the relevant control is written separately from,
+ * and ahead of the rest of the batched controls.
+ *
+ * Only controls specified in \a controlParams are handled. If it's desired to
+ * mix delayed controls and controls that take effect immediately the immediate
+ * controls must be listed in the \a controlParams map with a delay value of 0.
*/
DelayedControls::DelayedControls(V4L2Device *device,
- const std::unordered_map<uint32_t, unsigned int> &delays)
+ const std::unordered_map<uint32_t, ControlParams> &controlParams)
: device_(device), maxDelay_(0)
{
const ControlInfoMap &controls = device_->controls();
@@ -57,12 +61,12 @@ DelayedControls::DelayedControls(V4L2Device *device,
* Create a map of control ids to delays for controls exposed by the
* device.
*/
- for (auto const &delay : delays) {
- auto it = controls.find(delay.first);
+ for (auto const &param : controlParams) {
+ auto it = controls.find(param.first);
if (it == controls.end()) {
LOG(DelayedControls, Error)
<< "Delay request for control id "
- << utils::hex(delay.first)
+ << utils::hex(param.first)
<< " but control is not exposed by device "
<< device_->deviceNode();
continue;
@@ -70,13 +74,14 @@ DelayedControls::DelayedControls(V4L2Device *device,
const ControlId *id = it->first;
- delays_[id] = delay.second;
+ controlParams_[id] = param.second;
LOG(DelayedControls, Debug)
- << "Set a delay of " << delays_[id]
+ << "Set a delay of " << controlParams_[id].delay
+ << " and priority write flag " << controlParams_[id].priorityWrite
<< " for " << id->name();
- maxDelay_ = std::max(maxDelay_, delays_[id]);
+ maxDelay_ = std::max(maxDelay_, controlParams_[id].delay);
}
reset();
@@ -97,8 +102,8 @@ void DelayedControls::reset()
/* Retrieve control as reported by the device. */
std::vector<uint32_t> ids;
- for (auto const &delay : delays_)
- ids.push_back(delay.first->id());
+ for (auto const &param : controlParams_)
+ ids.push_back(param.first->id());
ControlList controls = device_->getControls(ids);
@@ -140,7 +145,7 @@ bool DelayedControls::push(const ControlList &controls)
const ControlId *id = it->second;
- if (delays_.find(id) == delays_.end())
+ if (controlParams_.find(id) == controlParams_.end())
return false;
Info &info = values_[id][queueCount_];
@@ -220,12 +225,27 @@ void DelayedControls::applyControls(uint32_t sequence)
ControlList out(device_->controls());
for (const auto &ctrl : values_) {
const ControlId *id = ctrl.first;
- unsigned int delayDiff = maxDelay_ - delays_[id];
+ unsigned int delayDiff = maxDelay_ - controlParams_[id].delay;
unsigned int index = std::max<int>(0, writeCount_ - delayDiff);
const Info &info = ctrl.second[index];
if (info.updated) {
- out.set(id->id(), info);
+ if (controlParams_[id].priorityWrite) {
+ /*
+ * This control must be written now, it could
+ * affect validity of the other controls.
+ */
+ ControlList priority(device_->controls());
+ priority.set(id->id(), info);
+ device_->setControls(&priority);
+ } else {
+ /*
+ * Batch up the list of controls and write them
+ * at the end of the function.
+ */
+ out.set(id->id(), info);
+ }
+
LOG(DelayedControls, Debug)
<< "Setting " << id->name()
<< " to " << info.toString()
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 00da2bb2..2ea13ec9 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -1058,14 +1058,14 @@ int PipelineHandlerIPU3::registerCameras()
* a sensor database. For now use generic values taken from
* the Raspberry Pi and listed as 'generic values'.
*/
- std::unordered_map<uint32_t, unsigned int> delays = {
- { V4L2_CID_ANALOGUE_GAIN, 1 },
- { V4L2_CID_EXPOSURE, 2 },
+ std::unordered_map<uint32_t, DelayedControls::ControlParams> params = {
+ { V4L2_CID_ANALOGUE_GAIN, { 1, false } },
+ { V4L2_CID_EXPOSURE, { 2, false } },
};
data->delayedCtrls_ =
std::make_unique<DelayedControls>(cio2->sensor()->device(),
- delays);
+ params);
data->cio2_.frameStart().connect(data->delayedCtrls_.get(),
&DelayedControls::applyControls);
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index 0f01ce8f..41f1cbff 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -1260,16 +1260,15 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)
if (result.params & ipa::RPi::ConfigSensorParams) {
/*
* Setup our delayed control writer with the sensor default
- * gain and exposure delays.
+ * gain and exposure delays. Mark VBLANK for priority write.
*/
- std::unordered_map<uint32_t, unsigned int> delays = {
- { V4L2_CID_ANALOGUE_GAIN, result.sensorConfig.gainDelay },
- { V4L2_CID_EXPOSURE, result.sensorConfig.exposureDelay },
- { V4L2_CID_VBLANK, result.sensorConfig.vblankDelay }
+ std::unordered_map<uint32_t, DelayedControls::ControlParams> params = {
+ { V4L2_CID_ANALOGUE_GAIN, { result.sensorConfig.gainDelay, false } },
+ { V4L2_CID_EXPOSURE, { result.sensorConfig.exposureDelay, false } },
+ { V4L2_CID_VBLANK, { result.sensorConfig.vblankDelay, true } }
};
- delayedCtrls_ = std::make_unique<DelayedControls>(unicam_[Unicam::Image].dev(), delays);
-
+ delayedCtrls_ = std::make_unique<DelayedControls>(unicam_[Unicam::Image].dev(), params);
sensorMetadata_ = result.sensorConfig.sensorMetadata;
}
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 4376d720..03757327 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -945,14 +945,14 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
* a sensor database. For now use generic values taken from
* the Raspberry Pi and listed as generic values.
*/
- std::unordered_map<uint32_t, unsigned int> delays = {
- { V4L2_CID_ANALOGUE_GAIN, 1 },
- { V4L2_CID_EXPOSURE, 2 },
+ std::unordered_map<uint32_t, DelayedControls::ControlParams> params = {
+ { V4L2_CID_ANALOGUE_GAIN, { 1, false } },
+ { V4L2_CID_EXPOSURE, { 2, false } },
};
data->delayedCtrls_ =
std::make_unique<DelayedControls>(data->sensor_->device(),
- delays);
+ params);
isp_->frameStart.connect(data->delayedCtrls_.get(),
&DelayedControls::applyControls);
diff --git a/test/delayed_contols.cpp b/test/delayed_contols.cpp
index 50169b12..3855eb18 100644
--- a/test/delayed_contols.cpp
+++ b/test/delayed_contols.cpp
@@ -72,8 +72,8 @@ protected:
int singleControlNoDelay()
{
- std::unordered_map<uint32_t, unsigned int> delays = {
- { V4L2_CID_BRIGHTNESS, 0 },
+ std::unordered_map<uint32_t, DelayedControls::ControlParams> delays = {
+ { V4L2_CID_BRIGHTNESS, { 0, false } },
};
std::unique_ptr<DelayedControls> delayed =
std::make_unique<DelayedControls>(dev_.get(), delays);
@@ -109,8 +109,8 @@ protected:
int singleControlWithDelay()
{
- std::unordered_map<uint32_t, unsigned int> delays = {
- { V4L2_CID_BRIGHTNESS, 1 },
+ std::unordered_map<uint32_t, DelayedControls::ControlParams> delays = {
+ { V4L2_CID_BRIGHTNESS, { 1, false } },
};
std::unique_ptr<DelayedControls> delayed =
std::make_unique<DelayedControls>(dev_.get(), delays);
@@ -150,9 +150,9 @@ protected:
int dualControlsWithDelay(uint32_t startOffset)
{
- std::unordered_map<uint32_t, unsigned int> delays = {
- { V4L2_CID_BRIGHTNESS, 1 },
- { V4L2_CID_CONTRAST, 2 },
+ std::unordered_map<uint32_t, DelayedControls::ControlParams> delays = {
+ { V4L2_CID_BRIGHTNESS, { 1, false } },
+ { V4L2_CID_CONTRAST, { 2, false } },
};
std::unique_ptr<DelayedControls> delayed =
std::make_unique<DelayedControls>(dev_.get(), delays);
@@ -197,9 +197,9 @@ protected:
int dualControlsMultiQueue()
{
- std::unordered_map<uint32_t, unsigned int> delays = {
- { V4L2_CID_BRIGHTNESS, 1 },
- { V4L2_CID_CONTRAST, 2 },
+ std::unordered_map<uint32_t, DelayedControls::ControlParams> delays = {
+ { V4L2_CID_BRIGHTNESS, { 1, false } },
+ { V4L2_CID_CONTRAST, { 2, false } }
};
std::unique_ptr<DelayedControls> delayed =
std::make_unique<DelayedControls>(dev_.get(), delays);