[PATCH v3 5/7] ipa: rpi: Add base classes and plumbing for sync algorithm

Naushir Patuck naush at raspberrypi.com
Fri Jan 17 10:15:13 CET 2025


Hi David,

On Thu, 9 Jan 2025 at 14:32, David Plowman
<david.plowman at raspberrypi.com> wrote:
>
> From: Naushir Patuck <naush at raspberrypi.com>
>
> We add a base class for a "sync algorithm", and define its inputs and
> outputs in the SyncStatus class.
>
> We add the necessary plumbing to the base IPA code so as to arrange
> for the necessary parameters to be made available to such an
> algorithm, and also to handle the return values, passing them back as
> necessary to the pipeline handler.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>

You probably want your SOB tag as well :)

Reviewed-by: Naushir Patuck <naush at raspberrypi.com>

> ---
>  src/ipa/rpi/common/ipa_base.cpp         | 78 +++++++++++++++++++++++--
>  src/ipa/rpi/common/ipa_base.h           |  4 +-
>  src/ipa/rpi/controller/sync_algorithm.h | 31 ++++++++++
>  src/ipa/rpi/controller/sync_status.h    | 27 +++++++++
>  4 files changed, 135 insertions(+), 5 deletions(-)
>  create mode 100644 src/ipa/rpi/controller/sync_algorithm.h
>  create mode 100644 src/ipa/rpi/controller/sync_status.h
>
> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
> index 6ff1e22b..f1e4a161 100644
> --- a/src/ipa/rpi/common/ipa_base.cpp
> +++ b/src/ipa/rpi/common/ipa_base.cpp
> @@ -28,6 +28,8 @@
>  #include "controller/lux_status.h"
>  #include "controller/sharpen_algorithm.h"
>  #include "controller/statistics.h"
> +#include "controller/sync_algorithm.h"
> +#include "controller/sync_status.h"
>
>  namespace libcamera {
>
> @@ -72,6 +74,8 @@ const ControlInfoMap::Map ipaControls{
>         { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },
>         { &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },
>         { &controls::FrameDurationLimits, ControlInfo(INT64_C(33333), INT64_C(120000)) },
> +       { &controls::rpi::SyncMode, ControlInfo(controls::rpi::SyncModeValues) },
> +       { &controls::rpi::SyncFrames, ControlInfo(1, 1000000, 100) },
>         { &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },
>         { &controls::rpi::StatsOutputEnable, ControlInfo(false, true, false) },
>  };
> @@ -390,6 +394,7 @@ void IpaBase::prepareIsp(const PrepareParams &params)
>
>         rpiMetadata.clear();
>         fillDeviceStatus(params.sensorControls, ipaContext);
> +       fillSyncParams(params, ipaContext);
>
>         if (params.buffers.embedded) {
>                 /*
> @@ -488,10 +493,23 @@ void IpaBase::processStats(const ProcessParams &params)
>                 helper_->process(statistics, rpiMetadata);
>                 controller_.process(statistics, &rpiMetadata);
>
> +               /* Send any sync algorithm outputs back to the pipeline handler */
> +               Duration offset(0s);
> +               struct SyncStatus syncStatus;
> +               if (rpiMetadata.get("sync.status", syncStatus) == 0) {
> +                       if (minFrameDuration_ != maxFrameDuration_)
> +                               LOG(IPARPI, Error) << "Sync algorithm enabled with variable framerate. " << minFrameDuration_ << " " << maxFrameDuration_;

Might need to wrap this line?

> +                       offset = syncStatus.frameDurationOffset;
> +
> +                       libcameraMetadata_.set(controls::rpi::SyncReady, syncStatus.ready);
> +                       if (syncStatus.timerKnown)
> +                               libcameraMetadata_.set(controls::rpi::SyncTimer, syncStatus.timerValue);
> +               }
> +
>                 struct AgcStatus agcStatus;
>                 if (rpiMetadata.get("agc.status", agcStatus) == 0) {
>                         ControlList ctrls(sensorCtrls_);
> -                       applyAGC(&agcStatus, ctrls);
> +                       applyAGC(&agcStatus, ctrls, offset);
>                         setDelayedControls.emit(ctrls, ipaContext);
>                         setCameraTimeoutValue();
>                 }
> @@ -728,6 +746,7 @@ void IpaBase::applyControls(const ControlList &controls)
>         using RPiController::ContrastAlgorithm;
>         using RPiController::DenoiseAlgorithm;
>         using RPiController::HdrAlgorithm;
> +       using RPiController::SyncAlgorithm;
>
>         /* Clear the return metadata buffer. */
>         libcameraMetadata_.clear();
> @@ -1274,6 +1293,35 @@ void IpaBase::applyControls(const ControlList &controls)
>                         statsMetadataOutput_ = ctrl.second.get<bool>();
>                         break;
>
> +               case controls::rpi::SYNC_MODE: {
> +                       SyncAlgorithm *sync = dynamic_cast<SyncAlgorithm *>(controller_.getAlgorithm("sync"));
> +
> +                       if (sync) {
> +                               int mode = ctrl.second.get<int32_t>();
> +                               SyncAlgorithm::Mode m = SyncAlgorithm::Mode::Off;
> +                               if (mode == controls::rpi::SyncModeServer) {
> +                                       m = SyncAlgorithm::Mode::Server;
> +                                       LOG(IPARPI, Info) << "Sync mode set to server";
> +                               } else if (mode == controls::rpi::SyncModeClient) {
> +                                       m = SyncAlgorithm::Mode::Client;
> +                                       LOG(IPARPI, Info) << "Sync mode set to client";
> +                               }
> +                               sync->setMode(m);
> +                       }
> +                       break;
> +               }
> +
> +               case controls::rpi::SYNC_FRAMES: {
> +                       SyncAlgorithm *sync = dynamic_cast<SyncAlgorithm *>(controller_.getAlgorithm("sync"));
> +
> +                       if (sync) {
> +                               int frames = ctrl.second.get<int32_t>();
> +                               if (frames > 0)
> +                                       sync->setReadyFrame(frames);
> +                       }
> +                       break;
> +               }
> +
>                 default:
>                         LOG(IPARPI, Warning)
>                                 << "Ctrl " << controls::controls.at(ctrl.first)->name()
> @@ -1310,6 +1358,19 @@ void IpaBase::fillDeviceStatus(const ControlList &sensorControls, unsigned int i
>         rpiMetadata_[ipaContext].set("device.status", deviceStatus);
>  }
>
> +void IpaBase::fillSyncParams(const PrepareParams &params, unsigned int ipaContext)
> +{
> +       RPiController::SyncAlgorithm *sync = dynamic_cast<RPiController::SyncAlgorithm *>(
> +               controller_.getAlgorithm("sync"));
> +       if (!sync)
> +               return;
> +
> +       SyncParams syncParams;
> +       syncParams.wallClock = *params.sensorControls.get(controls::FrameWallClock);
> +       syncParams.sensorTimestamp = *params.sensorControls.get(controls::SensorTimestamp);
> +       rpiMetadata_[ipaContext].set("sync.params", syncParams);
> +}
> +
>  void IpaBase::reportMetadata(unsigned int ipaContext)
>  {
>         RPiController::Metadata &rpiMetadata = rpiMetadata_[ipaContext];
> @@ -1478,14 +1539,22 @@ void IpaBase::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDu
>          * value possible.
>          */
>         Duration maxExposureTime = Duration::max();
> -       helper_->getBlanking(maxExposureTime, minFrameDuration_, maxFrameDuration_);
> +       auto [vblank, hblank] = helper_->getBlanking(maxExposureTime, minFrameDuration_, maxFrameDuration_);
>
>         RPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(
>                 controller_.getAlgorithm("agc"));
>         agc->setMaxExposureTime(maxExposureTime);
> +
> +       RPiController::SyncAlgorithm *sync = dynamic_cast<RPiController::SyncAlgorithm *>(
> +               controller_.getAlgorithm("sync"));
> +       if (sync) {
> +               Duration duration = (mode_.height + vblank) * ((mode_.width + hblank) * 1.0s / mode_.pixelRate);
> +               LOG(IPARPI, Debug) << "setting sync frame duration to  " << duration;
> +               sync->setFrameDuration(duration);
> +       }
>  }
>
> -void IpaBase::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)
> +void IpaBase::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls, Duration frameDurationOffset)
>  {
>         const int32_t minGainCode = helper_->gainCode(mode_.minAnalogueGain);
>         const int32_t maxGainCode = helper_->gainCode(mode_.maxAnalogueGain);
> @@ -1500,7 +1569,8 @@ void IpaBase::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)
>
>         /* getBlanking might clip exposure time to the fps limits. */
>         Duration exposure = agcStatus->exposureTime;
> -       auto [vblank, hblank] = helper_->getBlanking(exposure, minFrameDuration_, maxFrameDuration_);
> +       auto [vblank, hblank] = helper_->getBlanking(exposure, minFrameDuration_ - frameDurationOffset,
> +                                                    maxFrameDuration_ - frameDurationOffset);
>         int32_t exposureLines = helper_->exposureLines(exposure,
>                                                        helper_->hblankToLineLength(hblank));
>
> diff --git a/src/ipa/rpi/common/ipa_base.h b/src/ipa/rpi/common/ipa_base.h
> index 1a811beb..b53d0bfb 100644
> --- a/src/ipa/rpi/common/ipa_base.h
> +++ b/src/ipa/rpi/common/ipa_base.h
> @@ -95,9 +95,11 @@ private:
>         void applyControls(const ControlList &controls);
>         virtual void handleControls(const ControlList &controls) = 0;
>         void fillDeviceStatus(const ControlList &sensorControls, unsigned int ipaContext);
> +       void fillSyncParams(const PrepareParams &params, unsigned int ipaContext);
>         void reportMetadata(unsigned int ipaContext);
>         void applyFrameDurations(utils::Duration minFrameDuration, utils::Duration maxFrameDuration);
> -       void applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls);
> +       void applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls,
> +                     utils::Duration frameDurationOffset = utils::Duration(0));
>
>         std::map<unsigned int, MappedFrameBuffer> buffers_;
>
> diff --git a/src/ipa/rpi/controller/sync_algorithm.h b/src/ipa/rpi/controller/sync_algorithm.h
> new file mode 100644
> index 00000000..c242def6
> --- /dev/null
> +++ b/src/ipa/rpi/controller/sync_algorithm.h
> @@ -0,0 +1,31 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2024, Raspberry Pi Ltd
> + *
> + * sync_algorithm.h - Camera sync algorithm interface
> + */
> +#pragma once
> +
> +#include <libcamera/base/utils.h>
> +
> +#include "algorithm.h"
> +
> +namespace RPiController {
> +
> +class SyncAlgorithm : public Algorithm
> +{
> +public:
> +       enum class Mode {
> +               Off,
> +               Server,
> +               Client,
> +       };
> +
> +       SyncAlgorithm(Controller *controller)
> +               : Algorithm(controller) {}
> +       virtual void setFrameDuration(libcamera::utils::Duration frameDuration) = 0;
> +       virtual void setReadyFrame(unsigned int frame) = 0;
> +       virtual void setMode(Mode mode) = 0;
> +};
> +
> +} /* namespace RPiController */
> diff --git a/src/ipa/rpi/controller/sync_status.h b/src/ipa/rpi/controller/sync_status.h
> new file mode 100644
> index 00000000..10f97502
> --- /dev/null
> +++ b/src/ipa/rpi/controller/sync_status.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2024, Raspberry Pi Ltd
> + *
> + * sync_status.h - Sync algorithm params and status structures
> + */
> +#pragma once
> +
> +#include <libcamera/base/utils.h>
> +
> +struct SyncParams {
> +       /* Wall clock time for this frame */
> +       uint64_t wallClock;
> +       /* Kernel timestamp for this frame */
> +       uint64_t sensorTimestamp;
> +};
> +
> +struct SyncStatus {
> +       /* Frame length correction to apply */
> +       libcamera::utils::Duration frameDurationOffset;
> +       /* Whether the "ready time" has been reached */
> +       bool ready;
> +       /* Time until the "ready time" */
> +       int64_t timerValue;
> +       /* Whether timerValue is known (client has to wait for a server message) */
> +       bool timerKnown;
> +};
> --
> 2.39.5
>


More information about the libcamera-devel mailing list