[libcamera-devel] [PATCH v6 3/8] ipa: raspberrypi: Add an operator<< to struct DeviceStatus
Naushir Patuck
naush at raspberrypi.com
Mon Jul 12 10:59:34 CEST 2021
Hi Laurent,
Thank you for your review feedback.
On Sun, 11 Jul 2021 at 19:54, Laurent Pinchart <
laurent.pinchart at ideasonboard.com> wrote:
> Hi Naush,
>
> Thank you for the patch.
>
> On Fri, Jul 09, 2021 at 03:58:20PM +0100, Naushir Patuck wrote:
> > Add an operator<< overload to log all fields in DeviceStatus, and remove
> the
> > manual logging statements in the IPA and CamHelper.
> >
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
> > ---
> > src/ipa/raspberrypi/cam_helper.cpp | 5 +----
> > src/ipa/raspberrypi/controller/device_status.h | 15 +++++++++++++++
> > src/ipa/raspberrypi/raspberrypi.cpp | 5 +----
> > 3 files changed, 17 insertions(+), 8 deletions(-)
> >
> > diff --git a/src/ipa/raspberrypi/cam_helper.cpp
> b/src/ipa/raspberrypi/cam_helper.cpp
> > index e6d2258c66d7..1ec3f03e1aa3 100644
> > --- a/src/ipa/raspberrypi/cam_helper.cpp
> > +++ b/src/ipa/raspberrypi/cam_helper.cpp
> > @@ -188,10 +188,7 @@ void CamHelper::parseEmbeddedData(Span<const
> uint8_t> buffer,
> > deviceStatus.shutter_speed = parsedDeviceStatus.shutter_speed;
> > deviceStatus.analogue_gain = parsedDeviceStatus.analogue_gain;
> >
> > - LOG(IPARPI, Debug) << "Metadata updated - Exposure : "
> > - << deviceStatus.shutter_speed
> > - << " Gain : "
> > - << deviceStatus.analogue_gain;
> > + LOG(IPARPI, Debug) << "Metadata updated - " << deviceStatus;
> >
> > metadata.Set("device.status", deviceStatus);
> > }
> > diff --git a/src/ipa/raspberrypi/controller/device_status.h
> b/src/ipa/raspberrypi/controller/device_status.h
> > index 73df7ce228dd..e2511d19b96d 100644
> > --- a/src/ipa/raspberrypi/controller/device_status.h
> > +++ b/src/ipa/raspberrypi/controller/device_status.h
> > @@ -6,6 +6,8 @@
> > */
> > #pragma once
> >
> > +#include <iostream>
> > +
> > #include <libcamera/base/utils.h>
> >
> > /*
> > @@ -20,6 +22,19 @@ struct DeviceStatus {
> > {
> > }
> >
> > + friend std::ostream &operator<<(std::ostream &out, const
> DeviceStatus &d)
> > + {
> > + using namespace libcamera; /* for the Duration operator<<
> overload */
> > +
> > + out << "Exposure: " << d.shutter_speed
> > + << " Gain: " << d.analogue_gain
> > + << " Aperture: " << d.aperture
> > + << " Lens: " << d.lens_position
> > + << " Flash: " << d.flash_intensity;
> > +
> > + return out;
> > + }
>
> Does this function need to be inline ? Moving it to a new
> device_status.cpp file would avoid duplicating the implementation in
> multiple object files.
>
Not necessary to be inline, but it was small enough that I did not think it
would
matter.
I will send an update with this in its own cpp file. Will also address
your other
feedback comments in the next update.
Regards,
Naush
> > +
> > /* time shutter is open */
> > libcamera::utils::Duration shutter_speed;
> > double analogue_gain;
> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp
> b/src/ipa/raspberrypi/raspberrypi.cpp
> > index 4d09a84f6532..f51c970befb5 100644
> > --- a/src/ipa/raspberrypi/raspberrypi.cpp
> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> > @@ -1019,10 +1019,7 @@ void IPARPi::fillDeviceStatus(const ControlList
> &sensorControls)
> > deviceStatus.shutter_speed = helper_->Exposure(exposureLines);
> > deviceStatus.analogue_gain = helper_->Gain(gainCode);
> >
> > - LOG(IPARPI, Debug) << "Metadata - Exposure : "
> > - << deviceStatus.shutter_speed
> > - << " Gain : "
> > - << deviceStatus.analogue_gain;
> > + LOG(IPARPI, Debug) << "Metadata - " << deviceStatus;
> >
> > rpiMetadata_.Set("device.status", deviceStatus);
> > }
>
> --
> Regards,
>
> Laurent Pinchart
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210712/8e2d33f5/attachment.htm>
More information about the libcamera-devel
mailing list