[libcamera-devel] [PATCH v4 0/5] Raspberry Pi: Generalise statistics

Laurent Pinchart laurent.pinchart at ideasonboard.com
Thu Feb 9 20:34:49 CET 2023


On Thu, Feb 09, 2023 at 03:07:54PM +0000, Naushir Patuck via libcamera-devel wrote:
> On Thu, 9 Feb 2023 at 14:20, Kieran Bingham wrote:
> > Quoting Naushir Patuck (2023-02-09 11:47:23)
> > > On Thu, 9 Feb 2023 at 11:38, Kieran Bingham via libcamera-devel wrote:
> > > > Quoting Kieran Bingham (2023-02-09 10:37:22)
> > > > > On 09/02/2023 08:31, Naushir Patuck via libcamera-devel wrote:
> > > > > > Hi all,
> > > > > >
> > > > > > If there are no other review comments or feedback, I think this is
> > > > > > ready to be merged.
> > > > >
> > > > > I'll apply this now.
> > > > >
> > > > > I wonder if it's worth using/trying pull requests when you have code
> > > > > that is explicitly ready for merge from Raspberry Pi.
> > > > >
> > > > >   - https://git-scm.com/docs/git-request-pull
> > >
> > > Interesting, I've never tried this before.  Will investigate exactly
> > > what it does.
> >
> > My integration scripts all work from a branch on a repository, so this
> > would help me quite a bit as I could just directly pull the code you
> > want to be merged.
> >
> > Speaking of which. This series is merged.
> 
> Thanks Kieran!
> 
> On the subject of git request-pull, what kind of process were you
> thinking of?  Do you think something like:
> 
> - Post patches for review to the ML as normal.
> - Once patches are reviewed and ready for submission, I create a
> branch on github.
> - Run git request-pull and copy/paste the output in an email to you?

That e-mail should go to the list. Kieran can tell you if he prefers
being CC'ed or if he will handle the pull request directly from the
list.

> - You have scripts that parse the output and do the right thing(TM).
> 
> might be useful?

I don't know if Kieran has such scripts, but the rest of the procedure
which details your tasks seems perfect to me. I'm sure we'll grow
scripts if we don't have them yet :-)

