[libcamera-devel] [PATCH 2/2] libcamera: pipeline: raspberrypi: Move StaggeredCtrl to libcamera namespace

Laurent Pinchart laurent.pinchart at ideasonboard.com
Fri May 15 03:43:23 CEST 2020


Hi Naush,

On Thu, May 14, 2020 at 06:01:11PM +0300, Laurent Pinchart wrote:
> On Thu, May 14, 2020 at 03:58:14PM +0100, Naushir Patuck wrote:
> > On Tue, 12 May 2020 at 01:39, Laurent Pinchart wrote:
> > >
> > > The StaggeredCtrl class, part of the Raspberry Pi pipeline handler, is
> > > part of libcamera. Move it to the libcamera namespace to simplify usage
> > > of libcamera APIs.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > ---
> > >  .../pipeline/raspberrypi/raspberrypi.cpp      |  2 +-
> > >  .../pipeline/raspberrypi/staggered_ctrl.cpp   | 25 ++++++++++---------
> > >  .../pipeline/raspberrypi/staggered_ctrl.h     | 13 ++++++----
> > >  3 files changed, 22 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > index 21a1d7f7cca3..41d1a522fa71 100644
> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > @@ -330,7 +330,7 @@ public:
> > >         std::vector<IPABuffer> ipaBuffers_;
> > >
> > >         /* VCSM allocation helper. */
> > > -       RPi::Vcsm vcsm_;
> > > +       ::RPi::Vcsm vcsm_;
> > 
> > I haven't tried it myself, but is this change of scope necessary?
> 
> I would have asked the same had I received such a patch :-) It is
> necessary, as the compiler complains otherwise, but I haven't
> investigated why. I was considering moving RPi::Vcsm to the libcamera
> namespace too, but given that the goal is to replace Vcsm with dmabuf, I
> ended up taking the easy path.

For reference:

[4/70] Compiling C++ object 'src/libcamera/4ab8042@@camera at sha/pipeline_raspberrypi_raspberrypi.cpp.o'.
FAILED: src/libcamera/4ab8042@@camera at sha/pipeline_raspberrypi_raspberrypi.cpp.o
clang++-8 -Isrc/libcamera/4ab8042@@camera at sha -Isrc/libcamera -I../../src/libcamera -Iinclude -I../../include -Isrc/libcamera/include -I../../src/libcamera/include -I../../include/android/hardware/libhardware/include/ -I../../include/android/metadata/ -I../../include/android/system/core/include -Iinclude/libcamera -Xclang -fcolor-diagnostics -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++14 -g -stdlib=libc++ -Wno-unused-parameter -include config.h -fPIC -pthread -MD -MQ 'src/libcamera/4ab8042@@camera at sha/pipeline_raspberrypi_raspberrypi.cpp.o' -MF 'src/libcamera/4ab8042@@camera at sha/pipeline_raspberrypi_raspberrypi.cpp.o.d' -o 'src/libcamera/4ab8042@@camera at sha/pipeline_raspberrypi_raspberrypi.cpp.o' -c ../../src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
../../src/libcamera/pipeline/raspberrypi/raspberrypi.cpp:333:2: error: no type named 'Vcsm' in namespace 'libcamera::RPi'; did you mean '::RPi::Vcsm'?
        RPi::Vcsm vcsm_;
        ^~~~~~~~~
        ::RPi::Vcsm
../../src/libcamera/pipeline/raspberrypi/vcsm.h:22:7: note: '::RPi::Vcsm' declared here
class Vcsm
      ^
1 error generated.
ninja: build stopped: subcommand failed.

There's a similar error with gcc 8.3.0.

Is the patch otherwise OK ?

