[libcamera-devel] [PATCH v1 2/3] libcamera: rpi: pipeline_base: Move findBestFormat to CameraData
Naushir Patuck
naush at raspberrypi.com
Wed Jul 12 13:00:59 CEST 2023
Hi Jacopo,
Thank you for your work!
On Wed, 12 Jul 2023 at 11:55, Naushir Patuck <naush at raspberrypi.com> wrote:
>
> From: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
>
> The findBestFormat() helper operates on the list of sensor formats,
> which is owned by the CameraData class. Move the function to that class
> as well to:
>
> 1) Avoid passing the list of formats to the function
> 2) Remove a static helper in favour of a class function
> 3) Allow subclasses with access to CameraData to call the function
>
> Move to the CameraData class the scoreFormat helper as well.
>
> Signed-off-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> ---
> .../pipeline/rpi/common/pipeline_base.cpp | 140 ++++++++++--------
> .../pipeline/rpi/common/pipeline_base.h | 3 +
> 2 files changed, 79 insertions(+), 64 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> index fb3756a47590..725c1db0d527 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> @@ -74,63 +74,6 @@ bool isMonoSensor(std::unique_ptr<CameraSensor> &sensor)
> return bayer.order == BayerFormat::Order::MONO;
> }
>
> -double scoreFormat(double desired, double actual)
> -{
> - double score = desired - actual;
> - /* Smaller desired dimensions are preferred. */
> - if (score < 0.0)
> - score = (-score) / 8;
> - /* Penalise non-exact matches. */
> - if (actual != desired)
> - score *= 2;
> -
> - return score;
> -}
> -
> -V4L2SubdeviceFormat findBestFormat(const SensorFormats &formatsMap, const Size &req, unsigned int bitDepth)
> -{
> - double bestScore = std::numeric_limits<double>::max(), score;
> - V4L2SubdeviceFormat bestFormat;
> - bestFormat.colorSpace = ColorSpace::Raw;
> -
> - 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) {
> - const unsigned int mbusCode = iter.first;
> - const PixelFormat format = mbusCodeToPixelFormat(mbusCode,
> - BayerFormat::Packing::None);
> - const PixelFormatInfo &info = PixelFormatInfo::info(format);
> -
> - for (const Size &size : iter.second) {
> - double reqAr = static_cast<double>(req.width) / req.height;
> - double fmtAr = static_cast<double>(size.width) / size.height;
> -
> - /* Score the dimensions for closeness. */
> - score = scoreFormat(req.width, size.width);
> - score += scoreFormat(req.height, size.height);
> - score += penaltyAr * scoreFormat(reqAr, fmtAr);
> -
> - /* Add any penalties... this is not an exact science! */
> - score += utils::abs_diff(info.bitsPerPixel, bitDepth) * penaltyBitDepth;
> -
> - if (score <= bestScore) {
> - bestScore = score;
> - bestFormat.mbus_code = mbusCode;
> - bestFormat.size = size;
> - }
> -
> - LOG(RPI, Debug) << "Format: " << size
> - << " fmt " << format
> - << " Score: " << score
> - << " (best " << bestScore << ")";
> - }
> - }
> -
> - return bestFormat;
> -}
> -
> const std::vector<ColorSpace> validColorSpaces = {
> ColorSpace::Sycc,
> ColorSpace::Smpte170m,
> @@ -265,6 +208,17 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> [](auto &l, auto &r) { return l.cfg->size > r.cfg->size; });
>
> /* Do any platform specific fixups. */
> + unsigned int bitDepth = defaultRawBitDepth;
> + if (!rawStreams.empty()) {
> + BayerFormat bayerFormat = BayerFormat::fromPixelFormat(rawStreams[0].cfg->pixelFormat);
> + bitDepth = bayerFormat.bitDepth;
> + }
> +
> + V4L2SubdeviceFormat sensorFormat =
> + data_->findBestFormat(rawStreams.empty() ? outStreams[0].cfg->size
> + : rawStreams[0].cfg->size,
> + bitDepth);
> +
> status = data_->platformValidate(rawStreams, outStreams);
> if (status == Invalid)
> return Invalid;
> @@ -275,8 +229,8 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> V4L2DeviceFormat rawFormat;
>
> const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);
> - unsigned int bitDepth = info.isValid() ? info.bitsPerPixel : defaultRawBitDepth;
> - V4L2SubdeviceFormat sensorFormat = findBestFormat(data_->sensorFormats_, cfg.size, bitDepth);
> + bitDepth = info.isValid() ? info.bitsPerPixel : defaultRawBitDepth;
> + sensorFormat = data_->findBestFormat(cfg.size, bitDepth);
I think the above lines can now be removed as sensorFormat is generated above,
and ought to be the same as if it were computed in this loop. A brief test with
this removed seems to confirm this.
>
> BayerFormat::Packing packing = BayerFormat::fromPixelFormat(cfg.pixelFormat).packing;
> rawFormat = PipelineHandlerBase::toV4L2DeviceFormat(raw.dev, sensorFormat, packing);
> @@ -391,7 +345,7 @@ PipelineHandlerBase::generateConfiguration(Camera *camera, Span<const StreamRole
> switch (role) {
> case StreamRole::Raw:
> size = sensorSize;
> - sensorFormat = findBestFormat(data->sensorFormats_, size, defaultRawBitDepth);
> + sensorFormat = data->findBestFormat(size, defaultRawBitDepth);
> pixelFormat = mbusCodeToPixelFormat(sensorFormat.mbus_code,
> BayerFormat::Packing::CSI2);
> ASSERT(pixelFormat.isValid());
> @@ -531,10 +485,11 @@ int PipelineHandlerBase::configure(Camera *camera, CameraConfiguration *config)
> bitDepth = bayerFormat.bitDepth;
> }
>
> - V4L2SubdeviceFormat sensorFormat = findBestFormat(data->sensorFormats_,
> - rawStreams.empty() ? ispStreams[0].cfg->size
> - : rawStreams[0].cfg->size,
> - bitDepth);
> + V4L2SubdeviceFormat sensorFormat =
> + data->findBestFormat(rawStreams.empty() ? ispStreams[0].cfg->size
> + : rawStreams[0].cfg->size,
> + bitDepth);
> +
> /* Apply any cached transform. */
> const RPiCameraConfiguration *rpiConfig = static_cast<const RPiCameraConfiguration *>(config);
>
> @@ -954,6 +909,63 @@ int PipelineHandlerBase::queueAllBuffers(Camera *camera)
> return 0;
> }
>
> +double CameraData::scoreFormat(double desired, double actual) const
> +{
> + double score = desired - actual;
> + /* Smaller desired dimensions are preferred. */
> + if (score < 0.0)
> + score = (-score) / 8;
> + /* Penalise non-exact matches. */
> + if (actual != desired)
> + score *= 2;
> +
> + return score;
> +}
This does not need to be a member function, but I'm not too bothered either way.
Apart from clarification of the sensorFormat bit above,
Reviewed-by: Naushir Patuck <naush at raspberrypi.com>
Regards,
Naush
> +
> +V4L2SubdeviceFormat CameraData::findBestFormat(const Size &req, unsigned int bitDepth) const
> +{
> + double bestScore = std::numeric_limits<double>::max(), score;
> + V4L2SubdeviceFormat bestFormat;
> + bestFormat.colorSpace = ColorSpace::Raw;
> +
> + constexpr float penaltyAr = 1500.0;
> + constexpr float penaltyBitDepth = 500.0;
> +
> + /* Calculate the closest/best mode from the user requested size. */
> + for (const auto &iter : sensorFormats_) {
> + const unsigned int mbusCode = iter.first;
> + const PixelFormat format = mbusCodeToPixelFormat(mbusCode,
> + BayerFormat::Packing::None);
> + const PixelFormatInfo &info = PixelFormatInfo::info(format);
> +
> + for (const Size &size : iter.second) {
> + double reqAr = static_cast<double>(req.width) / req.height;
> + double fmtAr = static_cast<double>(size.width) / size.height;
> +
> + /* Score the dimensions for closeness. */
> + score = scoreFormat(req.width, size.width);
> + score += scoreFormat(req.height, size.height);
> + score += penaltyAr * scoreFormat(reqAr, fmtAr);
> +
> + /* Add any penalties... this is not an exact science! */
> + score += utils::abs_diff(info.bitsPerPixel, bitDepth) * penaltyBitDepth;
> +
> + if (score <= bestScore) {
> + bestScore = score;
> + bestFormat.mbus_code = mbusCode;
> + bestFormat.size = size;
> + }
> +
> + LOG(RPI, Debug) << "Format: " << size
> + << " fmt " << format
> + << " Score: " << score
> + << " (best " << bestScore << ")";
> + }
> + }
> +
> + return bestFormat;
> +}
> +
> void CameraData::freeBuffers()
> {
> if (ipa_) {
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h
> index f648e81054bb..2eda3cd89812 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h
> @@ -81,6 +81,9 @@ public:
> virtual void platformStart() = 0;
> virtual void platformStop() = 0;
>
> + double scoreFormat(double desired, double actual) const;
> + V4L2SubdeviceFormat findBestFormat(const Size &req, unsigned int bitDepth) const;
> +
> void freeBuffers();
> virtual void platformFreeBuffers() = 0;
>
> --
> 2.34.1
>
More information about the libcamera-devel
mailing list