[libcamera-devel] [PATCH v3 03/16] libcamera: ipu3: Rationalize constant expressions names
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Oct 13 03:20:08 CEST 2021
Hi Jacopo,
Thank you for the patch.
On Mon, Oct 11, 2021 at 05:11:41PM +0200, Jacopo Mondi wrote:
> Following the previous patch that moved all the ImgU-related contants in
> the ImgUDevice class namespace and that aligned their naming scheme to
> the 'kNameOfConstant' scheme, apply the same changes to the other
> components of the IPU3 pipeline handler.
>
> Cosmetic change, no functional changes intended.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> src/libcamera/pipeline/ipu3/cio2.cpp | 6 +++---
> src/libcamera/pipeline/ipu3/cio2.h | 2 +-
> src/libcamera/pipeline/ipu3/ipu3.cpp | 29 ++++++++++++++--------------
> 3 files changed, 18 insertions(+), 19 deletions(-)
>
> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
> index dc62ab197acb..59dda56bdd3d 100644
> --- a/src/libcamera/pipeline/ipu3/cio2.cpp
> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp
> @@ -234,7 +234,7 @@ StreamConfiguration CIO2Device::generateConfiguration(Size size) const
>
> cfg.size = sensorFormat.size;
> cfg.pixelFormat = mbusCodesToPixelFormat.at(sensorFormat.mbus_code);
> - cfg.bufferCount = CIO2_BUFFER_COUNT;
> + cfg.bufferCount = kBufferCount;
>
> return cfg;
> }
> @@ -338,11 +338,11 @@ int CIO2Device::exportBuffers(unsigned int count,
>
> int CIO2Device::start()
> {
> - int ret = output_->exportBuffers(CIO2_BUFFER_COUNT, &buffers_);
> + int ret = output_->exportBuffers(kBufferCount, &buffers_);
> if (ret < 0)
> return ret;
>
> - ret = output_->importBuffers(CIO2_BUFFER_COUNT);
> + ret = output_->importBuffers(kBufferCount);
> if (ret)
> LOG(IPU3, Error) << "Failed to import CIO2 buffers";
>
> diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h
> index 5fffe921f213..ba8f0052c5b3 100644
> --- a/src/libcamera/pipeline/ipu3/cio2.h
> +++ b/src/libcamera/pipeline/ipu3/cio2.h
> @@ -30,7 +30,7 @@ struct StreamConfiguration;
> class CIO2Device
> {
> public:
> - static constexpr unsigned int CIO2_BUFFER_COUNT = 4;
> + static constexpr unsigned int kBufferCount = 4;
>
> CIO2Device();
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index bb3826eee422..6449fa543191 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -39,10 +39,6 @@ namespace libcamera {
>
> LOG_DEFINE_CATEGORY(IPU3)
>
> -static constexpr unsigned int IPU3_BUFFER_COUNT = 4;
> -static constexpr unsigned int IPU3_MAX_STREAMS = 3;
> -static constexpr Size IPU3ViewfinderSize(1280, 720);
> -
> static const ControlInfoMap::Map IPU3Controls = {
> { &controls::draft::PipelineDepth, ControlInfo(2, 3) },
> };
> @@ -93,6 +89,9 @@ private:
> class IPU3CameraConfiguration : public CameraConfiguration
> {
> public:
> + static constexpr unsigned int kBufferCount = 4;
> + static constexpr unsigned int kMaxStreams = 3;
> +
> IPU3CameraConfiguration(IPU3CameraData *data);
>
> Status validate() override;
> @@ -118,7 +117,8 @@ private:
> class PipelineHandlerIPU3 : public PipelineHandler
> {
> public:
> - static constexpr unsigned int V4L2_CID_IPU3_PIPE_MODE = 0x009819c1;
> + static constexpr unsigned int kPipeModeCtrl = 0x009819c1;
This I would keep as-is, as I think it should be moved to the
intel-ipu3.h kernel header.
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> + static constexpr Size kViewfinderSize{ 1280, 720 };
>
> enum IPU3PipeModes {
> IPU3PipeModeVideo = 0,
> @@ -218,8 +218,8 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
> combinedTransform_ = combined;
>
> /* Cap the number of entries to the available streams. */
> - if (config_.size() > IPU3_MAX_STREAMS) {
> - config_.resize(IPU3_MAX_STREAMS);
> + if (config_.size() > kMaxStreams) {
> + config_.resize(kMaxStreams);
> status = Adjusted;
> }
>
> @@ -355,7 +355,7 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
> ImgUDevice::kOutputAlignHeight);
>
> cfg->pixelFormat = formats::NV12;
> - cfg->bufferCount = IPU3_BUFFER_COUNT;
> + cfg->bufferCount = kBufferCount;
> cfg->stride = info.stride(cfg->size.width, 0, 1);
> cfg->frameSize = info.frameSize(cfg->size, 1);
>
> @@ -444,7 +444,7 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,
> size.height = utils::alignDown(size.height - 1,
> ImgUDevice::kOutputMarginHeight);
> pixelFormat = formats::NV12;
> - bufferCount = IPU3_BUFFER_COUNT;
> + bufferCount = IPU3CameraConfiguration::kBufferCount;
> streamFormats[pixelFormat] = { { ImgUDevice::kOutputMinSize, size } };
>
> break;
> @@ -469,11 +469,11 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,
> * capped to the maximum sensor resolution and aligned
> * to the ImgU output constraints.
> */
> - size = sensorResolution.boundedTo(IPU3ViewfinderSize)
> + size = sensorResolution.boundedTo(kViewfinderSize)
> .alignedDownTo(ImgUDevice::kOutputAlignWidth,
> ImgUDevice::kOutputAlignHeight);
> pixelFormat = formats::NV12;
> - bufferCount = IPU3_BUFFER_COUNT;
> + bufferCount = IPU3CameraConfiguration::kBufferCount;
> streamFormats[pixelFormat] = { { ImgUDevice::kOutputMinSize, size } };
>
> break;
> @@ -645,8 +645,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> * \todo Figure out what the 'Still Capture' mode is meant for, and use
> * it accordingly.
> */
> - ctrls.set(V4L2_CID_IPU3_PIPE_MODE,
> - static_cast<int32_t>(IPU3PipeModeVideo));
> + ctrls.set(kPipeModeCtrl, static_cast<int32_t>(IPU3PipeModeVideo));
> ret = imgu->imgu_->setControls(&ctrls);
> if (ret) {
> LOG(IPU3, Error) << "Unable to set pipe_mode control";
> @@ -1005,8 +1004,8 @@ int PipelineHandlerIPU3::initControls(IPU3CameraData *data)
> * Either the smallest margin-aligned size larger than the viewfinder
> * size or the adjusted sensor resolution.
> */
> - minSize = Size(IPU3ViewfinderSize.width + 1,
> - IPU3ViewfinderSize.height + 1)
> + minSize = Size(kViewfinderSize.width + 1,
> + kViewfinderSize.height + 1)
> .alignedUpTo(ImgUDevice::kOutputMarginWidth,
> ImgUDevice::kOutputMarginHeight)
> .boundedTo(minSize);
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list