[libcamera-devel] [PATCH v3 15/16] cam: Implement OptMetadata

[Hirokazu Honda]

On Thu, Apr 22, 2021 at 2:42 PM Hirokazu Honda <hiroh at chromium.org> wrote:
>
> Hi Jacopo, thank you for the patch.
>
> On Thu, Apr 22, 2021 at 3:49 AM Niklas Söderlund
> <niklas.soderlund at ragnatech.se> wrote:
> >
> > Hi Jacopo,
> >
> > Thanks for your work.
> >
> > On 2021-04-21 18:03:18 +0200, Jacopo Mondi wrote:
> > > Implement support for the new '--metadata' option by printing the
> > > value of each metadata entry associated with a completed Request.
> > >
> > > As sample of the output, running on raspberry pi, looks like the
> > > following:
> > >
> > > 3050.205672 (30.01 fps) stream0 seq: 000033 bytesused: 720000
> > >       ScalerCrop = (0x2)/3280x2460
> > >       ExposureTime = 13969
> > >       AeLocked = true
> > >       DigitalGain = 1.000721
> > >       Lux = 771.204224
> > >       ColourGains = [ 1.561101, 1.629698 ]
> > >       ColourTemperature = 4289
> > >       SensorBlackLevels = [ 4096, 4096, 4096, 4096 ]
> > >       ColourCorrectionMatrix = [ 1.691066, -0.599756, -0.091317, -0.437452, 1.983766, -0.546314, -0.083429, -0.722407, 1.805836 ]
> > >       AnalogueGain = 2.000000
> > >       SensorTimestamp = 3050205672000
> > > 3050.238999 (30.01 fps) stream0 seq: 000034 bytesused: 720000
> > >       ScalerCrop = (0x2)/3280x2460
> > >       ExposureTime = 13969
> > >       AeLocked = true
> > >       DigitalGain = 1.000709
> > >       Lux = 771.232422
> > >       ColourGains = [ 1.560868, 1.630029 ]
> > >       ColourTemperature = 4289
> > >       SensorBlackLevels = [ 4096, 4096, 4096, 4096 ]
> > >       ColourCorrectionMatrix = [ 1.691081, -0.599726, -0.091362, -0.437497, 1.983627, -0.546130, -0.083420, -0.722523, 1.805943 ]
> > >       AnalogueGain = 2.000000
> > >       SensorTimestamp = 3050238999000
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> >
> > Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> >
>
> I would squash 14/16 and 15/16 as they are small enough.
>
> Reviewed-by: Hirokazu Honda <hiroh at chromium.org>
>
> > > ---
> > >  src/cam/capture.cpp | 15 ++++++++++++++-
> > >  src/cam/capture.h   |  1 +
> > >  2 files changed, 15 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
> > > index 7b55fc677022..7213abd934ec 100644
> > > --- a/src/cam/capture.cpp
> > > +++ b/src/cam/capture.cpp
> > > @@ -10,6 +10,8 @@
> > >  #include <limits.h>
> > >  #include <sstream>
> > >
> > > +#include <libcamera/control_ids.h>
> > > +
> > >  #include "capture.h"
> > >  #include "main.h"
> > >
> > > @@ -18,7 +20,8 @@ using namespace libcamera;
> > >  Capture::Capture(std::shared_ptr<Camera> camera, CameraConfiguration *config,
> > >                EventLoop *loop)
> > >       : camera_(camera), config_(config), writer_(nullptr), last_(0), loop_(loop),
> > > -       queueCount_(0), captureCount_(0), captureLimit_(0)
> > > +       queueCount_(0), captureCount_(0), captureLimit_(0),
> > > +       printMetadata_(false)
> > >  {
> > >  }
> > >
> > > @@ -29,6 +32,7 @@ int Capture::run(const OptionsParser::Options &options)
> > >       queueCount_ = 0;
> > >       captureCount_ = 0;
> > >       captureLimit_ = options[OptCapture].toInteger();
> > > +     printMetadata_ = options.isSet(OptMetadata);
> > >
> > >       if (!camera_) {
> > >               std::cout << "Can't capture without a camera" << std::endl;
> > > @@ -217,6 +221,15 @@ void Capture::processRequest(Request *request)
> > >
> > >       std::cout << info.str() << std::endl;
> > >
> > > +     if (printMetadata_) {
> > > +             const ControlList &requestMetadata = request->metadata();
> > > +             for (const auto ctrl : requestMetadata) {

This must be const auto &ctrl, or I got this error.

../src/cam/capture.cpp:226:19: error: loop variable 'ctrl' creates a
copy from type 'const std::pair<const unsigned int,
libcamera::ControlValue>' [-Werror,-Wrange-loop-construct]
                for (const auto ctrl : requestMetadata) {
                                ^
../src/cam/capture.cpp:226:8: note: use reference type 'const
std::pair<const unsigned int, libcamera::ControlValue> &' to prevent
copying
                for (const auto ctrl : requestMetadata) {
                     ^~~~~~~~~~~~~~~~~

-Hiro

> > > +                     const ControlId *id = controls::controls.at(ctrl.first);
> > > +                     std::cout << "\t" << id->name() << " = "
> > > +                               << ctrl.second.toString() << std::endl;
> > > +             }
> > > +     }
> > > +
> > >       captureCount_++;
> > >       if (captureLimit_ && captureCount_ >= captureLimit_) {
> > >               loop_->exit(0);
> > > diff --git a/src/cam/capture.h b/src/cam/capture.h
> > > index c7c9dc00d30f..59d138766b1e 100644
> > > --- a/src/cam/capture.h
> > > +++ b/src/cam/capture.h
> > > @@ -47,6 +47,7 @@ private:
> > >       unsigned int queueCount_;
> > >       unsigned int captureCount_;
> > >       unsigned int captureLimit_;
> > > +     bool printMetadata_;
> > >
> > >       std::vector<std::unique_ptr<libcamera::Request>> requests_;
> > >  };
> > > --
> > > 2.31.1
> > >
> > > _______________________________________________
> > > libcamera-devel mailing list
> > > libcamera-devel at lists.libcamera.org
> > > https://lists.libcamera.org/listinfo/libcamera-devel
> >
> > --
> > Regards,
> > Niklas Söderlund
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel