[libcamera-devel] [PATCH v5 09/10] libcamera: ipa: rkisp1: Add basic control of auto exposure
Jacopo Mondi
jacopo at jmondi.org
Tue Oct 8 13:21:52 CEST 2019
Hi Niklas,
On Tue, Oct 08, 2019 at 02:45:33AM +0200, Niklas Söderlund wrote:
> Add an IPA which controls the exposure time and analog gain for a sensor
> connected to the rkisp1 pipeline. The IPA supports turning AE on and off
> and informing the camera of the status of the AE control loop.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> include/ipa/rkisp1.h | 23 ++++
> src/ipa/ipa_rkisp1.cpp | 281 +++++++++++++++++++++++++++++++++++++++++
> src/ipa/meson.build | 13 ++
> 3 files changed, 317 insertions(+)
> create mode 100644 include/ipa/rkisp1.h
> create mode 100644 src/ipa/ipa_rkisp1.cpp
>
> diff --git a/include/ipa/rkisp1.h b/include/ipa/rkisp1.h
> new file mode 100644
> index 0000000000000000..7db50afa6bef6e61
> --- /dev/null
> +++ b/include/ipa/rkisp1.h
> @@ -0,0 +1,23 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * rkisp1.h - Image Processing Algorithm interface for RkISP1
> + */
> +#ifndef __LIBCAMERA_IPA_INTERFACE_RKISP1_H__
> +#define __LIBCAMERA_IPA_INTERFACE_RKISP1_H__
> +
> +enum RkISP1BufferType {
> + RKISP1_BUFFER_PARAM = 1,
> + RKISP1_BUFFER_STAT = 2,
> +};
> +
> +enum RkISP1Operations {
> + RKISP1_IPA_ACTION_V4L2_SET = 1,
> + RKISP1_IPA_ACTION_QUEUE_BUFFER = 2,
> + RKISP1_IPA_ACTION_METADATA = 3,
> + RKISP1_IPA_EVENT_SIGNAL_BUFFER = 4,
> + RKISP1_IPA_EVENT_QUEUE_REQUEST = 5,
> +};
> +
> +#endif /* __LIBCAMERA_IPA_INTERFACE_RKISP1_H__ */
> diff --git a/src/ipa/ipa_rkisp1.cpp b/src/ipa/ipa_rkisp1.cpp
> new file mode 100644
> index 0000000000000000..0cc0772422e1ea51
> --- /dev/null
> +++ b/src/ipa/ipa_rkisp1.cpp
> @@ -0,0 +1,281 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * ipa_rkisp1.cpp - RkISP1 Image Processing Algorithms
> + */
> +
> +#include <algorithm>
> +#include <cstdint>
> +#include <math.h>
> +#include <queue>
> +#include <string.h>
> +
> +#include <linux/rkisp1-config.h>
> +
> +#include <ipa/ipa_interface.h>
> +#include <ipa/ipa_module_info.h>
> +#include <ipa/rkisp1.h>
> +#include <libcamera/buffer.h>
> +#include <libcamera/control_ids.h>
> +#include <libcamera/request.h>
> +
> +#include "log.h"
> +#include "utils.h"
> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(IPARkISP1)
> +
> +class IPARkISP1 : public IPAInterface
> +{
> +public:
> + int init() override { return 0; }
> +
> + void configure(const std::map<unsigned int, IPAStream> &streamConfig,
> + const std::map<unsigned int, V4L2ControlInfoMap> &entityControls) override;
> + void mapBuffers(const std::vector<IPABuffer> &buffers) override;
> + void unmapBuffers(const std::vector<IPABuffer> &buffers) override;
> + void processEvent(const IPAOperationData &event) override;
> +
> +private:
> + void signalBuffer(unsigned int type, unsigned int id);
> + void queueRequest(unsigned int frame, const ControlList &controls);
> + void updateStatistics(unsigned int frame, BufferMemory &statistics);
> +
> + void setControls(unsigned int frame);
> + void queueBuffer(unsigned int frame, unsigned int type,
> + unsigned int id);
> + void metadataReady(unsigned int frame, unsigned int aeState);
> +
> + std::map<unsigned int, std::map<unsigned int, BufferMemory>> bufferInfo_;
> + std::map<unsigned int, std::map<unsigned int, unsigned int>> bufferFrame_;
> + std::map<unsigned int, std::queue<unsigned int>> bufferFree_;
I really think this structures to keep track of free/available buffers
is very fragile. But for now we'll have to live with that.
> +
> + /* Camera sensor controls. */
> + bool autoExposure_;
> + uint64_t exposure_;
> + uint64_t minExposure_;
> + uint64_t maxExposure_;
> + uint64_t gain_;
> + uint64_t minGain_;
> + uint64_t maxGain_;
> +};
> +
> +void IPARkISP1::configure(const std::map<unsigned int, IPAStream> &streamConfig,
> + const std::map<unsigned int, V4L2ControlInfoMap> &entityControls)
> +{
> + if (entityControls.empty())
> + return;
> +
> + const V4L2ControlInfoMap &ctrls = entityControls.at(0);
> +
> + const auto itExp = ctrls.find(V4L2_CID_EXPOSURE);
> + if (itExp == ctrls.end()) {
> + LOG(IPARkISP1, Error) << "Can't find exposure control";
> + return;
> + }
> +
> + const auto itGain = ctrls.find(V4L2_CID_ANALOGUE_GAIN);
> + if (itGain == ctrls.end()) {
> + LOG(IPARkISP1, Error) << "Can't find gain control";
> + return;
> + }
> +
> + autoExposure_ = true;
> +
> + minExposure_ = std::max<uint64_t>(itExp->second.range().min().get<int32_t>(), 1);
> + maxExposure_ = itExp->second.range().max().get<int32_t>();
> + exposure_ = minExposure_;
> +
> + minGain_ = std::max<uint64_t>(itGain->second.range().min().get<int32_t>(), 1);
> + maxGain_ = itGain->second.range().max().get<int32_t>();
> + gain_ = minGain_;
> +
> + LOG(IPARkISP1, Info)
> + << "Exposure: " << minExposure_ << "-" << maxExposure_
> + << " Gain: " << minGain_ << "-" << maxGain_;
> +
> + setControls(0);
> +}
> +
> +void IPARkISP1::mapBuffers(const std::vector<IPABuffer> &buffers)
> +{
> + for (IPABuffer buffer : buffers) {
> + bufferInfo_[buffer.type][buffer.id] = buffer.buffer;
> + bufferInfo_[buffer.type][buffer.id].planes()[0].mem();
You don't need this unless you want to store the result of the memory
mapping somewhere at this point.
> + bufferFree_[buffer.type].push(buffer.id);
> + }
> +}
> +
> +void IPARkISP1::unmapBuffers(const std::vector<IPABuffer> &buffers)
> +{
> + for (IPABuffer buffer : buffers)
> + bufferInfo_[buffer.type].erase(buffer.id);
Should buffers be removed from bufferFree_ too ?
> +}
> +
> +void IPARkISP1::processEvent(const IPAOperationData &event)
> +{
> + switch (event.operation) {
> + case RKISP1_IPA_EVENT_SIGNAL_BUFFER:
> + signalBuffer(event.data[0], event.data[1]);
> + break;
> + case RKISP1_IPA_EVENT_QUEUE_REQUEST:
> + queueRequest(event.data[0], event.controls[0]);
> + break;
> + default:
> + LOG(IPARkISP1, Error) << "Unkown event " << event.operation;
> + break;
> + }
> +}
> +
> +void IPARkISP1::signalBuffer(unsigned int type, unsigned int id)
> +{
> + if (type == RKISP1_BUFFER_STAT) {
> + unsigned int frame = bufferFrame_[type][id];
> + BufferMemory &mem = bufferInfo_[type][id];
> + updateStatistics(frame, mem);
> + }
> +
> + bufferFree_[type].push(id);
> +}
> +
> +void IPARkISP1::queueRequest(unsigned int frame, const ControlList &controls)
> +{
> + /* Find buffers. */
> + if (bufferFree_[RKISP1_BUFFER_PARAM].empty()) {
> + LOG(IPARkISP1, Error) << "Param buffer underrun";
> + return;
> + }
> +
> + if (bufferFree_[RKISP1_BUFFER_STAT].empty()) {
> + LOG(IPARkISP1, Error) << "Statistics buffer underrun";
> + return;
> + }
> +
> + unsigned int paramid = bufferFree_[RKISP1_BUFFER_PARAM].front();
> + bufferFree_[RKISP1_BUFFER_PARAM].pop();
> + unsigned int statid = bufferFree_[RKISP1_BUFFER_STAT].front();
> + bufferFree_[RKISP1_BUFFER_STAT].pop();
> +
> + bufferFrame_[RKISP1_BUFFER_PARAM][paramid] = frame;
> + bufferFrame_[RKISP1_BUFFER_STAT][statid] = frame;
> +
> + /* Prepare parameters buffer. */
> + BufferMemory &mem = bufferInfo_[RKISP1_BUFFER_PARAM][paramid];
> + rkisp1_isp_params_cfg *params =
> + static_cast<rkisp1_isp_params_cfg *>(mem.planes()[0].mem());
Could you store in the buffers the mapped memory location instead of
going through BufferMemory
> +
> + memset(params, 0, sizeof(*params));
> +
> + /* Auto Exposure on/off. */
> + if (controls.contains(controls::AeEnable)) {
> + autoExposure_ = controls.get(controls::AeEnable);
> + if (autoExposure_)
> + params->module_ens = CIFISP_MODULE_AEC;
> +
> + params->module_en_update = CIFISP_MODULE_AEC;
> + }
> +
> + /* Queue buffers to pipeline. */
> + queueBuffer(frame, RKISP1_BUFFER_PARAM, paramid);
> + queueBuffer(frame, RKISP1_BUFFER_STAT, statid);
> +}
> +
> +void IPARkISP1::updateStatistics(unsigned int frame, BufferMemory &statistics)
> +{
> + const rkisp1_stat_buffer *stats =
> + static_cast<rkisp1_stat_buffer *>(statistics.planes()[0].mem());
> + const cifisp_stat *params = &stats->params;
> + unsigned int aeState = 0;
> +
> + if (stats->meas_type & CIFISP_STAT_AUTOEXP) {
> + const cifisp_ae_stat *ae = ¶ms->ae;
> +
> + const unsigned int target = 60;
> +
> + unsigned int value = 0;
> + unsigned int num = 0;
Why this empty lines ?
> + for (int i = 0; i < CIFISP_AE_MEAN_MAX; i++) {
> + if (ae->exp_mean[i] > 15) {
if (ae->exp_mean[i] < 15)
continue
?
> + value += ae->exp_mean[i];
> + num++;
> + }
> + }
> + value /= num;
> +
> + double factor = (double)target / value;
> +
> + if (frame % 3 == 0) {
Why every 3 frames ?
> + double tmp;
> +
> + tmp = factor * exposure_ * gain_ / minGain_;
> + exposure_ = utils::clamp<uint64_t>((uint64_t)tmp, minExposure_, maxExposure_);
this could be made to fit in 80cols
> +
> + tmp = tmp / exposure_ * minGain_;
> + gain_ = utils::clamp<uint64_t>((uint64_t)tmp, minGain_, maxGain_);
same here
> +
> + setControls(frame + 1);
> + }
> +
> + aeState = fabs(factor - 1.0f) < 0.05f ? 2 : 1;
> + }
> +
> + metadataReady(frame, aeState);
As a general comment, this goes:
PIPE IPA
dqbuf(Meta)
signalbuffer --->
Update Stats
metadataReady
queueFrameAction
<----
CompleteRequest
I wonder if the request could be completed while the metadata are
processed? Is it enough to make the metadata ControlList read only ?
> +}
> +
> +void IPARkISP1::setControls(unsigned int frame)
> +{
> + IPAOperationData op;
> + op.operation = RKISP1_IPA_ACTION_V4L2_SET;
> + op.data = { frame };
> +
> + V4L2ControlList ctrls;
> + ctrls.add(V4L2_CID_EXPOSURE);
> + ctrls.add(V4L2_CID_ANALOGUE_GAIN);
> + ctrls[V4L2_CID_EXPOSURE]->value().set<int32_t>(exposure_);
> + ctrls[V4L2_CID_ANALOGUE_GAIN]->value().set<int32_t>(gain_);
I think you could do
ctrl.add(V4L2_CID_.., value)
> + op.v4l2controls.push_back(ctrls);
> +
> + queueFrameAction.emit(op);
> +}
> +
> +void IPARkISP1::queueBuffer(unsigned int frame, unsigned int type,
> + unsigned int id)
> +{
> + IPAOperationData op;
> + op.operation = RKISP1_IPA_ACTION_QUEUE_BUFFER;
> + op.data = { frame, type, id };
> +
> + queueFrameAction.emit(op);
> +}
> +
> +void IPARkISP1::metadataReady(unsigned int frame, unsigned int aeState)
> +{
> + IPAOperationData op;
> + op.operation = RKISP1_IPA_ACTION_METADATA;
> + op.data = { frame, aeState };
> +
> + queueFrameAction.emit(op);
> +}
> +
> +/*
> + * External IPA module interface
> + */
> +
> +extern "C" {
> +const struct IPAModuleInfo ipaModuleInfo = {
> + IPA_MODULE_API_VERSION,
> + 1,
> + "PipelineHandlerRkISP1",
> + "RkISP1 IPA",
> + "LGPL-2.1-or-later",
> +};
> +
> +IPAInterface *ipaCreate()
> +{
> + return new IPARkISP1();
> +}
> +};
> +
> +}; /* namespace libcamera */
> diff --git a/src/ipa/meson.build b/src/ipa/meson.build
> index 10448c2ffc76af4b..eb5b852eec282735 100644
> --- a/src/ipa/meson.build
> +++ b/src/ipa/meson.build
> @@ -3,6 +3,10 @@ ipa_dummy_sources = [
> ['ipa_dummy_isolate', 'Proprietary'],
> ]
>
> +ipa_sources = [
> + ['ipa_rkisp1', 'ipa_rkisp1.cpp', 'LGPL-2.1-or-later'],
> +]
> +
> ipa_install_dir = join_paths(get_option('libdir'), 'libcamera')
>
> foreach t : ipa_dummy_sources
> @@ -14,5 +18,14 @@ foreach t : ipa_dummy_sources
> cpp_args : '-DLICENSE="' + t[1] + '"')
> endforeach
>
> +foreach t : ipa_sources
> + ipa = shared_module(t[0], t[1],
> + name_prefix : '',
> + include_directories : includes,
> + install : true,
> + install_dir : ipa_install_dir,
> + cpp_args : '-DLICENSE="' + t[2] + '"')
> +endforeach
> +
Do you need this or could you just create the rkisp1 shared_module
outside of the for loop ?
> config_h.set('IPA_MODULE_DIR',
> '"' + join_paths(get_option('prefix'), ipa_install_dir) + '"')
> --
> 2.23.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20191008/d6f6987f/attachment.sig>
More information about the libcamera-devel
mailing list