[libcamera-devel] [PATCH v2 2/2] pipeline: raspberrypi: Choose bit depth and packing according to raw stream
David Plowman
david.plowman at raspberrypi.com
Wed Dec 1 14:36:07 CET 2021
So it sounds like I want to include <cmath> and call std::abs and then
I would see the same error. I guess I'm not really sure why it
compiles for me and not you, but it's clearly not right. I'll send an
updated version of just that patch.
Thanks
David
On Wed, 1 Dec 2021 at 13:04, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> On Wed, Dec 01, 2021 at 01:00:30PM +0000, Kieran Bingham wrote:
> > Quoting David Plowman (2021-12-01 10:15:08)
> > > When a raw stream is specified, the bit depth and packing requested
> > > should influence our choice of camera mode to match (if possible).
> > >
> > > Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> > > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > > Reviewed-by: Naushir Patuck <naush at raspberrypi.com>
> > > ---
> > > .../pipeline/raspberrypi/raspberrypi.cpp | 37 ++++++++++---------
> > > 1 file changed, 20 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > index 045725dd..c01e0769 100644
> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > @@ -49,6 +49,8 @@ LOG_DEFINE_CATEGORY(RPI)
> > >
> > > namespace {
> > >
> > > +constexpr unsigned int defaultRawBitDepth = 12;
> > > +
> > > /* Map of mbus codes to supported sizes reported by the sensor. */
> > > using SensorFormats = std::map<unsigned int, std::vector<Size>>;
> > >
> > > @@ -125,15 +127,13 @@ double scoreFormat(double desired, double actual)
> > > return score;
> > > }
> > >
> > > -V4L2SubdeviceFormat findBestFormat(const SensorFormats &formatsMap, const Size &req)
> > > +V4L2SubdeviceFormat findBestFormat(const SensorFormats &formatsMap, const Size &req, unsigned int bitDepth)
> > > {
> > > double bestScore = std::numeric_limits<double>::max(), score;
> > > V4L2SubdeviceFormat bestFormat;
> > >
> > > -#define PENALTY_AR 1500.0
> > > -#define PENALTY_8BIT 2000.0
> > > -#define PENALTY_10BIT 1000.0
> > > -#define PENALTY_12BIT 0.0
> > > + constexpr float penaltyAr = 1500.0;
> > > + constexpr float penaltyBitDepth = 500.0;
> > >
> > > /* Calculate the closest/best mode from the user requested size. */
> > > for (const auto &iter : formatsMap) {
> > > @@ -149,15 +149,10 @@ V4L2SubdeviceFormat findBestFormat(const SensorFormats &formatsMap, const Size &
> > > /* Score the dimensions for closeness. */
> > > score = scoreFormat(req.width, size.width);
> > > score += scoreFormat(req.height, size.height);
> > > - score += PENALTY_AR * scoreFormat(reqAr, fmtAr);
> > > + score += penaltyAr * scoreFormat(reqAr, fmtAr);
> > >
> > > /* Add any penalties... this is not an exact science! */
> > > - if (info.bitsPerPixel == 12)
> > > - score += PENALTY_12BIT;
> > > - else if (info.bitsPerPixel == 10)
> > > - score += PENALTY_10BIT;
> > > - else if (info.bitsPerPixel == 8)
> > > - score += PENALTY_8BIT;
> > > + score += abs(info.bitsPerPixel - bitDepth) * penaltyBitDepth;
> >
> > This causes the following compile faiure in my build-matrix:
> >
> > Building for clang-12
> > Checking for libcamera builddir: /home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-12
> > ninja: Entering directory `/home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-12'
> > [1/90] Generating version.cpp with a custom command
> > [2/90] Compiling C++ object src/libcamera/libcamera.so.0.0.0.p/meson-generated_.._version.cpp.o
> > [3/90] Compiling C++ object src/ipa/rkisp1/ipa_rkisp1.so.p/algorithms_agc.cpp.o
> > [4/90] Compiling C++ object test/gstreamer/multi_stream_test.p/gstreamer_multi_stream_test.cpp.o
> > [5/90] Compiling C++ object src/libcamera/libcamera.so.0.0.0.p/pipeline_raspberrypi_raspberrypi.cpp.o
> > FAILED: src/libcamera/libcamera.so.0.0.0.p/pipeline_raspberrypi_raspberrypi.cpp.o
> > clang++-12 -Isrc/libcamera/libcamera.so.0.0.0.p -Isrc/libcamera -I../../../src/libcamera/src/libcamera -Iinclude -I../../../src/libcamera/include -Iinclude/libcamera -Iinclude/libcamera/ipa
> > -Iinclude/libcamera/internal -Isrc/libcamera/proxy -fcolor-diagnostics -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++17 -g -stdlib=libc++ -Wextra-semi
> > -Wthread-safety -Wshadow -include config.h -Wno-c99-designator -fPIC -DLIBCAMERA_BASE_PRIVATE -MD -MQ src/libcamera/libcamera.so.0.0.0.p/pipeline_raspberrypi_raspberrypi.cpp.o -MF
> > src/libcamera/libcamera.so.0.0.0.p/pipeline_raspberrypi_raspberrypi.cpp.o.d -o src/libcamera/libcamera.so.0.0.0.p/pipeline_raspberrypi_raspberrypi.cpp.o -c
> > ../../../src/libcamera/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > ../../../src/libcamera/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp:155:13: error: call to 'abs' is ambiguous
> > score += abs(info.bitsPerPixel - bitDepth) * penaltyBitDepth;
> > ^~~
> > /usr/include/stdlib.h:840:12: note: candidate function
> > extern int abs (int __x) __THROW __attribute__ ((__const__)) __wur;
> > ^
> > /usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:107:39: note: candidate function
> > inline _LIBCPP_INLINE_VISIBILITY long abs(long __x) _NOEXCEPT {
> > ^
> > /usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:111:44: note: candidate function
> > inline _LIBCPP_INLINE_VISIBILITY long long abs(long long __x) _NOEXCEPT {
> > ^
> > /usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:118:40: note: candidate function
> > inline _LIBCPP_INLINE_VISIBILITY float abs(float __lcpp_x) _NOEXCEPT {
> > ^
> > /usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:122:41: note: candidate function
> > inline _LIBCPP_INLINE_VISIBILITY double abs(double __lcpp_x) _NOEXCEPT {
> > ^
> > /usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:127:1: note: candidate function
> > abs(long double __lcpp_x) _NOEXCEPT {
> > ^
> > 1 error generated.
> > ninja: build stopped: subcommand failed.
> >
> > I can fix while applying with:
> >
> > - score += abs(info.bitsPerPixel - bitDepth) * penaltyBitDepth;
> > + score += abs(static_cast<int>(info.bitsPerPixel - bitDepth)) * penaltyBitDepth;
> >
> > if you're happy with that, or otherwise if I get any better
> > recommendations.
>
> Does https://libcamera.org/coding-style.html#c-compatibility-headers
> (the part about std:: functions) help ?
>
> > >
> > > if (score <= bestScore) {
> > > bestScore = score;
> > > @@ -397,9 +392,14 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> > > * Calculate the best sensor mode we can use based on
> > > * the user request.
> > > */
> > > - V4L2SubdeviceFormat sensorFormat = findBestFormat(data_->sensorFormats_, cfg.size);
> > > + const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);
> > > + unsigned int bitDepth = info.isValid() ? info.bitsPerPixel : defaultRawBitDepth;
> > > + V4L2SubdeviceFormat sensorFormat = findBestFormat(data_->sensorFormats_, cfg.size, bitDepth);
> > > + BayerFormat::Packing packing = BayerFormat::Packing::CSI2;
> > > + if (info.isValid() && !info.packed)
> > > + packing = BayerFormat::Packing::None;
> > > V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat,
> > > - BayerFormat::Packing::CSI2);
> > > + packing);
> > > int ret = data_->unicam_[Unicam::Image].dev()->tryFormat(&unicamFormat);
> > > if (ret)
> > > return Invalid;
> > > @@ -533,7 +533,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
> > > switch (role) {
> > > case StreamRole::Raw:
> > > size = data->sensor_->resolution();
> > > - sensorFormat = findBestFormat(data->sensorFormats_, size);
> > > + sensorFormat = findBestFormat(data->sensorFormats_, size, defaultRawBitDepth);
> > > pixelFormat = mbusCodeToPixelFormat(sensorFormat.mbus_code,
> > > BayerFormat::Packing::CSI2);
> > > ASSERT(pixelFormat.isValid());
> > > @@ -622,6 +622,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> > > Size maxSize, sensorSize;
> > > unsigned int maxIndex = 0;
> > > bool rawStream = false;
> > > + unsigned int bitDepth = defaultRawBitDepth;
> > >
> > > /*
> > > * Look for the RAW stream (if given) size as well as the largest
> > > @@ -638,7 +639,9 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> > > sensorSize = cfg.size;
> > > rawStream = true;
> > > /* Check if the user has explicitly set an unpacked format. */
> > > - packing = BayerFormat::fromPixelFormat(cfg.pixelFormat).packing;
> > > + BayerFormat bayerFormat = BayerFormat::fromPixelFormat(cfg.pixelFormat);
> > > + packing = bayerFormat.packing;
> > > + bitDepth = bayerFormat.bitDepth;
> > > } else {
> > > if (cfg.size > maxSize) {
> > > maxSize = config->at(i).size;
> > > @@ -664,7 +667,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> > > }
> > >
> > > /* First calculate the best sensor mode we can use based on the user request. */
> > > - V4L2SubdeviceFormat sensorFormat = findBestFormat(data->sensorFormats_, rawStream ? sensorSize : maxSize);
> > > + V4L2SubdeviceFormat sensorFormat = findBestFormat(data->sensorFormats_, rawStream ? sensorSize : maxSize, bitDepth);
> > > ret = data->sensor_->setFormat(&sensorFormat);
> > > if (ret)
> > > return ret;
>
> --
> Regards,
>
> Laurent Pinchart
More information about the libcamera-devel
mailing list