[libcamera-devel,v4,0/8] libcamera: Add helper for controls that take effect with a delay
mbox series

Message ID 20201215004811.602429-1-niklas.soderlund@ragnatech.se
Headers show
Series
  • libcamera: Add helper for controls that take effect with a delay
Related show

Message

Niklas Söderlund Dec. 15, 2020, 12:48 a.m. UTC 
Hello,

This series adds a new helper class to libcamera-core based on the
StaggerdCtrl principle from the Raspberry Pi pipeline handler. The new
helper matches perfectly the RPi implementation and can therefore
replace the pipeline specific implementation. There are slight changes
in the API of the two but noting preventing it to be a drop in
replacement.

The major new addition to the DelayedControls implementation is the
ability to queue controls ahead of time. This allows the concept of
pipeline depth we already have for buffers to be extended to controls.

Patch 1/8 and 2/8 adds the new core helper and its unit test. Patch 3/8
and 4/8 replaces StaggerdCtrl with DelayedControls in the Raspberry Pi
pipeline handler. Patch 5/8 adds an accessors to CameraSensor. And last 
6/8 - 8/8 make use of the new helper in the  RkISP1 pipeline to 
completely remove another local helper (Timeline).

The RkISP1 IPA feels much snappier after this change but this is
anecdotal and no real measurements have been done as the RkISP1 IPA at
this stage is neither advanced nor tuned.

Niklas Söderlund (8):
  libcamera: delayed_controls: Add helper for controls that applies with
    a delay
  test: delayed_controls: Add test case for DelayedControls
  libcamera: raspberrypi: Switch to DelayedControls
  libcamera: raspberrypi: Remove StaggeredCtrl
  libcamera: camera_sensor: Expose the camera device
  libcamera: pipeline: rkisp1: Use delayed controls
  libcamera: pipeline: rkisp1: Use SOF event to warn about late
    parameters
  libcamera: pipeline: rkisp1: Remove Timeline

 include/libcamera/internal/camera_sensor.h    |   2 +
 include/libcamera/internal/delayed_controls.h |  82 +++++
 src/libcamera/camera_sensor.cpp               |   6 +
 src/libcamera/delayed_controls.cpp            | 252 +++++++++++++++
 src/libcamera/meson.build                     |   1 +
 .../pipeline/raspberrypi/meson.build          |   1 -
 .../pipeline/raspberrypi/raspberrypi.cpp      |  54 ++--
 .../pipeline/raspberrypi/staggered_ctrl.cpp   | 174 ----------
 .../pipeline/raspberrypi/staggered_ctrl.h     |  96 ------
 src/libcamera/pipeline/rkisp1/meson.build     |   1 -
 src/libcamera/pipeline/rkisp1/rkisp1.cpp      | 166 +++-------
 src/libcamera/pipeline/rkisp1/timeline.cpp    | 227 -------------
 src/libcamera/pipeline/rkisp1/timeline.h      |  71 ----
 test/delayed_contols.cpp                      | 304 ++++++++++++++++++
 test/meson.build                              |   1 +
 15 files changed, 719 insertions(+), 719 deletions(-)
 create mode 100644 include/libcamera/internal/delayed_controls.h
 create mode 100644 src/libcamera/delayed_controls.cpp
 delete mode 100644 src/libcamera/pipeline/raspberrypi/staggered_ctrl.cpp
 delete mode 100644 src/libcamera/pipeline/raspberrypi/staggered_ctrl.h
 delete mode 100644 src/libcamera/pipeline/rkisp1/timeline.cpp
 delete mode 100644 src/libcamera/pipeline/rkisp1/timeline.h
 create mode 100644 test/delayed_contols.cpp

Comments

Naushir Patuck Jan. 8, 2021, 3:38 p.m. UTC | #1 
Hi Niklas,

Thank you for your patch.


On Tue, 15 Dec 2020 at 00:48, Niklas Söderlund <
niklas.soderlund@ragnatech.se> wrote:

> There are no users left, remove it.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>

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


