[{"id":21520,"web_url":"https://patchwork.libcamera.org/comment/21520/","msgid":"<163836363085.3059017.15476441460153937095@Monstersaurus>","date":"2021-12-01T13:00:30","subject":"Re: [libcamera-devel] [PATCH v2 2/2] pipeline: raspberrypi: Choose\n\tbit depth and packing according to raw stream","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting David Plowman (2021-12-01 10:15:08)\n> When a raw stream is specified, the bit depth and packing requested\n> should influence our choice of camera mode to match (if possible).\n> \n> Signed-off-by: David Plowman \n> Reviewed-by: Kieran Bingham \n> Reviewed-by: Naushir Patuck \n> ---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 37 ++++++++++---------\n> 1 file changed, 20 insertions(+), 17 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 045725dd..c01e0769 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -49,6 +49,8 @@ LOG_DEFINE_CATEGORY(RPI)\n> \n> namespace {\n> \n> +constexpr unsigned int defaultRawBitDepth = 12;\n> +\n> /* Map of mbus codes to supported sizes reported by the sensor. */\n> using SensorFormats = std::map>;\n> \n> @@ -125,15 +127,13 @@ double scoreFormat(double desired, double actual)\n> return score;\n> }\n> \n> -V4L2SubdeviceFormat findBestFormat(const SensorFormats &formatsMap, const Size &req)\n> +V4L2SubdeviceFormat findBestFormat(const SensorFormats &formatsMap, const Size &req, unsigned int bitDepth)\n> {\n> double bestScore = std::numeric_limits::max(), score;\n> V4L2SubdeviceFormat bestFormat;\n> \n> -#define PENALTY_AR 1500.0\n> -#define PENALTY_8BIT 2000.0\n> -#define PENALTY_10BIT 1000.0\n> -#define PENALTY_12BIT 0.0\n> + constexpr float penaltyAr = 1500.0;\n> + constexpr float penaltyBitDepth = 500.0;\n> \n> /* Calculate the closest/best mode from the user requested size. */\n> for (const auto &iter : formatsMap) {\n> @@ -149,15 +149,10 @@ V4L2SubdeviceFormat findBestFormat(const SensorFormats &formatsMap, const Size &\n> /* Score the dimensions for closeness. */\n> score = scoreFormat(req.width, size.width);\n> score += scoreFormat(req.height, size.height);\n> - score += PENALTY_AR * scoreFormat(reqAr, fmtAr);\n> + score += penaltyAr * scoreFormat(reqAr, fmtAr);\n> \n> /* Add any penalties... this is not an exact science! */\n> - if (info.bitsPerPixel == 12)\n> - score += PENALTY_12BIT;\n> - else if (info.bitsPerPixel == 10)\n> - score += PENALTY_10BIT;\n> - else if (info.bitsPerPixel == 8)\n> - score += PENALTY_8BIT;\n> + score += abs(info.bitsPerPixel - bitDepth) * penaltyBitDepth;\n\nThis causes the following compile faiure in my build-matrix:\n\nBuilding for clang-12\nChecking for libcamera builddir: /home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-12\nninja: Entering directory `/home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-12'\n[1/90] Generating version.cpp with a custom command\n[2/90] Compiling C++ object src/libcamera/libcamera.so.0.0.0.p/meson-generated_.._version.cpp.o\n[3/90] Compiling C++ object src/ipa/rkisp1/ipa_rkisp1.so.p/algorithms_agc.cpp.o\n[4/90] Compiling C++ object test/gstreamer/multi_stream_test.p/gstreamer_multi_stream_test.cpp.o\n[5/90] Compiling C++ object src/libcamera/libcamera.so.0.0.0.p/pipeline_raspberrypi_raspberrypi.cpp.o\nFAILED: src/libcamera/libcamera.so.0.0.0.p/pipeline_raspberrypi_raspberrypi.cpp.o\nclang++-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\n-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\n-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\nsrc/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\n../../../src/libcamera/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n../../../src/libcamera/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp:155:13: error: call to 'abs' is ambiguous\n score += abs(info.bitsPerPixel - bitDepth) * penaltyBitDepth;\n ^~~\n/usr/include/stdlib.h:840:12: note: candidate function\nextern int abs (int __x) __THROW __attribute__ ((__const__)) __wur;\n ^\n/usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:107:39: note: candidate function\ninline _LIBCPP_INLINE_VISIBILITY long abs(long __x) _NOEXCEPT {\n ^\n/usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:111:44: note: candidate function\ninline _LIBCPP_INLINE_VISIBILITY long long abs(long long __x) _NOEXCEPT {\n ^\n/usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:118:40: note: candidate function\ninline _LIBCPP_INLINE_VISIBILITY float abs(float __lcpp_x) _NOEXCEPT {\n ^\n/usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:122:41: note: candidate function\ninline _LIBCPP_INLINE_VISIBILITY double abs(double __lcpp_x) _NOEXCEPT {\n ^\n/usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:127:1: note: candidate function\nabs(long double __lcpp_x) _NOEXCEPT {\n^\n1 error generated.\nninja: build stopped: subcommand failed.\n\nI can fix while applying with:\n\n- score += abs(info.bitsPerPixel - bitDepth) * penaltyBitDepth;\n+ score += abs(static_cast(info.bitsPerPixel - bitDepth)) * penaltyBitDepth;\n\nif you're happy with that, or otherwise if I get any better\nrecommendations.\n\n--\nKieran\n\n\n> \n> if (score <= bestScore) {\n> bestScore = score;\n> @@ -397,9 +392,14 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> * Calculate the best sensor mode we can use based on\n> * the user request.\n> */\n> - V4L2SubdeviceFormat sensorFormat = findBestFormat(data_->sensorFormats_, cfg.size);\n> + const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);\n> + unsigned int bitDepth = info.isValid() ? info.bitsPerPixel : defaultRawBitDepth;\n> + V4L2SubdeviceFormat sensorFormat = findBestFormat(data_->sensorFormats_, cfg.size, bitDepth);\n> + BayerFormat::Packing packing = BayerFormat::Packing::CSI2;\n> + if (info.isValid() && !info.packed)\n> + packing = BayerFormat::Packing::None;\n> V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat,\n> - BayerFormat::Packing::CSI2);\n> + packing);\n> int ret = data_->unicam_[Unicam::Image].dev()->tryFormat(&unicamFormat);\n> if (ret)\n> return Invalid;\n> @@ -533,7 +533,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n> switch (role) {\n> case StreamRole::Raw:\n> size = data->sensor_->resolution();\n> - sensorFormat = findBestFormat(data->sensorFormats_, size);\n> + sensorFormat = findBestFormat(data->sensorFormats_, size, defaultRawBitDepth);\n> pixelFormat = mbusCodeToPixelFormat(sensorFormat.mbus_code,\n> BayerFormat::Packing::CSI2);\n> ASSERT(pixelFormat.isValid());\n> @@ -622,6 +622,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> Size maxSize, sensorSize;\n> unsigned int maxIndex = 0;\n> bool rawStream = false;\n> + unsigned int bitDepth = defaultRawBitDepth;\n> \n> /*\n> * Look for the RAW stream (if given) size as well as the largest\n> @@ -638,7 +639,9 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> sensorSize = cfg.size;\n> rawStream = true;\n> /* Check if the user has explicitly set an unpacked format. */\n> - packing = BayerFormat::fromPixelFormat(cfg.pixelFormat).packing;\n> + BayerFormat bayerFormat = BayerFormat::fromPixelFormat(cfg.pixelFormat);\n> + packing = bayerFormat.packing;\n> + bitDepth = bayerFormat.bitDepth;\n> } else {\n> if (cfg.size > maxSize) {\n> maxSize = config->at(i).size;\n> @@ -664,7 +667,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> }\n> \n> /* First calculate the best sensor mode we can use based on the user request. */\n> - V4L2SubdeviceFormat sensorFormat = findBestFormat(data->sensorFormats_, rawStream ? sensorSize : maxSize);\n> + V4L2SubdeviceFormat sensorFormat = findBestFormat(data->sensorFormats_, rawStream ? sensorSize : maxSize, bitDepth);\n> ret = data->sensor_->setFormat(&sensorFormat);\n> if (ret)\n> return ret;\n> -- \n> 2.30.2\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D784ABDB13\n\tfor ;\n\tWed, 1 Dec 2021 13:00:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 38875604FC;\n\tWed, 1 Dec 2021 14:00:35 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2952B6011A\n\tfor ;\n\tWed, 1 Dec 2021 14:00:33 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A23F3DEE;\n\tWed, 1 Dec 2021 14:00:32 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"g2zqSpeQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638363632;\n\tbh=cziX0iIn44zgGEjN9y0fM1syFOmOWMKvoqYQ2342iDI=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=g2zqSpeQJ6ME4BiM8Wa6tXxCFDmuR4JcG/7jOgXAxYfNlf8j3Cd0NPz6vOrlT/PBC\n\t/GT74YBDeQuaE1YZTY3XanFGEZvOtGX2HFs0kDDMif+rAHzl2Ehgh9MiVZmAr0+E1c\n\tA+9cR/U2SJ/sCkNWB2srZEdUdLvZUeCDN+n3pIuI=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211201101508.10619-3-david.plowman@raspberrypi.com>","References":"<20211201101508.10619-1-david.plowman@raspberrypi.com>\n\t<20211201101508.10619-3-david.plowman@raspberrypi.com>","From":"Kieran Bingham ","To":"David Plowman ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 01 Dec 2021 13:00:30 +0000","Message-ID":"<163836363085.3059017.15476441460153937095@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] pipeline: raspberrypi: Choose\n\tbit depth and packing according to raw stream","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21521,"web_url":"https://patchwork.libcamera.org/comment/21521/","msgid":"","date":"2021-12-01T13:04:24","subject":"Re: [libcamera-devel] [PATCH v2 2/2] pipeline: raspberrypi: Choose\n\tbit depth and packing according to raw stream","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Dec 01, 2021 at 01:00:30PM +0000, Kieran Bingham wrote:\n> Quoting David Plowman (2021-12-01 10:15:08)\n> > When a raw stream is specified, the bit depth and packing requested\n> > should influence our choice of camera mode to match (if possible).\n> > \n> > Signed-off-by: David Plowman \n> > Reviewed-by: Kieran Bingham \n> > Reviewed-by: Naushir Patuck \n> > ---\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 37 ++++++++++---------\n> > 1 file changed, 20 insertions(+), 17 deletions(-)\n> > \n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 045725dd..c01e0769 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -49,6 +49,8 @@ LOG_DEFINE_CATEGORY(RPI)\n> > \n> > namespace {\n> > \n> > +constexpr unsigned int defaultRawBitDepth = 12;\n> > +\n> > /* Map of mbus codes to supported sizes reported by the sensor. */\n> > using SensorFormats = std::map>;\n> > \n> > @@ -125,15 +127,13 @@ double scoreFormat(double desired, double actual)\n> > return score;\n> > }\n> > \n> > -V4L2SubdeviceFormat findBestFormat(const SensorFormats &formatsMap, const Size &req)\n> > +V4L2SubdeviceFormat findBestFormat(const SensorFormats &formatsMap, const Size &req, unsigned int bitDepth)\n> > {\n> > double bestScore = std::numeric_limits::max(), score;\n> > V4L2SubdeviceFormat bestFormat;\n> > \n> > -#define PENALTY_AR 1500.0\n> > -#define PENALTY_8BIT 2000.0\n> > -#define PENALTY_10BIT 1000.0\n> > -#define PENALTY_12BIT 0.0\n> > + constexpr float penaltyAr = 1500.0;\n> > + constexpr float penaltyBitDepth = 500.0;\n> > \n> > /* Calculate the closest/best mode from the user requested size. */\n> > for (const auto &iter : formatsMap) {\n> > @@ -149,15 +149,10 @@ V4L2SubdeviceFormat findBestFormat(const SensorFormats &formatsMap, const Size &\n> > /* Score the dimensions for closeness. */\n> > score = scoreFormat(req.width, size.width);\n> > score += scoreFormat(req.height, size.height);\n> > - score += PENALTY_AR * scoreFormat(reqAr, fmtAr);\n> > + score += penaltyAr * scoreFormat(reqAr, fmtAr);\n> > \n> > /* Add any penalties... this is not an exact science! */\n> > - if (info.bitsPerPixel == 12)\n> > - score += PENALTY_12BIT;\n> > - else if (info.bitsPerPixel == 10)\n> > - score += PENALTY_10BIT;\n> > - else if (info.bitsPerPixel == 8)\n> > - score += PENALTY_8BIT;\n> > + score += abs(info.bitsPerPixel - bitDepth) * penaltyBitDepth;\n> \n> This causes the following compile faiure in my build-matrix:\n> \n> Building for clang-12\n> Checking for libcamera builddir: /home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-12\n> ninja: Entering directory `/home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-12'\n> [1/90] Generating version.cpp with a custom command\n> [2/90] Compiling C++ object src/libcamera/libcamera.so.0.0.0.p/meson-generated_.._version.cpp.o\n> [3/90] Compiling C++ object src/ipa/rkisp1/ipa_rkisp1.so.p/algorithms_agc.cpp.o\n> [4/90] Compiling C++ object test/gstreamer/multi_stream_test.p/gstreamer_multi_stream_test.cpp.o\n> [5/90] Compiling C++ object src/libcamera/libcamera.so.0.0.0.p/pipeline_raspberrypi_raspberrypi.cpp.o\n> FAILED: src/libcamera/libcamera.so.0.0.0.p/pipeline_raspberrypi_raspberrypi.cpp.o\n> 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\n> -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\n> -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\n> 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\n> ../../../src/libcamera/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> ../../../src/libcamera/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp:155:13: error: call to 'abs' is ambiguous\n> score += abs(info.bitsPerPixel - bitDepth) * penaltyBitDepth;\n> ^~~\n> /usr/include/stdlib.h:840:12: note: candidate function\n> extern int abs (int __x) __THROW __attribute__ ((__const__)) __wur;\n> ^\n> /usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:107:39: note: candidate function\n> inline _LIBCPP_INLINE_VISIBILITY long abs(long __x) _NOEXCEPT {\n> ^\n> /usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:111:44: note: candidate function\n> inline _LIBCPP_INLINE_VISIBILITY long long abs(long long __x) _NOEXCEPT {\n> ^\n> /usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:118:40: note: candidate function\n> inline _LIBCPP_INLINE_VISIBILITY float abs(float __lcpp_x) _NOEXCEPT {\n> ^\n> /usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:122:41: note: candidate function\n> inline _LIBCPP_INLINE_VISIBILITY double abs(double __lcpp_x) _NOEXCEPT {\n> ^\n> /usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:127:1: note: candidate function\n> abs(long double __lcpp_x) _NOEXCEPT {\n> ^\n> 1 error generated.\n> ninja: build stopped: subcommand failed.\n> \n> I can fix while applying with:\n> \n> - score += abs(info.bitsPerPixel - bitDepth) * penaltyBitDepth;\n> + score += abs(static_cast(info.bitsPerPixel - bitDepth)) * penaltyBitDepth;\n> \n> if you're happy with that, or otherwise if I get any better\n> recommendations.\n\nDoes https://libcamera.org/coding-style.html#c-compatibility-headers\n(the part about std:: functions) help ?\n\n> > \n> > if (score <= bestScore) {\n> > bestScore = score;\n> > @@ -397,9 +392,14 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> > * Calculate the best sensor mode we can use based on\n> > * the user request.\n> > */\n> > - V4L2SubdeviceFormat sensorFormat = findBestFormat(data_->sensorFormats_, cfg.size);\n> > + const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);\n> > + unsigned int bitDepth = info.isValid() ? info.bitsPerPixel : defaultRawBitDepth;\n> > + V4L2SubdeviceFormat sensorFormat = findBestFormat(data_->sensorFormats_, cfg.size, bitDepth);\n> > + BayerFormat::Packing packing = BayerFormat::Packing::CSI2;\n> > + if (info.isValid() && !info.packed)\n> > + packing = BayerFormat::Packing::None;\n> > V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat,\n> > - BayerFormat::Packing::CSI2);\n> > + packing);\n> > int ret = data_->unicam_[Unicam::Image].dev()->tryFormat(&unicamFormat);\n> > if (ret)\n> > return Invalid;\n> > @@ -533,7 +533,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n> > switch (role) {\n> > case StreamRole::Raw:\n> > size = data->sensor_->resolution();\n> > - sensorFormat = findBestFormat(data->sensorFormats_, size);\n> > + sensorFormat = findBestFormat(data->sensorFormats_, size, defaultRawBitDepth);\n> > pixelFormat = mbusCodeToPixelFormat(sensorFormat.mbus_code,\n> > BayerFormat::Packing::CSI2);\n> > ASSERT(pixelFormat.isValid());\n> > @@ -622,6 +622,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> > Size maxSize, sensorSize;\n> > unsigned int maxIndex = 0;\n> > bool rawStream = false;\n> > + unsigned int bitDepth = defaultRawBitDepth;\n> > \n> > /*\n> > * Look for the RAW stream (if given) size as well as the largest\n> > @@ -638,7 +639,9 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> > sensorSize = cfg.size;\n> > rawStream = true;\n> > /* Check if the user has explicitly set an unpacked format. */\n> > - packing = BayerFormat::fromPixelFormat(cfg.pixelFormat).packing;\n> > + BayerFormat bayerFormat = BayerFormat::fromPixelFormat(cfg.pixelFormat);\n> > + packing = bayerFormat.packing;\n> > + bitDepth = bayerFormat.bitDepth;\n> > } else {\n> > if (cfg.size > maxSize) {\n> > maxSize = config->at(i).size;\n> > @@ -664,7 +667,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> > }\n> > \n> > /* First calculate the best sensor mode we can use based on the user request. */\n> > - V4L2SubdeviceFormat sensorFormat = findBestFormat(data->sensorFormats_, rawStream ? sensorSize : maxSize);\n> > + V4L2SubdeviceFormat sensorFormat = findBestFormat(data->sensorFormats_, rawStream ? sensorSize : maxSize, bitDepth);\n> > ret = data->sensor_->setFormat(&sensorFormat);\n> > if (ret)\n> > return ret;","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id C4228BF415\n\tfor ;\n\tWed, 1 Dec 2021 13:04:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F3B6F60724;\n\tWed, 1 Dec 2021 14:04:50 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 094366011A\n\tfor ;\n\tWed, 1 Dec 2021 14:04:50 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7B44AA15;\n\tWed, 1 Dec 2021 14:04:49 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"RYWZcSuR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638363889;\n\tbh=i5zmpFKmq8Ttmje1jjtoE9tcGBqwr7UvjD+kaXZ+0UQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=RYWZcSuRQvUUCPzcLLdcSik67Ylz58QBXQ82Ek4QQX/hASPi5AhzA7qYkxi1eqA8E\n\tdbnRLZrK5Ek5dObr2NwCmrb0IoN9P7lyMVG3MX49BHjNkDwTDolt5lCsJjVWEEo8tw\n\tMu5srCkQl9G70R5uEGHkVoKvdmmj9l6iJtSeXsCw=","Date":"Wed, 1 Dec 2021 15:04:24 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20211201101508.10619-1-david.plowman@raspberrypi.com>\n\t<20211201101508.10619-3-david.plowman@raspberrypi.com>\n\t<163836363085.3059017.15476441460153937095@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<163836363085.3059017.15476441460153937095@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] pipeline: raspberrypi: Choose\n\tbit depth and packing according to raw stream","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21524,"web_url":"https://patchwork.libcamera.org/comment/21524/","msgid":"","date":"2021-12-01T13:36:07","subject":"Re: [libcamera-devel] [PATCH v2 2/2] pipeline: raspberrypi: Choose\n\tbit depth and packing according to raw stream","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"So it sounds like I want to include and call std::abs and then\nI would see the same error. I guess I'm not really sure why it\ncompiles for me and not you, but it's clearly not right. I'll send an\nupdated version of just that patch.\n\nThanks\nDavid\n\nOn Wed, 1 Dec 2021 at 13:04, Laurent Pinchart\n wrote:\n>\n> On Wed, Dec 01, 2021 at 01:00:30PM +0000, Kieran Bingham wrote:\n> > Quoting David Plowman (2021-12-01 10:15:08)\n> > > When a raw stream is specified, the bit depth and packing requested\n> > > should influence our choice of camera mode to match (if possible).\n> > >\n> > > Signed-off-by: David Plowman \n> > > Reviewed-by: Kieran Bingham \n> > > Reviewed-by: Naushir Patuck \n> > > ---\n> > > .../pipeline/raspberrypi/raspberrypi.cpp | 37 ++++++++++---------\n> > > 1 file changed, 20 insertions(+), 17 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > index 045725dd..c01e0769 100644\n> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > @@ -49,6 +49,8 @@ LOG_DEFINE_CATEGORY(RPI)\n> > >\n> > > namespace {\n> > >\n> > > +constexpr unsigned int defaultRawBitDepth = 12;\n> > > +\n> > > /* Map of mbus codes to supported sizes reported by the sensor. */\n> > > using SensorFormats = std::map>;\n> > >\n> > > @@ -125,15 +127,13 @@ double scoreFormat(double desired, double actual)\n> > > return score;\n> > > }\n> > >\n> > > -V4L2SubdeviceFormat findBestFormat(const SensorFormats &formatsMap, const Size &req)\n> > > +V4L2SubdeviceFormat findBestFormat(const SensorFormats &formatsMap, const Size &req, unsigned int bitDepth)\n> > > {\n> > > double bestScore = std::numeric_limits::max(), score;\n> > > V4L2SubdeviceFormat bestFormat;\n> > >\n> > > -#define PENALTY_AR 1500.0\n> > > -#define PENALTY_8BIT 2000.0\n> > > -#define PENALTY_10BIT 1000.0\n> > > -#define PENALTY_12BIT 0.0\n> > > + constexpr float penaltyAr = 1500.0;\n> > > + constexpr float penaltyBitDepth = 500.0;\n> > >\n> > > /* Calculate the closest/best mode from the user requested size. */\n> > > for (const auto &iter : formatsMap) {\n> > > @@ -149,15 +149,10 @@ V4L2SubdeviceFormat findBestFormat(const SensorFormats &formatsMap, const Size &\n> > > /* Score the dimensions for closeness. */\n> > > score = scoreFormat(req.width, size.width);\n> > > score += scoreFormat(req.height, size.height);\n> > > - score += PENALTY_AR * scoreFormat(reqAr, fmtAr);\n> > > + score += penaltyAr * scoreFormat(reqAr, fmtAr);\n> > >\n> > > /* Add any penalties... this is not an exact science! */\n> > > - if (info.bitsPerPixel == 12)\n> > > - score += PENALTY_12BIT;\n> > > - else if (info.bitsPerPixel == 10)\n> > > - score += PENALTY_10BIT;\n> > > - else if (info.bitsPerPixel == 8)\n> > > - score += PENALTY_8BIT;\n> > > + score += abs(info.bitsPerPixel - bitDepth) * penaltyBitDepth;\n> >\n> > This causes the following compile faiure in my build-matrix:\n> >\n> > Building for clang-12\n> > Checking for libcamera builddir: /home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-12\n> > ninja: Entering directory `/home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-12'\n> > [1/90] Generating version.cpp with a custom command\n> > [2/90] Compiling C++ object src/libcamera/libcamera.so.0.0.0.p/meson-generated_.._version.cpp.o\n> > [3/90] Compiling C++ object src/ipa/rkisp1/ipa_rkisp1.so.p/algorithms_agc.cpp.o\n> > [4/90] Compiling C++ object test/gstreamer/multi_stream_test.p/gstreamer_multi_stream_test.cpp.o\n> > [5/90] Compiling C++ object src/libcamera/libcamera.so.0.0.0.p/pipeline_raspberrypi_raspberrypi.cpp.o\n> > FAILED: src/libcamera/libcamera.so.0.0.0.p/pipeline_raspberrypi_raspberrypi.cpp.o\n> > 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\n> > -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\n> > -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\n> > 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\n> > ../../../src/libcamera/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > ../../../src/libcamera/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp:155:13: error: call to 'abs' is ambiguous\n> > score += abs(info.bitsPerPixel - bitDepth) * penaltyBitDepth;\n> > ^~~\n> > /usr/include/stdlib.h:840:12: note: candidate function\n> > extern int abs (int __x) __THROW __attribute__ ((__const__)) __wur;\n> > ^\n> > /usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:107:39: note: candidate function\n> > inline _LIBCPP_INLINE_VISIBILITY long abs(long __x) _NOEXCEPT {\n> > ^\n> > /usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:111:44: note: candidate function\n> > inline _LIBCPP_INLINE_VISIBILITY long long abs(long long __x) _NOEXCEPT {\n> > ^\n> > /usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:118:40: note: candidate function\n> > inline _LIBCPP_INLINE_VISIBILITY float abs(float __lcpp_x) _NOEXCEPT {\n> > ^\n> > /usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:122:41: note: candidate function\n> > inline _LIBCPP_INLINE_VISIBILITY double abs(double __lcpp_x) _NOEXCEPT {\n> > ^\n> > /usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:127:1: note: candidate function\n> > abs(long double __lcpp_x) _NOEXCEPT {\n> > ^\n> > 1 error generated.\n> > ninja: build stopped: subcommand failed.\n> >\n> > I can fix while applying with:\n> >\n> > - score += abs(info.bitsPerPixel - bitDepth) * penaltyBitDepth;\n> > + score += abs(static_cast(info.bitsPerPixel - bitDepth)) * penaltyBitDepth;\n> >\n> > if you're happy with that, or otherwise if I get any better\n> > recommendations.\n>\n> Does https://libcamera.org/coding-style.html#c-compatibility-headers\n> (the part about std:: functions) help ?\n>\n> > >\n> > > if (score <= bestScore) {\n> > > bestScore = score;\n> > > @@ -397,9 +392,14 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> > > * Calculate the best sensor mode we can use based on\n> > > * the user request.\n> > > */\n> > > - V4L2SubdeviceFormat sensorFormat = findBestFormat(data_->sensorFormats_, cfg.size);\n> > > + const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);\n> > > + unsigned int bitDepth = info.isValid() ? info.bitsPerPixel : defaultRawBitDepth;\n> > > + V4L2SubdeviceFormat sensorFormat = findBestFormat(data_->sensorFormats_, cfg.size, bitDepth);\n> > > + BayerFormat::Packing packing = BayerFormat::Packing::CSI2;\n> > > + if (info.isValid() && !info.packed)\n> > > + packing = BayerFormat::Packing::None;\n> > > V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat,\n> > > - BayerFormat::Packing::CSI2);\n> > > + packing);\n> > > int ret = data_->unicam_[Unicam::Image].dev()->tryFormat(&unicamFormat);\n> > > if (ret)\n> > > return Invalid;\n> > > @@ -533,7 +533,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n> > > switch (role) {\n> > > case StreamRole::Raw:\n> > > size = data->sensor_->resolution();\n> > > - sensorFormat = findBestFormat(data->sensorFormats_, size);\n> > > + sensorFormat = findBestFormat(data->sensorFormats_, size, defaultRawBitDepth);\n> > > pixelFormat = mbusCodeToPixelFormat(sensorFormat.mbus_code,\n> > > BayerFormat::Packing::CSI2);\n> > > ASSERT(pixelFormat.isValid());\n> > > @@ -622,6 +622,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> > > Size maxSize, sensorSize;\n> > > unsigned int maxIndex = 0;\n> > > bool rawStream = false;\n> > > + unsigned int bitDepth = defaultRawBitDepth;\n> > >\n> > > /*\n> > > * Look for the RAW stream (if given) size as well as the largest\n> > > @@ -638,7 +639,9 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> > > sensorSize = cfg.size;\n> > > rawStream = true;\n> > > /* Check if the user has explicitly set an unpacked format. */\n> > > - packing = BayerFormat::fromPixelFormat(cfg.pixelFormat).packing;\n> > > + BayerFormat bayerFormat = BayerFormat::fromPixelFormat(cfg.pixelFormat);\n> > > + packing = bayerFormat.packing;\n> > > + bitDepth = bayerFormat.bitDepth;\n> > > } else {\n> > > if (cfg.size > maxSize) {\n> > > maxSize = config->at(i).size;\n> > > @@ -664,7 +667,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> > > }\n> > >\n> > > /* First calculate the best sensor mode we can use based on the user request. */\n> > > - V4L2SubdeviceFormat sensorFormat = findBestFormat(data->sensorFormats_, rawStream ? sensorSize : maxSize);\n> > > + V4L2SubdeviceFormat sensorFormat = findBestFormat(data->sensorFormats_, rawStream ? sensorSize : maxSize, bitDepth);\n> > > ret = data->sensor_->setFormat(&sensorFormat);\n> > > if (ret)\n> > > return ret;\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 8395ABDB13\n\tfor ;\n\tWed, 1 Dec 2021 13:36:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DBC70604FC;\n\tWed, 1 Dec 2021 14:36:22 +0100 (CET)","from mail-wr1-x42d.google.com (mail-wr1-x42d.google.com\n\t[IPv6:2a00:1450:4864:20::42d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 908756011A\n\tfor ;\n\tWed, 1 Dec 2021 14:36:20 +0100 (CET)","by mail-wr1-x42d.google.com with SMTP id q3so29466879wru.5\n\tfor ;\n\tWed, 01 Dec 2021 05:36:20 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"h82b4/Of\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=D2yOtjQw8dEA0LGGbi+RuAhc5HRGoq3lqHDS4JCAsO8=;\n\tb=h82b4/OfcwMLCADEJWwLXGM4RTZ0J2ekHVDLD8xRBe151Ygk7yF0cgrTDW3W4VKMp2\n\tUhwK4oXPjUn5AYlyxtLrJtVBd6O2SRvDYkYYzyP+3txxYeJ2rIUHolrFs9r/Lqi4rIv+\n\ttKeaThQbkWNWMzbl6aR2D3ZgetSA5HecunsJWc4EEwDgruEk4srd10AhCGNESnWI9uO3\n\t595HV0ju9DDqJrza310Eh2Tzviv/3oJ6P8q1DL/diQsIK9dfuTXmlvTla+rRap85Yy3K\n\tmvBLc/i1fP9mtEhxa7AyyURwiS4qGtSj5yg8NyJrnirU5kgK3ISVfDuCOKGakOLdVrOf\n\tA5xw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=D2yOtjQw8dEA0LGGbi+RuAhc5HRGoq3lqHDS4JCAsO8=;\n\tb=a/tk//wPK1rGM5jtOqv2Fsa7MrMvjt2RjO8yQcXKkx1Xya1l/yZfFQU+Wul+ECuwiR\n\tvHkKun9Yj8kUYlptBenoBbD06dUugI7m7NI/MUWV31gkpIPzxAowyF87mFgVLLonSF9g\n\tDIuiEdnH4uQb4YyBcx3GFu5Ipbqb4pR88rzyuWWx7jLJSPpKbOdXEcM8XdJ74eVBtRJL\n\tUDFLD54ADBsmFqf9VrF4uj8SQfvmSY4y+n2D4DEy2618CDs2H7MM32TQSAPJP6VxuarC\n\t/beAUuQkPVah1paQUSFkUaN31+iadiWwASfjkM5mDrcNfFbqMokzgPNoFT8kSPbPYL6h\n\tNH9A==","X-Gm-Message-State":"AOAM533xDRDXFYtxog+SJkJca2WdV1jf6lGt0Pu7rkahEkzE6QtbGbzf\n\tkXtrmiYhf32ZgQqvXBb96srk9VRFL28VBr2OK9d6k/y0FNOdxg==","X-Google-Smtp-Source":"ABdhPJzIS9J8VYkLDHGMoZf1lU8SRe4rlyK1lyt6rUzm7EAml22KNp0CTj8RcKWkiV7DA/zSjRWWZuHxH5cf9Hb7XzE=","X-Received":"by 2002:adf:e390:: with SMTP id\n\te16mr6714921wrm.494.1638365779974; \n\tWed, 01 Dec 2021 05:36:19 -0800 (PST)","MIME-Version":"1.0","References":"<20211201101508.10619-1-david.plowman@raspberrypi.com>\n\t<20211201101508.10619-3-david.plowman@raspberrypi.com>\n\t<163836363085.3059017.15476441460153937095@Monstersaurus>\n\t","In-Reply-To":"","From":"David Plowman ","Date":"Wed, 1 Dec 2021 13:36:07 +0000","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] pipeline: raspberrypi: Choose\n\tbit depth and packing according to raw stream","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21525,"web_url":"https://patchwork.libcamera.org/comment/21525/","msgid":"<163836711759.3059017.3550724596132324989@Monstersaurus>","date":"2021-12-01T13:58:37","subject":"Re: [libcamera-devel] [PATCH v2 2/2] pipeline: raspberrypi: Choose\n\tbit depth and packing according to raw stream","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nQuoting Laurent Pinchart (2021-12-01 13:04:24)\n> On Wed, Dec 01, 2021 at 01:00:30PM +0000, Kieran Bingham wrote:\n> > Quoting David Plowman (2021-12-01 10:15:08)\n> > > When a raw stream is specified, the bit depth and packing requested\n> > > should influence our choice of camera mode to match (if possible).\n> > > \n> > > Signed-off-by: David Plowman \n> > > Reviewed-by: Kieran Bingham \n> > > Reviewed-by: Naushir Patuck \n> > > ---\n> > > .../pipeline/raspberrypi/raspberrypi.cpp | 37 ++++++++++---------\n> > > 1 file changed, 20 insertions(+), 17 deletions(-)\n> > > \n> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > index 045725dd..c01e0769 100644\n> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > @@ -49,6 +49,8 @@ LOG_DEFINE_CATEGORY(RPI)\n> > > \n> > > namespace {\n> > > \n> > > +constexpr unsigned int defaultRawBitDepth = 12;\n> > > +\n> > > /* Map of mbus codes to supported sizes reported by the sensor. */\n> > > using SensorFormats = std::map>;\n> > > \n> > > @@ -125,15 +127,13 @@ double scoreFormat(double desired, double actual)\n> > > return score;\n> > > }\n> > > \n> > > -V4L2SubdeviceFormat findBestFormat(const SensorFormats &formatsMap, const Size &req)\n> > > +V4L2SubdeviceFormat findBestFormat(const SensorFormats &formatsMap, const Size &req, unsigned int bitDepth)\n> > > {\n> > > double bestScore = std::numeric_limits::max(), score;\n> > > V4L2SubdeviceFormat bestFormat;\n> > > \n> > > -#define PENALTY_AR 1500.0\n> > > -#define PENALTY_8BIT 2000.0\n> > > -#define PENALTY_10BIT 1000.0\n> > > -#define PENALTY_12BIT 0.0\n> > > + constexpr float penaltyAr = 1500.0;\n> > > + constexpr float penaltyBitDepth = 500.0;\n> > > \n> > > /* Calculate the closest/best mode from the user requested size. */\n> > > for (const auto &iter : formatsMap) {\n> > > @@ -149,15 +149,10 @@ V4L2SubdeviceFormat findBestFormat(const SensorFormats &formatsMap, const Size &\n> > > /* Score the dimensions for closeness. */\n> > > score = scoreFormat(req.width, size.width);\n> > > score += scoreFormat(req.height, size.height);\n> > > - score += PENALTY_AR * scoreFormat(reqAr, fmtAr);\n> > > + score += penaltyAr * scoreFormat(reqAr, fmtAr);\n> > > \n> > > /* Add any penalties... this is not an exact science! */\n> > > - if (info.bitsPerPixel == 12)\n> > > - score += PENALTY_12BIT;\n> > > - else if (info.bitsPerPixel == 10)\n> > > - score += PENALTY_10BIT;\n> > > - else if (info.bitsPerPixel == 8)\n> > > - score += PENALTY_8BIT;\n> > > + score += abs(info.bitsPerPixel - bitDepth) * penaltyBitDepth;\n> > \n> > This causes the following compile faiure in my build-matrix:\n> > \n> > Building for clang-12\n> > Checking for libcamera builddir: /home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-12\n> > ninja: Entering directory `/home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-12'\n> > [1/90] Generating version.cpp with a custom command\n> > [2/90] Compiling C++ object src/libcamera/libcamera.so.0.0.0.p/meson-generated_.._version.cpp.o\n> > [3/90] Compiling C++ object src/ipa/rkisp1/ipa_rkisp1.so.p/algorithms_agc.cpp.o\n> > [4/90] Compiling C++ object test/gstreamer/multi_stream_test.p/gstreamer_multi_stream_test.cpp.o\n> > [5/90] Compiling C++ object src/libcamera/libcamera.so.0.0.0.p/pipeline_raspberrypi_raspberrypi.cpp.o\n> > FAILED: src/libcamera/libcamera.so.0.0.0.p/pipeline_raspberrypi_raspberrypi.cpp.o\n> > 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\n> > -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\n> > -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\n> > 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\n> > ../../../src/libcamera/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > ../../../src/libcamera/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp:155:13: error: call to 'abs' is ambiguous\n> > score += abs(info.bitsPerPixel - bitDepth) * penaltyBitDepth;\n> > ^~~\n> > /usr/include/stdlib.h:840:12: note: candidate function\n> > extern int abs (int __x) __THROW __attribute__ ((__const__)) __wur;\n> > ^\n> > /usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:107:39: note: candidate function\n> > inline _LIBCPP_INLINE_VISIBILITY long abs(long __x) _NOEXCEPT {\n> > ^\n> > /usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:111:44: note: candidate function\n> > inline _LIBCPP_INLINE_VISIBILITY long long abs(long long __x) _NOEXCEPT {\n> > ^\n> > /usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:118:40: note: candidate function\n> > inline _LIBCPP_INLINE_VISIBILITY float abs(float __lcpp_x) _NOEXCEPT {\n> > ^\n> > /usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:122:41: note: candidate function\n> > inline _LIBCPP_INLINE_VISIBILITY double abs(double __lcpp_x) _NOEXCEPT {\n> > ^\n> > /usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:127:1: note: candidate function\n> > abs(long double __lcpp_x) _NOEXCEPT {\n> > ^\n> > 1 error generated.\n> > ninja: build stopped: subcommand failed.\n> > \n> > I can fix while applying with:\n> > \n> > - score += abs(info.bitsPerPixel - bitDepth) * penaltyBitDepth;\n> > + score += abs(static_cast(info.bitsPerPixel - bitDepth)) * penaltyBitDepth;\n> > \n> > if you're happy with that, or otherwise if I get any better\n> > recommendations.\n> \n> Does https://libcamera.org/coding-style.html#c-compatibility-headers\n> (the part about std:: functions) help ?\n\nNo, I'm afraid it doesn't.\n\nBoth info.bitsPerPixel and bitDepth are unsigned int.\nThere is no (point to having?) abs(unsigned int); So it has to decide\nwhich to map to ... and could pick any of them.\n\nI can't see any other solution other than making sure this is an int\nwhen passed to abs() currently. That could be done by calculating the\ndiff outside the parameter, \n int diff = info.bitsPerPixel - bitDepth;\n\nor with the cast...\n\n--\nKieran\n\n> \n> > > \n> > > if (score <= bestScore) {\n> > > bestScore = score;\n> > > @@ -397,9 +392,14 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> > > * Calculate the best sensor mode we can use based on\n> > > * the user request.\n> > > */\n> > > - V4L2SubdeviceFormat sensorFormat = findBestFormat(data_->sensorFormats_, cfg.size);\n> > > + const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);\n> > > + unsigned int bitDepth = info.isValid() ? info.bitsPerPixel : defaultRawBitDepth;\n> > > + V4L2SubdeviceFormat sensorFormat = findBestFormat(data_->sensorFormats_, cfg.size, bitDepth);\n> > > + BayerFormat::Packing packing = BayerFormat::Packing::CSI2;\n> > > + if (info.isValid() && !info.packed)\n> > > + packing = BayerFormat::Packing::None;\n> > > V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat,\n> > > - BayerFormat::Packing::CSI2);\n> > > + packing);\n> > > int ret = data_->unicam_[Unicam::Image].dev()->tryFormat(&unicamFormat);\n> > > if (ret)\n> > > return Invalid;\n> > > @@ -533,7 +533,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n> > > switch (role) {\n> > > case StreamRole::Raw:\n> > > size = data->sensor_->resolution();\n> > > - sensorFormat = findBestFormat(data->sensorFormats_, size);\n> > > + sensorFormat = findBestFormat(data->sensorFormats_, size, defaultRawBitDepth);\n> > > pixelFormat = mbusCodeToPixelFormat(sensorFormat.mbus_code,\n> > > BayerFormat::Packing::CSI2);\n> > > ASSERT(pixelFormat.isValid());\n> > > @@ -622,6 +622,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> > > Size maxSize, sensorSize;\n> > > unsigned int maxIndex = 0;\n> > > bool rawStream = false;\n> > > + unsigned int bitDepth = defaultRawBitDepth;\n> > > \n> > > /*\n> > > * Look for the RAW stream (if given) size as well as the largest\n> > > @@ -638,7 +639,9 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> > > sensorSize = cfg.size;\n> > > rawStream = true;\n> > > /* Check if the user has explicitly set an unpacked format. */\n> > > - packing = BayerFormat::fromPixelFormat(cfg.pixelFormat).packing;\n> > > + BayerFormat bayerFormat = BayerFormat::fromPixelFormat(cfg.pixelFormat);\n> > > + packing = bayerFormat.packing;\n> > > + bitDepth = bayerFormat.bitDepth;\n> > > } else {\n> > > if (cfg.size > maxSize) {\n> > > maxSize = config->at(i).size;\n> > > @@ -664,7 +667,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> > > }\n> > > \n> > > /* First calculate the best sensor mode we can use based on the user request. */\n> > > - V4L2SubdeviceFormat sensorFormat = findBestFormat(data->sensorFormats_, rawStream ? sensorSize : maxSize);\n> > > + V4L2SubdeviceFormat sensorFormat = findBestFormat(data->sensorFormats_, rawStream ? sensorSize : maxSize, bitDepth);\n> > > ret = data->sensor_->setFormat(&sensorFormat);\n> > > if (ret)\n> > > return ret;\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 4F901BDB13\n\tfor ;\n\tWed, 1 Dec 2021 13:58:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9E770604FC;\n\tWed, 1 Dec 2021 14:58:42 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A6DC66011A\n\tfor ;\n\tWed, 1 Dec 2021 14:58:40 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3EAE8A15;\n\tWed, 1 Dec 2021 14:58:40 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"uADcfBs/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638367120;\n\tbh=rggmfTQ+uDgdfQ1IOotuNjHbZ4GE0T0I1ivCKNGD+Vk=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=uADcfBs/J/M254szz9Tts1396zEfALzzS1cu6aEiI+Os8JW5uHiZ0nLQuDqiRx6UC\n\tEtYJEI/RWR+qLW5Ua3jzOPh2HeUO9aRfX6ijbO80gXbmjuv8/DKdaUF7ne4gEiG0LI\n\tA2pxvxfBpgO/Emy+l6s0K/HaA6UlmrrSStBMgfkI=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20211201101508.10619-1-david.plowman@raspberrypi.com>\n\t<20211201101508.10619-3-david.plowman@raspberrypi.com>\n\t<163836363085.3059017.15476441460153937095@Monstersaurus>\n\t","From":"Kieran Bingham ","To":"Laurent Pinchart ","Date":"Wed, 01 Dec 2021 13:58:37 +0000","Message-ID":"<163836711759.3059017.3550724596132324989@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] pipeline: raspberrypi: Choose\n\tbit depth and packing according to raw stream","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21527,"web_url":"https://patchwork.libcamera.org/comment/21527/","msgid":"","date":"2021-12-01T14:04:14","subject":"Re: [libcamera-devel] [PATCH v2 2/2] pipeline: raspberrypi: Choose\n\tbit depth and packing according to raw stream","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Well, it is addressing the broader question, including why it failed\nfor you and not me. It's talking about whether you're going to pick up\nautomatic conversions without realising, or overloads at which point\nit can realise that there's a problem. And then to fix it you do need\nthe cast, but at least it told you!\n\nDavid\n\nOn Wed, 1 Dec 2021 at 13:58, Kieran Bingham\n wrote:\n>\n> Hi Laurent,\n>\n> Quoting Laurent Pinchart (2021-12-01 13:04:24)\n> > On Wed, Dec 01, 2021 at 01:00:30PM +0000, Kieran Bingham wrote:\n> > > Quoting David Plowman (2021-12-01 10:15:08)\n> > > > When a raw stream is specified, the bit depth and packing requested\n> > > > should influence our choice of camera mode to match (if possible).\n> > > >\n> > > > Signed-off-by: David Plowman \n> > > > Reviewed-by: Kieran Bingham \n> > > > Reviewed-by: Naushir Patuck \n> > > > ---\n> > > > .../pipeline/raspberrypi/raspberrypi.cpp | 37 ++++++++++---------\n> > > > 1 file changed, 20 insertions(+), 17 deletions(-)\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > index 045725dd..c01e0769 100644\n> > > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > @@ -49,6 +49,8 @@ LOG_DEFINE_CATEGORY(RPI)\n> > > >\n> > > > namespace {\n> > > >\n> > > > +constexpr unsigned int defaultRawBitDepth = 12;\n> > > > +\n> > > > /* Map of mbus codes to supported sizes reported by the sensor. */\n> > > > using SensorFormats = std::map>;\n> > > >\n> > > > @@ -125,15 +127,13 @@ double scoreFormat(double desired, double actual)\n> > > > return score;\n> > > > }\n> > > >\n> > > > -V4L2SubdeviceFormat findBestFormat(const SensorFormats &formatsMap, const Size &req)\n> > > > +V4L2SubdeviceFormat findBestFormat(const SensorFormats &formatsMap, const Size &req, unsigned int bitDepth)\n> > > > {\n> > > > double bestScore = std::numeric_limits::max(), score;\n> > > > V4L2SubdeviceFormat bestFormat;\n> > > >\n> > > > -#define PENALTY_AR 1500.0\n> > > > -#define PENALTY_8BIT 2000.0\n> > > > -#define PENALTY_10BIT 1000.0\n> > > > -#define PENALTY_12BIT 0.0\n> > > > + constexpr float penaltyAr = 1500.0;\n> > > > + constexpr float penaltyBitDepth = 500.0;\n> > > >\n> > > > /* Calculate the closest/best mode from the user requested size. */\n> > > > for (const auto &iter : formatsMap) {\n> > > > @@ -149,15 +149,10 @@ V4L2SubdeviceFormat findBestFormat(const SensorFormats &formatsMap, const Size &\n> > > > /* Score the dimensions for closeness. */\n> > > > score = scoreFormat(req.width, size.width);\n> > > > score += scoreFormat(req.height, size.height);\n> > > > - score += PENALTY_AR * scoreFormat(reqAr, fmtAr);\n> > > > + score += penaltyAr * scoreFormat(reqAr, fmtAr);\n> > > >\n> > > > /* Add any penalties... this is not an exact science! */\n> > > > - if (info.bitsPerPixel == 12)\n> > > > - score += PENALTY_12BIT;\n> > > > - else if (info.bitsPerPixel == 10)\n> > > > - score += PENALTY_10BIT;\n> > > > - else if (info.bitsPerPixel == 8)\n> > > > - score += PENALTY_8BIT;\n> > > > + score += abs(info.bitsPerPixel - bitDepth) * penaltyBitDepth;\n> > >\n> > > This causes the following compile faiure in my build-matrix:\n> > >\n> > > Building for clang-12\n> > > Checking for libcamera builddir: /home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-12\n> > > ninja: Entering directory `/home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-12'\n> > > [1/90] Generating version.cpp with a custom command\n> > > [2/90] Compiling C++ object src/libcamera/libcamera.so.0.0.0.p/meson-generated_.._version.cpp.o\n> > > [3/90] Compiling C++ object src/ipa/rkisp1/ipa_rkisp1.so.p/algorithms_agc.cpp.o\n> > > [4/90] Compiling C++ object test/gstreamer/multi_stream_test.p/gstreamer_multi_stream_test.cpp.o\n> > > [5/90] Compiling C++ object src/libcamera/libcamera.so.0.0.0.p/pipeline_raspberrypi_raspberrypi.cpp.o\n> > > FAILED: src/libcamera/libcamera.so.0.0.0.p/pipeline_raspberrypi_raspberrypi.cpp.o\n> > > 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\n> > > -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\n> > > -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\n> > > 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\n> > > ../../../src/libcamera/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > ../../../src/libcamera/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp:155:13: error: call to 'abs' is ambiguous\n> > > score += abs(info.bitsPerPixel - bitDepth) * penaltyBitDepth;\n> > > ^~~\n> > > /usr/include/stdlib.h:840:12: note: candidate function\n> > > extern int abs (int __x) __THROW __attribute__ ((__const__)) __wur;\n> > > ^\n> > > /usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:107:39: note: candidate function\n> > > inline _LIBCPP_INLINE_VISIBILITY long abs(long __x) _NOEXCEPT {\n> > > ^\n> > > /usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:111:44: note: candidate function\n> > > inline _LIBCPP_INLINE_VISIBILITY long long abs(long long __x) _NOEXCEPT {\n> > > ^\n> > > /usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:118:40: note: candidate function\n> > > inline _LIBCPP_INLINE_VISIBILITY float abs(float __lcpp_x) _NOEXCEPT {\n> > > ^\n> > > /usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:122:41: note: candidate function\n> > > inline _LIBCPP_INLINE_VISIBILITY double abs(double __lcpp_x) _NOEXCEPT {\n> > > ^\n> > > /usr/lib/llvm-12/bin/../include/c++/v1/stdlib.h:127:1: note: candidate function\n> > > abs(long double __lcpp_x) _NOEXCEPT {\n> > > ^\n> > > 1 error generated.\n> > > ninja: build stopped: subcommand failed.\n> > >\n> > > I can fix while applying with:\n> > >\n> > > - score += abs(info.bitsPerPixel - bitDepth) * penaltyBitDepth;\n> > > + score += abs(static_cast(info.bitsPerPixel - bitDepth)) * penaltyBitDepth;\n> > >\n> > > if you're happy with that, or otherwise if I get any better\n> > > recommendations.\n> >\n> > Does https://libcamera.org/coding-style.html#c-compatibility-headers\n> > (the part about std:: functions) help ?\n>\n> No, I'm afraid it doesn't.\n>\n> Both info.bitsPerPixel and bitDepth are unsigned int.\n> There is no (point to having?) abs(unsigned int); So it has to decide\n> which to map to ... and could pick any of them.\n>\n> I can't see any other solution other than making sure this is an int\n> when passed to abs() currently. That could be done by calculating the\n> diff outside the parameter,\n> int diff = info.bitsPerPixel - bitDepth;\n>\n> or with the cast...\n>\n> --\n> Kieran\n>\n> >\n> > > >\n> > > > if (score <= bestScore) {\n> > > > bestScore = score;\n> > > > @@ -397,9 +392,14 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> > > > * Calculate the best sensor mode we can use based on\n> > > > * the user request.\n> > > > */\n> > > > - V4L2SubdeviceFormat sensorFormat = findBestFormat(data_->sensorFormats_, cfg.size);\n> > > > + const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);\n> > > > + unsigned int bitDepth = info.isValid() ? info.bitsPerPixel : defaultRawBitDepth;\n> > > > + V4L2SubdeviceFormat sensorFormat = findBestFormat(data_->sensorFormats_, cfg.size, bitDepth);\n> > > > + BayerFormat::Packing packing = BayerFormat::Packing::CSI2;\n> > > > + if (info.isValid() && !info.packed)\n> > > > + packing = BayerFormat::Packing::None;\n> > > > V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat,\n> > > > - BayerFormat::Packing::CSI2);\n> > > > + packing);\n> > > > int ret = data_->unicam_[Unicam::Image].dev()->tryFormat(&unicamFormat);\n> > > > if (ret)\n> > > > return Invalid;\n> > > > @@ -533,7 +533,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n> > > > switch (role) {\n> > > > case StreamRole::Raw:\n> > > > size = data->sensor_->resolution();\n> > > > - sensorFormat = findBestFormat(data->sensorFormats_, size);\n> > > > + sensorFormat = findBestFormat(data->sensorFormats_, size, defaultRawBitDepth);\n> > > > pixelFormat = mbusCodeToPixelFormat(sensorFormat.mbus_code,\n> > > > BayerFormat::Packing::CSI2);\n> > > > ASSERT(pixelFormat.isValid());\n> > > > @@ -622,6 +622,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> > > > Size maxSize, sensorSize;\n> > > > unsigned int maxIndex = 0;\n> > > > bool rawStream = false;\n> > > > + unsigned int bitDepth = defaultRawBitDepth;\n> > > >\n> > > > /*\n> > > > * Look for the RAW stream (if given) size as well as the largest\n> > > > @@ -638,7 +639,9 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> > > > sensorSize = cfg.size;\n> > > > rawStream = true;\n> > > > /* Check if the user has explicitly set an unpacked format. */\n> > > > - packing = BayerFormat::fromPixelFormat(cfg.pixelFormat).packing;\n> > > > + BayerFormat bayerFormat = BayerFormat::fromPixelFormat(cfg.pixelFormat);\n> > > > + packing = bayerFormat.packing;\n> > > > + bitDepth = bayerFormat.bitDepth;\n> > > > } else {\n> > > > if (cfg.size > maxSize) {\n> > > > maxSize = config->at(i).size;\n> > > > @@ -664,7 +667,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> > > > }\n> > > >\n> > > > /* First calculate the best sensor mode we can use based on the user request. */\n> > > > - V4L2SubdeviceFormat sensorFormat = findBestFormat(data->sensorFormats_, rawStream ? sensorSize : maxSize);\n> > > > + V4L2SubdeviceFormat sensorFormat = findBestFormat(data->sensorFormats_, rawStream ? sensorSize : maxSize, bitDepth);\n> > > > ret = data->sensor_->setFormat(&sensorFormat);\n> > > > if (ret)\n> > > > return ret;\n> >\n> > --\n> > Regards,\n> >\n> > Laurent Pinchart","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 817BCBF415\n\tfor ;\n\tWed, 1 Dec 2021 14:04:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ED1DA6071A;\n\tWed, 1 Dec 2021 15:04:29 +0100 (CET)","from mail-wr1-x42a.google.com (mail-wr1-x42a.google.com\n\t[IPv6:2a00:1450:4864:20::42a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CD0D16011A\n\tfor ;\n\tWed, 1 Dec 2021 15:04:27 +0100 (CET)","by mail-wr1-x42a.google.com with SMTP id s13so52583197wrb.3\n\tfor ;\n\tWed, 01 Dec 2021 06:04:27 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"fyNdMaY1\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=+MWON/RZ8kTMFmMeeDqSIJv0CNle6kV/co3+Y2xx29c=;\n\tb=fyNdMaY1TLStQuioRWjvdtsNcPAXpIZRXZop+WYMrdTB/o1fwX8rabtOzsYVVTWvw4\n\tBWB+hTaNgpN5qdStdGHRKg1y5IF13tvRrbzTX5yRIzm+cO0wmOo7Ys81ET+OSMpSKLYz\n\tyPh0gecoop5Y9015uWcMeTpag8prSSsi+/sZUC+8jIFiUB5jB+HrCPte+nRD6n9k7ZSx\n\t+QTGn6o/do7HDkGOC+/dCXCh4GFg1pv9P8zNPStvGtLbn7lES8mOEgRhLMAhOBN+WyDP\n\tgXXGh2CjuLl8s9znXoPlAbo1A9qupCGJu836rywZk2dmTw1q4Bdtv9pJMaEFqRhm3RQN\n\t7Z5g==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=+MWON/RZ8kTMFmMeeDqSIJv0CNle6kV/co3+Y2xx29c=;\n\tb=dR3NtuhIfgfHqwb5Rme1bfoUtU4+pWrr0Z+ivbq92KUuV+syaZDonl0HSx6E7s2esR\n\tDx9N35YgAVyk4S20/fB2kgEmNA4dJKywt17wMmvkFxmtdiypRDhLhKVD7Yg3KSjA0oz1\n\tbizdD56VgN4eJTTJs+xKNH7kizyWf4ywpc9OsoS0KMi+pZlbv1y3EfIxkiUmSsRuU9W3\n\tdg4xeaJPs+AVD32cIqBHRZ5wErq17EY4I1gWskAuUXYIUB+4kOa4spMqY9mYyRPgDTjy\n\t99pzMw+dmHMPoad5EzD+WToYhMVnNBfn7GgiCTh3yzjurMbwxALOZdqXs+IWgWVjMMdC\n\tGJYQ==","X-Gm-Message-State":"AOAM532JGmEbfuRgpz56b9nt0k03Ke6H9kfd8KjVt/Ap8ilTDvbMRPzE\n\twC+4gMwwMipAR2Y5ikf0kwmUzLmZazCn7FWyaaeMXn+5VqSmKg==","X-Google-Smtp-Source":"ABdhPJzMb1OjBkGtLztoik5UW4YguevaySWxTO0jqQNerg9+ZTr6gkM01E9FSqJQpoulTN1yfgGznaKmT906ZAbBpfU=","X-Received":"by 2002:a5d:52c2:: with SMTP id r2mr7084561wrv.548.1638367465663;\n\tWed, 01 Dec 2021 06:04:25 -0800 (PST)","MIME-Version":"1.0","References":"<20211201101508.10619-1-david.plowman@raspberrypi.com>\n\t<20211201101508.10619-3-david.plowman@raspberrypi.com>\n\t<163836363085.3059017.15476441460153937095@Monstersaurus>\n\t\n\t<163836711759.3059017.3550724596132324989@Monstersaurus>","In-Reply-To":"<163836711759.3059017.3550724596132324989@Monstersaurus>","From":"David Plowman ","Date":"Wed, 1 Dec 2021 14:04:14 +0000","Message-ID":"","To":"Kieran Bingham ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] pipeline: raspberrypi: Choose\n\tbit depth and packing according to raw stream","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]