> > >         void *lsTable_;
> > >
> > >         RPi::StaggeredCtrl staggeredCtrl_;
> > > diff --git a/src/libcamera/pipeline/raspberrypi/staggered_ctrl.cpp b/src/libcamera/pipeline/raspberrypi/staggered_ctrl.cpp
> > > index fbd87d3e791e..d431887ea137 100644
> > > --- a/src/libcamera/pipeline/raspberrypi/staggered_ctrl.cpp
> > > +++ b/src/libcamera/pipeline/raspberrypi/staggered_ctrl.cpp
> > > @@ -9,20 +9,19 @@
> > >
> > >  #include <algorithm>
> > >
> > > +#include <libcamera/controls.h>
> > > +
> > >  #include "log.h"
> > >  #include "utils.h"
> > > +#include "v4l2_videodevice.h"
> > >
> > > -/* For logging... */
> > > -using libcamera::LogCategory;
> > > -using libcamera::LogDebug;
> > > -using libcamera::LogInfo;
> > > -using libcamera::utils::hex;
> > > +namespace libcamera {
> > >
> > >  LOG_DEFINE_CATEGORY(RPI_S_W);
> > >
> > >  namespace RPi {
> > >
> > > -void StaggeredCtrl::init(libcamera::V4L2VideoDevice *dev,
> > > +void StaggeredCtrl::init(V4L2VideoDevice *dev,
> > >           std::initializer_list<std::pair<const uint32_t, uint8_t>> delayList)
> > >  {
> > >         std::lock_guard<std::mutex> lock(lock_);
> > > @@ -35,7 +34,7 @@ void StaggeredCtrl::init(libcamera::V4L2VideoDevice *dev,
> > >         maxDelay_ = 0;
> > >         for (auto const &p : delay_) {
> > >                 LOG(RPI_S_W, Info) << "Init ctrl "
> > > -                                  << hex(p.first) << " with delay "
> > > +                                  << utils::hex(p.first) << " with delay "
> > >                                    << static_cast<int>(p.second);
> > >                 maxDelay_ = std::max(maxDelay_, p.second);
> > >         }
> > > @@ -92,7 +91,7 @@ bool StaggeredCtrl::set(std::initializer_list<std::pair<const uint32_t, int32_t>
> > >         return true;
> > >  }
> > >
> > > -bool StaggeredCtrl::set(libcamera::ControlList &controls)
> > > +bool StaggeredCtrl::set(ControlList &controls)
> > >  {
> > >         std::lock_guard<std::mutex> lock(lock_);
> > >
> > > @@ -103,7 +102,7 @@ bool StaggeredCtrl::set(libcamera::ControlList &controls)
> > >
> > >                 ctrl_[p.first][setCount_] = CtrlInfo(p.second.get<int32_t>());
> > >                 LOG(RPI_S_W, Debug) << "Setting ctrl "
> > > -                                   << hex(p.first) << " to "
> > > +                                   << utils::hex(p.first) << " to "
> > >                                     << ctrl_[p.first][setCount_].value
> > >                                     << " at index "
> > >                                     << setCount_;
> > > @@ -115,7 +114,7 @@ bool StaggeredCtrl::set(libcamera::ControlList &controls)
> > >  int StaggeredCtrl::write()
> > >  {
> > >         std::lock_guard<std::mutex> lock(lock_);
> > > -       libcamera::ControlList controls(dev_->controls());
> > > +       ControlList controls(dev_->controls());
> > >
> > >         for (auto &p : ctrl_) {
> > >                 int delayDiff = maxDelay_ - delay_[p.first];
> > > @@ -126,7 +125,7 @@ int StaggeredCtrl::write()
> > >                         controls.set(p.first, p.second[index].value);
> > >                         p.second[index].updated = false;
> > >                         LOG(RPI_S_W, Debug) << "Writing ctrl "
> > > -                                           << hex(p.first) << " to "
> > > +                                           << utils::hex(p.first) << " to "
> > >                                             << p.second[index].value
> > >                                             << " at index "
> > >                                             << index;
> > > @@ -149,7 +148,7 @@ void StaggeredCtrl::get(std::unordered_map<uint32_t, int32_t> &ctrl, uint8_t off
> > >                 int index = std::max<int>(0, getCount_ - maxDelay_);
> > >                 ctrl[p.first] = p.second[index].value;
> > >                 LOG(RPI_S_W, Debug) << "Getting ctrl "
> > > -                                   << hex(p.first) << " to "
> > > +                                   << utils::hex(p.first) << " to "
> > >                                     << p.second[index].value
> > >                                     << " at index "
> > >                                     << index;
> > > @@ -171,3 +170,5 @@ void StaggeredCtrl::nextFrame()
> > >  }
> > >
> > >  } /* namespace RPi */
> > > +
> > > +} /* namespace libcamera */
> > > diff --git a/src/libcamera/pipeline/raspberrypi/staggered_ctrl.h b/src/libcamera/pipeline/raspberrypi/staggered_ctrl.h
> > > index c8f000a0b43c..eef16eaac235 100644
> > > --- a/src/libcamera/pipeline/raspberrypi/staggered_ctrl.h
> > > +++ b/src/libcamera/pipeline/raspberrypi/staggered_ctrl.h
> > > @@ -12,9 +12,10 @@
> > >  #include <unordered_map>
> > >  #include <utility>
> > >
> > > -#include <libcamera/controls.h>
> > > +namespace libcamera {
> > >
> > > -#include "v4l2_videodevice.h"
> > > +class ControlList;
> > > +class V4L2VideoDevice;
> > >
> > >  namespace RPi {
> > >
> > > @@ -31,7 +32,7 @@ public:
> > >                 return init_;
> > >         }
> > >
> > > -       void init(libcamera::V4L2VideoDevice *dev,
> > > +       void init(V4L2VideoDevice *dev,
> > >                   std::initializer_list<std::pair<const uint32_t, uint8_t>> delayList);
> > >         void reset();
> > >
> > > @@ -39,7 +40,7 @@ public:
> > >
> > >         bool set(uint32_t ctrl, int32_t value);
> > >         bool set(std::initializer_list<std::pair<const uint32_t, int32_t>> ctrlList);
> > > -       bool set(libcamera::ControlList &controls);
> > > +       bool set(ControlList &controls);
> > >
> > >         int write();
> > >
> > > @@ -81,10 +82,12 @@ private:
> > >         uint32_t setCount_;
> > >         uint32_t getCount_;
> > >         uint8_t maxDelay_;
> > > -       libcamera::V4L2VideoDevice *dev_;
> > > +       V4L2VideoDevice *dev_;
> > >         std::unordered_map<uint32_t, uint8_t> delay_;
> > >         std::unordered_map<uint32_t, CircularArray> ctrl_;
> > >         std::mutex lock_;
> > >  };
> > >
> > >  } /* namespace RPi */
> > > +
> > > +} /* namespace libcamera */

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list