> ---
>  .../pipeline/raspberrypi/meson.build          |   1 -
>  .../pipeline/raspberrypi/staggered_ctrl.cpp   | 174 ------------------
>  .../pipeline/raspberrypi/staggered_ctrl.h     |  96 ----------
>  3 files changed, 271 deletions(-)
>  delete mode 100644 src/libcamera/pipeline/raspberrypi/staggered_ctrl.cpp
>  delete mode 100644 src/libcamera/pipeline/raspberrypi/staggered_ctrl.h
>
> diff --git a/src/libcamera/pipeline/raspberrypi/meson.build
> b/src/libcamera/pipeline/raspberrypi/meson.build
> index 7c5b6ff73741c968..f1a2f5ee72c2a0dd 100644
> --- a/src/libcamera/pipeline/raspberrypi/meson.build
> +++ b/src/libcamera/pipeline/raspberrypi/meson.build
> @@ -4,5 +4,4 @@ libcamera_sources += files([
>      'dma_heaps.cpp',
>      'raspberrypi.cpp',
>      'rpi_stream.cpp',
> -    'staggered_ctrl.cpp',
>  ])
> diff --git a/src/libcamera/pipeline/raspberrypi/staggered_ctrl.cpp
> b/src/libcamera/pipeline/raspberrypi/staggered_ctrl.cpp
> deleted file mode 100644
> index 62605c0fceee7d11..0000000000000000
> --- a/src/libcamera/pipeline/raspberrypi/staggered_ctrl.cpp
> +++ /dev/null
> @@ -1,174 +0,0 @@
> -/* SPDX-License-Identifier: LGPL-2.1-or-later */
> -/*
> - * Copyright (C) 2020, Raspberry Pi (Trading) Ltd.
> - *
> - * staggered_ctrl.cpp - Helper for writing staggered ctrls to a V4L2
> device.
> - */
> -
> -#include "staggered_ctrl.h"
> -
> -#include <algorithm>
> -
> -#include <libcamera/controls.h>
> -
> -#include "libcamera/internal/log.h"
> -#include "libcamera/internal/utils.h"
> -#include "libcamera/internal/v4l2_videodevice.h"
> -
> -namespace libcamera {
> -
> -LOG_DEFINE_CATEGORY(RPI_S_W)
> -
> -namespace RPi {
> -
> -void StaggeredCtrl::init(V4L2VideoDevice *dev,
> -         std::initializer_list<std::pair<const uint32_t, uint8_t>>
> delayList)
> -{
> -       std::lock_guard<std::mutex> lock(lock_);
> -
> -       dev_ = dev;
> -       delay_ = delayList;
> -       ctrl_.clear();
> -
> -       /* Find the largest delay across all controls. */
> -       maxDelay_ = 0;
> -       for (auto const &p : delay_) {
> -               LOG(RPI_S_W, Info) << "Init ctrl "
> -                                  << utils::hex(p.first) << " with delay "
> -                                  << static_cast<int>(p.second);
> -               maxDelay_ = std::max(maxDelay_, p.second);
> -       }
> -
> -       init_ = true;
> -}
> -
> -void StaggeredCtrl::reset()
> -{
> -       std::lock_guard<std::mutex> lock(lock_);
> -
> -       int lastSetCount = setCount_;
> -       std::unordered_map<uint32_t, int32_t> lastVal;
> -
> -       /* Reset the counters. */
> -       setCount_ = getCount_ = 0;
> -
> -       /* Look for the last set values. */
> -       for (auto const &c : ctrl_)
> -               lastVal[c.first] = c.second[lastSetCount].value;
> -
> -       /* Apply the last set values as the next to be applied. */
> -       ctrl_.clear();
> -       for (auto &c : lastVal)
> -               ctrl_[c.first][setCount_] = CtrlInfo(c.second);
> -}
> -
> -bool StaggeredCtrl::set(uint32_t ctrl, int32_t value)
> -{
> -       std::lock_guard<std::mutex> lock(lock_);
> -
> -       /* Can we find this ctrl as one that is registered? */
> -       if (delay_.find(ctrl) == delay_.end())
> -               return false;
> -
> -       ctrl_[ctrl][setCount_].value = value;
> -       ctrl_[ctrl][setCount_].updated = true;
> -
> -       return true;
> -}
> -
> -bool StaggeredCtrl::set(std::initializer_list<std::pair<const uint32_t,
> int32_t>> ctrlList)
> -{
> -       std::lock_guard<std::mutex> lock(lock_);
> -
> -       for (auto const &p : ctrlList) {
> -               /* Can we find this ctrl? */
> -               if (delay_.find(p.first) == delay_.end())
> -                       return false;
> -
> -               ctrl_[p.first][setCount_] = CtrlInfo(p.second);
> -       }
> -
> -       return true;
> -}
> -
> -bool StaggeredCtrl::set(const ControlList &controls)
> -{
> -       std::lock_guard<std::mutex> lock(lock_);
> -
> -       for (auto const &p : controls) {
> -               /* Can we find this ctrl? */
> -               if (delay_.find(p.first) == delay_.end())
> -                       return false;
> -
> -               ctrl_[p.first][setCount_] =
> CtrlInfo(p.second.get<int32_t>());
> -               LOG(RPI_S_W, Debug) << "Setting ctrl "
> -                                   << utils::hex(p.first) << " to "
> -                                   << ctrl_[p.first][setCount_].value
> -                                   << " at index "
> -                                   << setCount_;
> -       }
> -
> -       return true;
> -}
> -
> -int StaggeredCtrl::write()
> -{
> -       std::lock_guard<std::mutex> lock(lock_);
> -       ControlList controls(dev_->controls());
> -
> -       for (auto &p : ctrl_) {
> -               int delayDiff = maxDelay_ - delay_[p.first];
> -               int index = std::max<int>(0, setCount_ - delayDiff);
> -
> -               if (p.second[index].updated) {
> -                       /* We need to write this value out. */
> -                       controls.set(p.first, p.second[index].value);
> -                       p.second[index].updated = false;
> -                       LOG(RPI_S_W, Debug) << "Writing ctrl "
> -                                           << utils::hex(p.first) << " to
> "
> -                                           << p.second[index].value
> -                                           << " at index "
> -                                           << index;
> -               }
> -       }
> -
> -       nextFrame();
> -       return dev_->setControls(&controls);
> -}
> -
> -void StaggeredCtrl::get(std::unordered_map<uint32_t, int32_t> &ctrl,
> uint8_t offset)
> -{
> -       std::lock_guard<std::mutex> lock(lock_);
> -
> -       /* Account for the offset to reset the getCounter. */
> -       getCount_ += offset + 1;
> -
> -       ctrl.clear();
> -       for (auto &p : ctrl_) {
> -               int index = std::max<int>(0, getCount_ - maxDelay_);
> -               ctrl[p.first] = p.second[index].value;
> -               LOG(RPI_S_W, Debug) << "Getting ctrl "
> -                                   << utils::hex(p.first) << " to "
> -                                   << p.second[index].value
> -                                   << " at index "
> -                                   << index;
> -       }
> -}
> -
> -void StaggeredCtrl::nextFrame()
> -{
> -       /* Advance the control history to the next frame */
> -       int prevCount = setCount_;
> -       setCount_++;
> -
> -       LOG(RPI_S_W, Debug) << "Next frame, set index is " << setCount_;
> -
> -       for (auto &p : ctrl_) {
> -               p.second[setCount_].value = p.second[prevCount].value;
> -               p.second[setCount_].updated = false;
> -       }
> -}
> -
> -} /* namespace RPi */
> -
> -} /* namespace libcamera */
> diff --git a/src/libcamera/pipeline/raspberrypi/staggered_ctrl.h
> b/src/libcamera/pipeline/raspberrypi/staggered_ctrl.h
> deleted file mode 100644
> index 382fa31a685357f2..0000000000000000
> --- a/src/libcamera/pipeline/raspberrypi/staggered_ctrl.h
> +++ /dev/null
> @@ -1,96 +0,0 @@
> -/* SPDX-License-Identifier: LGPL-2.1-or-later */
> -/*
> - * Copyright (C) 2020, Raspberry Pi (Trading) Ltd.
> - *
> - * staggered_ctrl.h - Helper for writing staggered ctrls to a V4L2 device.
> - */
> -#ifndef __LIBCAMERA_PIPELINE_RASPBERRYPI_STAGGERED_CTRL_H__
> -#define __LIBCAMERA_PIPELINE_RASPBERRYPI_STAGGERED_CTRL_H__
> -
> -#include <array>
> -#include <initializer_list>
> -#include <mutex>
> -#include <unordered_map>
> -#include <utility>
> -
> -namespace libcamera {
> -
> -class ControlList;
> -class V4L2VideoDevice;
> -
> -namespace RPi {
> -
> -class StaggeredCtrl
> -{
> -public:
> -       StaggeredCtrl()
> -               : init_(false), setCount_(0), getCount_(0), maxDelay_(0)
> -       {
> -       }
> -
> -       operator bool() const
> -       {
> -               return init_;
> -       }
> -
> -       void init(V4L2VideoDevice *dev,
> -                 std::initializer_list<std::pair<const uint32_t,
> uint8_t>> delayList);
> -       void reset();
> -
> -       void get(std::unordered_map<uint32_t, int32_t> &ctrl, uint8_t
> offset = 0);
> -
> -       bool set(uint32_t ctrl, int32_t value);
> -       bool set(std::initializer_list<std::pair<const uint32_t, int32_t>>
> ctrlList);
> -       bool set(const ControlList &controls);
> -
> -       int write();
> -
> -private:
> -       void nextFrame();
> -
> -       /* listSize must be a power of 2. */
> -       static constexpr int listSize = (1 << 4);
> -       struct CtrlInfo {
> -               CtrlInfo()
> -                       : value(0), updated(false)
> -               {
> -               }
> -
> -               CtrlInfo(int32_t value_)
> -                       : value(value_), updated(true)
> -               {
> -               }
> -
> -               int32_t value;
> -               bool updated;
> -       };
> -
> -       class CircularArray : public std::array<CtrlInfo, listSize>
> -       {
> -       public:
> -               CtrlInfo &operator[](int index)
> -               {
> -                       return std::array<CtrlInfo,
> listSize>::operator[](index & (listSize - 1));
> -               }
> -
> -               const CtrlInfo &operator[](int index) const
> -               {
> -                       return std::array<CtrlInfo,
> listSize>::operator[](index & (listSize - 1));
> -               }
> -       };
> -
> -       bool init_;
> -       uint32_t setCount_;
> -       uint32_t getCount_;
> -       uint8_t maxDelay_;
> -       V4L2VideoDevice *dev_;
> -       std::unordered_map<uint32_t, uint8_t> delay_;
> -       std::unordered_map<uint32_t, CircularArray> ctrl_;
> -       std::mutex lock_;
> -};
> -
> -} /* namespace RPi */
> -
> -} /* namespace libcamera */
> -
> -#endif /* __LIBCAMERA_PIPELINE_RASPBERRYPI_STAGGERED_CTRL_H__ */
> --
> 2.29.2
>
>