[libcamera-devel] [PATCH v4 2/2] ipa: raspberrypi: Use CamHelpers to generalise sensor embedded data parsing
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sat May 8 01:27:46 CEST 2021
Hi David,
Thank you for the patch.
On Fri, May 07, 2021 at 12:37:28PM +0100, David Plowman wrote:
> CamHelpers get virtual Prepare() and Process() methods, running just
> before and just after the ISP, just like Raspberry Pi Algorithms.
>
> The Prepare() method is able to parse the register dumps in embedded
> data buffers, and can be specialised to perform custom processing when
> necessary.
>
> The IPA itself only needs to call the new Prepare() and Process()
> methods. It must fill in the DeviceStatus from the controls first, in
> case the Prepare() method does not supply its own values.
>
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> Reviewed-by: Naushir Patuck <naush at raspberrypi.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> src/ipa/raspberrypi/cam_helper.cpp | 54 +++++++++++++++++++
> src/ipa/raspberrypi/cam_helper.hpp | 11 +++-
> src/ipa/raspberrypi/raspberrypi.cpp | 80 ++++++++++-------------------
> 3 files changed, 90 insertions(+), 55 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp
> index 0ae0baa0..09917f3c 100644
> --- a/src/ipa/raspberrypi/cam_helper.cpp
> +++ b/src/ipa/raspberrypi/cam_helper.cpp
> @@ -17,6 +17,11 @@
> #include "md_parser.hpp"
>
> using namespace RPiController;
> +using namespace libcamera;
> +
> +namespace libcamera {
> +LOG_DECLARE_CATEGORY(IPARPI)
> +}
>
> static std::map<std::string, CamHelperCreateFunc> cam_helpers;
>
> @@ -45,6 +50,17 @@ CamHelper::~CamHelper()
> delete parser_;
> }
>
> +void CamHelper::Prepare(Span<const uint8_t> buffer,
> + Metadata &metadata)
> +{
> + parseEmbeddedData(buffer, metadata);
> +}
> +
> +void CamHelper::Process([[maybe_unused]] StatisticsPtr &stats,
> + [[maybe_unused]] Metadata &metadata)
> +{
> +}
> +
> uint32_t CamHelper::ExposureLines(double exposure_us) const
> {
> assert(initialized_);
> @@ -139,6 +155,44 @@ unsigned int CamHelper::MistrustFramesModeSwitch() const
> return 0;
> }
>
> +void CamHelper::parseEmbeddedData(Span<const uint8_t> buffer,
> + Metadata &metadata)
> +{
> + if (buffer.empty())
> + return;
> +
> + uint32_t exposureLines, gainCode;
> +
> + if (parser_->Parse(buffer) != MdParser::Status::OK ||
> + parser_->GetExposureLines(exposureLines) != MdParser::Status::OK ||
> + parser_->GetGainCode(gainCode) != MdParser::Status::OK) {
> + LOG(IPARPI, Error) << "Embedded data buffer parsing failed";
> + return;
> + }
> +
> + /*
> + * Overwrite the exposure/gain values in the DeviceStatus, as
> + * we know better. Fetch it first in case any other fields were
> + * set meaningfully.
> + */
> + DeviceStatus deviceStatus;
> +
> + if (metadata.Get("device.status", deviceStatus) != 0) {
> + LOG(IPARPI, Error) << "DeviceStatus not found";
> + return;
> + }
> +
> + deviceStatus.shutter_speed = Exposure(exposureLines);
> + deviceStatus.analogue_gain = Gain(gainCode);
> +
> + LOG(IPARPI, Debug) << "Metadata updated - Exposure : "
> + << deviceStatus.shutter_speed
> + << " Gain : "
> + << deviceStatus.analogue_gain;
> +
> + metadata.Set("device.status", deviceStatus);
> +}
> +
> RegisterCamHelper::RegisterCamHelper(char const *cam_name,
> CamHelperCreateFunc create_func)
> {
> diff --git a/src/ipa/raspberrypi/cam_helper.hpp b/src/ipa/raspberrypi/cam_helper.hpp
> index c3ed5362..a52f3f0b 100644
> --- a/src/ipa/raspberrypi/cam_helper.hpp
> +++ b/src/ipa/raspberrypi/cam_helper.hpp
> @@ -8,7 +8,11 @@
>
> #include <string>
>
> +#include <libcamera/span.h>
> +
> #include "camera_mode.h"
> +#include "controller/controller.hpp"
> +#include "controller/metadata.hpp"
> #include "md_parser.hpp"
>
> #include "libcamera/internal/v4l2_videodevice.h"
> @@ -65,7 +69,9 @@ public:
> CamHelper(MdParser *parser, unsigned int frameIntegrationDiff);
> virtual ~CamHelper();
> void SetCameraMode(const CameraMode &mode);
> - MdParser &Parser() const { return *parser_; }
> + virtual void Prepare(libcamera::Span<const uint8_t> buffer,
> + Metadata &metadata);
> + virtual void Process(StatisticsPtr &stats, Metadata &metadata);
> uint32_t ExposureLines(double exposure_us) const;
> double Exposure(uint32_t exposure_lines) const; // in us
> virtual uint32_t GetVBlanking(double &exposure_us, double minFrameDuration,
> @@ -81,6 +87,9 @@ public:
> virtual unsigned int MistrustFramesModeSwitch() const;
>
> protected:
> + void parseEmbeddedData(libcamera::Span<const uint8_t> buffer,
> + Metadata &metadata);
> +
> MdParser *parser_;
> CameraMode mode_;
>
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index dad6395f..bb55f931 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -101,9 +101,7 @@ private:
> void returnEmbeddedBuffer(unsigned int bufferId);
> void prepareISP(const ipa::RPi::ISPConfig &data);
> void reportMetadata();
> - bool parseEmbeddedData(unsigned int bufferId, struct DeviceStatus &deviceStatus);
> - void fillDeviceStatus(uint32_t exposureLines, uint32_t gainCode,
> - struct DeviceStatus &deviceStatus);
> + void fillDeviceStatus(const ControlList &sensorControls);
> void processStats(unsigned int bufferId);
> void applyFrameDurations(double minFrameDuration, double maxFrameDuration);
> void applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls);
> @@ -894,35 +892,34 @@ void IPARPi::returnEmbeddedBuffer(unsigned int bufferId)
>
> void IPARPi::prepareISP(const ipa::RPi::ISPConfig &data)
> {
> - struct DeviceStatus deviceStatus = {};
> - bool success = false;
> + Span<uint8_t> embeddedBuffer;
> +
> + rpiMetadata_.Clear();
> +
> + fillDeviceStatus(data.controls);
>
> if (data.embeddedBufferPresent) {
> /*
> * Pipeline handler has supplied us with an embedded data buffer,
> - * so parse it and extract the exposure and gain.
> + * we must pass it to the CamHelper for parsing.
> */
> - success = parseEmbeddedData(data.embeddedBufferId, deviceStatus);
> -
> - /* Done with embedded data now, return to pipeline handler asap. */
> - returnEmbeddedBuffer(data.embeddedBufferId);
> + auto it = buffers_.find(data.embeddedBufferId);
> + ASSERT(it != buffers_.end());
> + embeddedBuffer = it->second.maps()[0];
> }
>
> - if (!success) {
> - /*
> - * Pipeline handler has not supplied an embedded data buffer,
> - * or embedded data buffer parsing has failed for some reason,
> - * so pull the exposure and gain values from the control list.
> - */
> - int32_t exposureLines = data.controls.get(V4L2_CID_EXPOSURE).get<int32_t>();
> - int32_t gainCode = data.controls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>();
> - fillDeviceStatus(exposureLines, gainCode, deviceStatus);
> - }
> + /*
> + * This may overwrite the DeviceStatus using values from the sensor
> + * metadata, and may also do additional custom processing.
> + */
> + helper_->Prepare(embeddedBuffer, rpiMetadata_);
> +
> + /* Done with embedded data now, return to pipeline handler asap. */
> + if (data.embeddedBufferPresent)
> + returnEmbeddedBuffer(data.embeddedBufferId);
>
> ControlList ctrls(ispCtrls_);
>
> - rpiMetadata_.Clear();
> - rpiMetadata_.Set("device.status", deviceStatus);
> controller_.Prepare(&rpiMetadata_);
>
> /* Lock the metadata buffer to avoid constant locks/unlocks. */
> @@ -972,41 +969,13 @@ void IPARPi::prepareISP(const ipa::RPi::ISPConfig &data)
> setIspControls.emit(ctrls);
> }
>
> -bool IPARPi::parseEmbeddedData(unsigned int bufferId, struct DeviceStatus &deviceStatus)
> +void IPARPi::fillDeviceStatus(const ControlList &sensorControls)
> {
> - auto it = buffers_.find(bufferId);
> - if (it == buffers_.end()) {
> - LOG(IPARPI, Error) << "Could not find embedded buffer!";
> - return false;
> - }
> -
> - Span<uint8_t> mem = it->second.maps()[0];
> - helper_->Parser().SetBufferSize(mem.size());
> - RPiController::MdParser::Status status = helper_->Parser().Parse(mem.data());
> - if (status != RPiController::MdParser::Status::OK) {
> - LOG(IPARPI, Error) << "Embedded Buffer parsing failed, error " << status;
> - return false;
> - } else {
> - uint32_t exposureLines, gainCode;
> - if (helper_->Parser().GetExposureLines(exposureLines) != RPiController::MdParser::Status::OK) {
> - LOG(IPARPI, Error) << "Exposure time failed";
> - return false;
> - }
> -
> - if (helper_->Parser().GetGainCode(gainCode) != RPiController::MdParser::Status::OK) {
> - LOG(IPARPI, Error) << "Gain failed";
> - return false;
> - }
> -
> - fillDeviceStatus(exposureLines, gainCode, deviceStatus);
> - }
> + DeviceStatus deviceStatus = {};
>
> - return true;
> -}
> + int32_t exposureLines = sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
> + int32_t gainCode = sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>();
>
> -void IPARPi::fillDeviceStatus(uint32_t exposureLines, uint32_t gainCode,
> - struct DeviceStatus &deviceStatus)
> -{
> deviceStatus.shutter_speed = helper_->Exposure(exposureLines);
> deviceStatus.analogue_gain = helper_->Gain(gainCode);
>
> @@ -1014,6 +983,8 @@ void IPARPi::fillDeviceStatus(uint32_t exposureLines, uint32_t gainCode,
> << deviceStatus.shutter_speed
> << " Gain : "
> << deviceStatus.analogue_gain;
> +
> + rpiMetadata_.Set("device.status", deviceStatus);
> }
>
> void IPARPi::processStats(unsigned int bufferId)
> @@ -1027,6 +998,7 @@ void IPARPi::processStats(unsigned int bufferId)
> Span<uint8_t> mem = it->second.maps()[0];
> bcm2835_isp_stats *stats = reinterpret_cast<bcm2835_isp_stats *>(mem.data());
> RPiController::StatisticsPtr statistics = std::make_shared<bcm2835_isp_stats>(*stats);
> + helper_->Process(statistics, rpiMetadata_);
> controller_.Process(statistics, &rpiMetadata_);
>
> struct AgcStatus agcStatus;
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list