[libcamera-devel] [PATCH] ipa: rpi: Add "focus" algorithm
Kieran Bingham
kieran.bingham at ideasonboard.com
Fri Jun 26 10:23:49 CEST 2020
Hi Naush,
On 26/06/2020 08:33, Naushir Patuck wrote:
> Hi Kieran,
>
> On Thu, 25 Jun 2020 at 17:27, Kieran Bingham
> <kieran.bingham at ideasonboard.com> wrote:
>>
>> Hi David, Naush,
>>
>> On 28/05/2020 15:51, Naushir Patuck wrote:
>>> From: David Plowman <david.plowman at raspberrypi.com>
>>>
>>> Adds FocusStatus to the image metadata, containing contrast measurements
>>> across the image. Optionally also prints a contrast measure to the
>>> console, to aid in manual adjustment of the lens. Note that it is not an
>>> actual auto-focus algorithm that can drive a lens!
>>>
>>> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
>>> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
>>> ---
>>> src/ipa/raspberrypi/controller/focus_status.h | 26 ++++++++++
>>> src/ipa/raspberrypi/controller/rpi/focus.cpp | 51 +++++++++++++++++++
>>> src/ipa/raspberrypi/controller/rpi/focus.hpp | 31 +++++++++++
>>> src/ipa/raspberrypi/data/imx477.json | 4 ++
>>> src/ipa/raspberrypi/meson.build | 1 +
>>> 5 files changed, 113 insertions(+)
>>> create mode 100644 src/ipa/raspberrypi/controller/focus_status.h
>>> create mode 100644 src/ipa/raspberrypi/controller/rpi/focus.cpp
>>> create mode 100644 src/ipa/raspberrypi/controller/rpi/focus.hpp
>>>
>>> diff --git a/src/ipa/raspberrypi/controller/focus_status.h b/src/ipa/raspberrypi/controller/focus_status.h
>>> new file mode 100644
>>> index 00000000..3ad88777
>>> --- /dev/null
>>> +++ b/src/ipa/raspberrypi/controller/focus_status.h
>>> @@ -0,0 +1,26 @@
>>> +/* SPDX-License-Identifier: BSD-2-Clause */
>>> +/*
>>> + * Copyright (C) 2020, Raspberry Pi (Trading) Limited
>>> + *
>>> + * focus_status.h - focus measurement status
>>> + */
>>> +#pragma once
>>> +
>>> +#include <linux/bcm2835-isp.h>
>>> +
>>> +// The focus algorithm should post the following structure into the image's
>>> +// "focus.status" metadata. Recall that it's only reporting focus (contrast)
>>> +// measurements, it's not driving any kind of auto-focus algorithm!
>>> +
>>> +#ifdef __cplusplus
>>> +extern "C" {
>>> +#endif
>>> +
>>> +struct FocusStatus {
>>> + int num;
>>> + uint32_t focus_measures[FOCUS_REGIONS];
>>> +};
>>> +
>>> +#ifdef __cplusplus
>>> +}
>>> +#endif
>>> diff --git a/src/ipa/raspberrypi/controller/rpi/focus.cpp b/src/ipa/raspberrypi/controller/rpi/focus.cpp
>>> new file mode 100644
>>> index 00000000..d6cdc4bf
>>> --- /dev/null
>>> +++ b/src/ipa/raspberrypi/controller/rpi/focus.cpp
>>> @@ -0,0 +1,51 @@
>>> +/* SPDX-License-Identifier: BSD-2-Clause */
>>> +/*
>>> + * Copyright (C) 2020, Raspberry Pi (Trading) Limited
>>> + *
>>> + * focus.cpp - focus algorithm
>>> + */
>>> +#include <stdint.h>
>>> +
>>> +#include "../focus_status.h"
>>> +#include "../logging.hpp"
>>> +#include "focus.hpp"
>>> +
>>> +using namespace RPi;
>>> +
>>> +#define NAME "rpi.focus"
>>> +
>>> +Focus::Focus(Controller *controller)
>>> + : Algorithm(controller)
>>> +{
>>> +}
>>> +
>>> +char const *Focus::Name() const
>>> +{
>>> + return NAME;
>>> +}
>>> +
>>> +void Focus::Read(boost::property_tree::ptree const ¶ms)
>>> +{
>>> + print_ = params.get<int>("print", 0);
>>> +}
>>> +
>>> +void Focus::Process(StatisticsPtr &stats, Metadata *image_metadata)
>>> +{
>>> + FocusStatus status;
>>> + int i;
>>> + for (i = 0; i < FOCUS_REGIONS; i++)
>>> + status.focus_measures[i] = stats->focus_stats[i].contrast_val[1][1] / 1000;
>>> + status.num = i;
>>> + image_metadata->Set("focus.status", status);
>>> + if (print_) {
>>> + uint32_t value = (status.focus_measures[5] + status.focus_measures[6]) / 10;
>>> + RPI_LOG("Focus contrast measure: " << value);
>>> + }
>>> +}
>>> +
>>> +/* Register algorithm with the system. */
>>> +static Algorithm *Create(Controller *controller)
>>> +{
>>> + return new Focus(controller);
>>> +}
>>> +static RegisterAlgorithm reg(NAME, &Create);
>>> diff --git a/src/ipa/raspberrypi/controller/rpi/focus.hpp b/src/ipa/raspberrypi/controller/rpi/focus.hpp
>>> new file mode 100644
>>> index 00000000..d53401f7
>>> --- /dev/null
>>> +++ b/src/ipa/raspberrypi/controller/rpi/focus.hpp
>>> @@ -0,0 +1,31 @@
>>> +/* SPDX-License-Identifier: BSD-2-Clause */
>>> +/*
>>> + * Copyright (C) 2020, Raspberry Pi (Trading) Limited
>>> + *
>>> + * focus.hpp - focus algorithm
>>> + */
>>> +#pragma once
>>> +
>>> +#include "../algorithm.hpp"
>>> +#include "../metadata.hpp"
>>> +
>>> +/*
>>> + * The "focus" algorithm. All it does it print out a version of the
>>> + * focus contrast measure; there is no actual auto-focus mechanism to
>>> + * control.
>>> + */
>>> +
>>> +namespace RPi {
>>> +
>>> +class Focus : public Algorithm
>>> +{
>>> +public:
>>> + Focus(Controller *controller);
>>> + char const *Name() const override;
>>> + void Read(boost::property_tree::ptree const ¶ms) override;
>>> + void Process(StatisticsPtr &stats, Metadata *image_metadata) override;
>>> +private:
>>> + bool print_;
>>
>> This introduces the following coverity warning:
>>
>>
>> ** CID 293456: Uninitialized members (UNINIT_CTOR)
>> /home/linuxembedded/iob/libcamera/libcamera-daily/src/ipa/raspberrypi/controller/rpi/focus.cpp:
>> 20 in RPi::Focus::Focus(RPi::Controller *)()
>>
>>
>> ________________________________________________________________________________________________________
>> *** CID 293456: Uninitialized members (UNINIT_CTOR)
>> /home/linuxembedded/iob/libcamera/libcamera-daily/src/ipa/raspberrypi/controller/rpi/focus.cpp:
>> 20 in RPi::Focus::Focus(RPi::Controller *)()
>> 14
>> 15 #define NAME "rpi.focus"
>> 16
>> 17 Focus::Focus(Controller *controller)
>> 18 : Algorithm(controller)
>> 19 {
>>>>> CID 293456: Uninitialized members (UNINIT_CTOR)
>>>>> Non-static class member "print_" is not initialized in this
>> constructor nor in any functions that it calls.
>> 20 }
>> 21
>> 22 char const *Focus::Name() const
>> 23 {
>> 24 return NAME;
>> 25 }
>>
>> If you supply a patch to fix, please add the tag:
>>
>> Reported-by: Coverity CID=293456
>>
>>
>> Alternatively, if you believe it's a false positive and should not be
>> fixed, let me know and I'll mark it as such to close the issue.
>>
>
> Thanks for the heads-up. This is not actually a problem, as
> Focus::Read() initialises print_ and is always called before
> focus::process(). Not surprisingly, Coverity did not spot that. I'll
> put in an explicit initialiser to fix the defect. I'm about to push a
> set of patches for focus, so I'll add the fix to that.
Indeed, coverity does pick up a few false positives, particularly around
these where it can't determine the execution order, so in those cases
I'll just close them if the submitter choses.
Coverity runs on a daily cron job, but only on the master branch, so
I'll just send these reports manually until I can figure out how to
automate it ;-)
If you'd like an account to see the results, either sign up at
https://scan.coverity.com/projects/libcamera
or let me know and I can manually 'invite'.
Ideally, we'll get some 'pre-integration' tests running so that we can
catch things before they get merged too, but baby steps for now ... ;-)
--
Kieran
More information about the libcamera-devel
mailing list