[PATCH v4 06/18] libcamera: software_isp: Add SwStatsCpu class

[Milan Zamazal]

Hans de Goede <hdegoede at redhat.com> writes:

> Hi,
>
> On 3/4/24 4:05 PM, Milan Zamazal wrote:
>> Hans de Goede <hdegoede at redhat.com> writes:
>> 
>>> Add a CPU based SwStats implementation for SoftwareISP / SoftIPA use.
>>>
>>> This implementation offers a configure function + functions to gather
>>> statistics on a line by line basis. This allows CPU based software
>>> debayering to call into interlace debayering and statistics gathering
>>> on a line by line bases while the input data is still hot in the cache.
>>>
>>> This implementation also allows specifying a window over which to gather
>>> statistics instead of processing the whole frame.
>>>
>>> Doxygen documentation by Dennis Bonke.
>>>
>>> Tested-by: Bryan O'Donoghue <bryan.odonoghue at linaro.org> # sc8280xp Lenovo x13s
>>> Tested-by: Pavel Machek <pavel at ucw.cz>
>>> Reviewed-by: Pavel Machek <pavel at ucw.cz>
>>> Co-developed-by: Andrey Konovalov <andrey.konovalov at linaro.org>
>>> Signed-off-by: Andrey Konovalov <andrey.konovalov at linaro.org>
>>> Co-developed-by: Pavel Machek <pavel at ucw.cz>
>>> Signed-off-by: Pavel Machek <pavel at ucw.cz>
>>> Co-developed-by: Dennis Bonke <admin at dennisbonke.com>
>>> Signed-off-by: Dennis Bonke <admin at dennisbonke.com>
>>> Co-developed-by: Marttico <g.martti at gmail.com>
>>> Signed-off-by: Marttico <g.martti at gmail.com>
>>> Co-developed-by: Toon Langendam <t.langendam at gmail.com>
>>> Signed-off-by: Toon Langendam <t.langendam at gmail.com>
>>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>> 
>> One more minor typo in a docstring below, after it's fixed:
>> 
>> Reviewed-by: Milan Zamazal <mzamazal at redhat.com>
>> 
>>> ---
>>> Changes in v3:
>>> - Merge SwStats base-class into SwStatsCpu
>>> - Move swstats_cpu.h to src/libcamera/software_isp/
>>> - Move documentation to .cpp file and extend it
>>> - Rename a bunch of foo_bar symbols to fooBar
>>> - Change a couple of defines and hardcoded values to constexpr
>>> - Remove statsBayer10P in swstats_cpu.cpp, instead move the loop to
>>>   statsBGGR10PLine0 / statsGBRG10PLine0
>>> - Make startFrame() and finishFrame() normal methods instead of
>>>   using function pointers for these
>>> ---
>>>  include/libcamera/internal/meson.build        |   1 +
>>>  .../internal/software_isp/meson.build         |   5 +
>>>  .../internal/software_isp/swisp_stats.h       |  38 ++++
>>>  src/libcamera/meson.build                     |   1 +
>>>  src/libcamera/software_isp/meson.build        |   5 +
>>>  src/libcamera/software_isp/swstats_cpu.cpp    | 208 ++++++++++++++++++
>>>  src/libcamera/software_isp/swstats_cpu.h      | 159 +++++++++++++
>>>  7 files changed, 417 insertions(+)
>>>  create mode 100644 include/libcamera/internal/software_isp/meson.build
>>>  create mode 100644 include/libcamera/internal/software_isp/swisp_stats.h
>>>  create mode 100644 src/libcamera/software_isp/meson.build
>>>  create mode 100644 src/libcamera/software_isp/swstats_cpu.cpp
>>>  create mode 100644 src/libcamera/software_isp/swstats_cpu.h
>>>
>> 
>> [...]
>> 
>>> diff --git a/src/libcamera/software_isp/swstats_cpu.h b/src/libcamera/software_isp/swstats_cpu.h
>>> new file mode 100644
>>> index 00000000..df13263b
>>> --- /dev/null
>>> +++ b/src/libcamera/software_isp/swstats_cpu.h
>>> @@ -0,0 +1,159 @@
>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>>> +/*
>>> + * Copyright (C) 2023, Linaro Ltd
>>> + * Copyright (C) 2023, Red Hat Inc.
>>> + *
>>> + * Authors:
>>> + * Hans de Goede <hdegoede at redhat.com>
>>> + *
>>> + * swstats_cpu.h - CPU based software statistics implementation
>>> + */
>>> +
>>> +#pragma once
>>> +
>>> +#include <stdint.h>
>>> +
>>> +#include <libcamera/base/signal.h>
>>> +
>>> +#include <libcamera/geometry.h>
>>> +
>>> +#include "libcamera/internal/shared_mem_object.h"
>>> +#include "libcamera/internal/software_isp/swisp_stats.h"
>>> +
>>> +namespace libcamera {
>>> +
>>> +class PixelFormat;
>>> +struct StreamConfiguration;
>>> +
>>> +class SwStatsCpu
>>> +{
>>> +public:
>>> +	SwStatsCpu();
>>> +	~SwStatsCpu() = default;
>>> +
>>> +	/**
>>> +	 * \brief Gets wether the statistics object is valid.
>> 
>> wether -> weather
>
> Actually that should be whether, weather is talking about
> how sunny / cloudy it is. I'll do:

Absolutely!  Cool, typo in a typo fix :-).

I wonder how brain works when one wants to type an English word, the right one,
knows how to spell it and still the fingers type a similarly sounding different
word unnoticed.  I've observed this phenomenon also in e-mails of native
speakers so it cannot be explained simply as that writing in a foreign language
is more demanding.

> s/wether/whether/

This one looks correct. :-)

> for v5.
>
> Thanks & Regards,
>
> Hans
>
>
>
>
>
>
>> 
>> [...]
>>