| Message ID | 20191008004534.1585142-10-niklas.soderlund@ragnatech.se |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
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@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@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hello, On Tue, Oct 08, 2019 at 01:21:52PM +0200, Jacopo Mondi wrote: > 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@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. There's certainly room for improvement, but for now I think the main focus should be on the IPAInterface. We'll refine the IPA implementation. This being said, buffer tracking will need to be moved to the pipeline handler as we remove the IPABuffer::type field. > > + > > + /* Camera sensor controls. */ > > + bool autoExposure_; > > + uint64_t exposure_; > > + uint64_t minExposure_; > > + uint64_t maxExposure_; > > + uint64_t gain_; > > + uint64_t minGain_; > > + uint64_t maxGain_; Shouldn't these be 32-bit integers as the controls are defined as 32-bit in V4L2 ? > > +}; > > + > > +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 }; As metadata are handled as controls, should we pass the aeState through controls instead of data ? > > + > > + 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] + '"') No need for a license here. > > +endforeach > > + > > Do you need this or could you just create the rkisp1 shared_module > outside of the for loop ? We could even already move the IPA to a subdirectory. > > config_h.set('IPA_MODULE_DIR', > > '"' + join_paths(get_option('prefix'), ipa_install_dir) + '"')
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_; + + /* 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(); + bufferFree_[buffer.type].push(buffer.id); + } +} + +void IPARkISP1::unmapBuffers(const std::vector<IPABuffer> &buffers) +{ + for (IPABuffer buffer : buffers) + bufferInfo_[buffer.type].erase(buffer.id); +} + +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()); + + 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; + for (int i = 0; i < CIFISP_AE_MEAN_MAX; i++) { + if (ae->exp_mean[i] > 15) { + value += ae->exp_mean[i]; + num++; + } + } + value /= num; + + double factor = (double)target / value; + + if (frame % 3 == 0) { + double tmp; + + tmp = factor * exposure_ * gain_ / minGain_; + exposure_ = utils::clamp<uint64_t>((uint64_t)tmp, minExposure_, maxExposure_); + + tmp = tmp / exposure_ * minGain_; + gain_ = utils::clamp<uint64_t>((uint64_t)tmp, minGain_, maxGain_); + + setControls(frame + 1); + } + + aeState = fabs(factor - 1.0f) < 0.05f ? 2 : 1; + } + + metadataReady(frame, aeState); +} + +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_); + 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 + config_h.set('IPA_MODULE_DIR', '"' + join_paths(get_option('prefix'), ipa_install_dir) + '"')
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@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