> > > > It failed compilation matrix I'm afraid:
> > >
> > > Argh. My clang-9 did not seem to complan.
> > > Looks to be a trivial fix (remove std::move).  I'll make the change
> > > and re-submit.
> > >
> > > > Configuration: clang-11:clang++-11 (/home/kbingham/iob/libcamera/ci/integrator/logs/build-matrix-clang-11.log)
> > > >
> > > > ...
> > > > [51/120] Compiling C++ object src/ipa/raspberrypi/ipa_rpi.so.p/raspberrypi.cpp.o
> > > > FAILED: src/ipa/raspberrypi/ipa_rpi.so.p/raspberrypi.cpp.o
> > > > clang++-11 -Isrc/ipa/raspberrypi/ipa_rpi.so.p -Isrc/ipa/raspberrypi -I../../../src/libcamera/src/ipa/raspberrypi -Iinclude -I../../../src/libcamera/include -Isrc/ipa -I../../../src/libcamera/src/ipa -I../../../src/libcamera/src/ipa/raspberrypi/controller -Iinclude/libcamera/ipa -Iinclude/libcamera -fcolor-diagnostics -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++17 -O0 -g -Wextra-semi -Wthread-safety -Wshadow -include /home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-11/config.h -Wno-c99-designator -fPIC -DLIBCAMERA_BASE_PRIVATE -MD -MQ src/ipa/raspberrypi/ipa_rpi.so.p/raspberrypi.cpp.o -MF src/ipa/raspberrypi/ipa_rpi.so.p/raspberrypi.cpp.o.d -o src/ipa/raspberrypi/ipa_rpi.so.p/raspberrypi.cpp.o -c ../../../src/libcamera/src/ipa/raspberrypi/raspberrypi.cpp
> > > > ../../../src/libcamera/src/ipa/raspberrypi/raspberrypi.cpp:1378:22: error: moving a temporary object prevents copy elision [-Werror,-Wpessimizing-move]
> > > >         statistics->yHist = std::move(RPiController::Histogram(stats->hist[0].g_hist, NUM_HISTOGRAM_BINS));
> > > >                             ^
> > > > ../../../src/libcamera/src/ipa/raspberrypi/raspberrypi.cpp:1378:22: note: remove std::move call here
> > > >         statistics->yHist = std::move(RPiController::Histogram(stats->hist[0].g_hist, NUM_HISTOGRAM_BINS));
> > > >                             ^~~~~~~~~~                                                                   ~
> > > > 1 error generated.
> > > > [52/120] Compiling C++ object src/ipa/raspberrypi/ipa_rpi.so.p/controller_rpi_dpc.cpp.o
> > > > [53/120] Compiling C++ object src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_imx477.cpp.o
> > > > [54/120] Compiling C++ object src/ipa/raspberrypi/ipa_rpi.so.p/controller_rpi_noise.cpp.o
> > > > [55/120] Compiling C++ object src/android/libcamera-hal.so.p/camera_capabilities.cpp.o
> > > > [56/120] Compiling C++ object src/ipa/raspberrypi/ipa_rpi.so.p/controller_pwl.cpp.o
> > > > [57/120] Compiling C++ object src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_imx519.cpp.o
> > > > [58/120] Compiling C++ object src/ipa/raspberrypi/ipa_rpi.so.p/controller_rpi_sdn.cpp.o
> > > > [59/120] Compiling C++ object src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_imx708.cpp.o
> > > > FAILED: src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_imx708.cpp.o
> > > > clang++-11 -Isrc/ipa/raspberrypi/ipa_rpi.so.p -Isrc/ipa/raspberrypi -I../../../src/libcamera/src/ipa/raspberrypi -Iinclude -I../../../src/libcamera/include -Isrc/ipa -I../../../src/libcamera/src/ipa -I../../../src/libcamera/src/ipa/raspberrypi/controller -Iinclude/libcamera/ipa -Iinclude/libcamera -fcolor-diagnostics -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++17 -O0 -g -Wextra-semi -Wthread-safety -Wshadow -include /home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-11/config.h -Wno-c99-designator -fPIC -DLIBCAMERA_BASE_PRIVATE -MD -MQ src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_imx708.cpp.o -MF src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_imx708.cpp.o.d -o src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_imx708.cpp.o -c ../../../src/libcamera/src/ipa/raspberrypi/cam_helper_imx708.cpp
> > > > ../../../src/libcamera/src/ipa/raspberrypi/cam_helper_imx708.cpp:315:18: error: moving a temporary object prevents copy elision [-Werror,-Wpessimizing-move]
> > > >         aeHistLinear_ = std::move(Histogram(hist, 128));
> > > >                         ^
> > > > ../../../src/libcamera/src/ipa/raspberrypi/cam_helper_imx708.cpp:315:18: note: remove std::move call here
> > > >         aeHistLinear_ = std::move(Histogram(hist, 128));
> > > >                         ^~~~~~~~~~                    ~
> > > > 1 error generated.
> > > > ...
> > > >
> > > > > > On Fri, 3 Feb 2023 at 09:17, Naushir Patuck <naush at raspberrypi.com> wrote:
> > > > > >>
> > > > > >> Hi,
> > > > > >>
> > > > > >> Version 5 rebases the changes on-top of master - particularly updating the
> > > > > >> breakages in cam_helper_imx708.cpp in patch 4/5.
> > > > > >>
> > > > > >> Regards,
> > > > > >> Naush
> > > > > >>
> > > > > >> Naushir Patuck (5):
> > > > > >>    ipa: raspberrypi: Generalise statistics
> > > > > >>    ipa: raspberrypi: histogram: Add a constructor for an empty histogram
> > > > > >>    ipa: raspberrypi: awb: Delay release of the statistics buffer
> > > > > >>    ipa: raspberrypi: Use the generic statistics structure in the
> > > > > >>      algorithms
> > > > > >>    ipa: raspberrypi: Normalise region sums to 16-bits
> > > > > >>
> > > > > >>   src/ipa/raspberrypi/cam_helper_imx708.cpp     |  26 ++--
> > > > > >>   src/ipa/raspberrypi/controller/controller.h   |   4 +-
> > > > > >>   src/ipa/raspberrypi/controller/histogram.h    |   5 +
> > > > > >>   src/ipa/raspberrypi/controller/region_stats.h | 123 ++++++++++++++++++
> > > > > >>   src/ipa/raspberrypi/controller/rpi/af.cpp     |  10 +-
> > > > > >>   src/ipa/raspberrypi/controller/rpi/af.h       |   8 +-
> > > > > >>   src/ipa/raspberrypi/controller/rpi/agc.cpp    |  32 ++---
> > > > > >>   src/ipa/raspberrypi/controller/rpi/agc.h      |   2 +-
> > > > > >>   src/ipa/raspberrypi/controller/rpi/alsc.cpp   |  32 ++---
> > > > > >>   src/ipa/raspberrypi/controller/rpi/alsc.h     |   3 +-
> > > > > >>   src/ipa/raspberrypi/controller/rpi/awb.cpp    |  30 ++---
> > > > > >>   src/ipa/raspberrypi/controller/rpi/awb.h      |   1 +
> > > > > >>   .../raspberrypi/controller/rpi/contrast.cpp   |   8 +-
> > > > > >>   src/ipa/raspberrypi/controller/rpi/focus.cpp  |   7 +-
> > > > > >>   src/ipa/raspberrypi/controller/rpi/lux.cpp    |  14 +-
> > > > > >>   src/ipa/raspberrypi/raspberrypi.cpp           |  50 ++++++-
> > > > > >>   src/ipa/raspberrypi/statistics.h              |  78 +++++++++++
> > > > > >>   17 files changed, 338 insertions(+), 95 deletions(-)
> > > > > >>   create mode 100644 src/ipa/raspberrypi/controller/region_stats.h
> > > > > >>   create mode 100644 src/ipa/raspberrypi/statistics.h
> > > > > >>

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list