[PATCH v1] libcamera: pipeline: Put single file pipelines into anonymous namespace

[Jacopo Mondi]

On Fri, Mar 14, 2025 at 06:42:00PM +0100, Barnabás Pőcze wrote:
> Putting a symbol into an anonymous namespace gives its internal linkage.
> This is useful for a couple reasons:
>   * the compiler can do more optimizations (e.g. inlining);
>   * fewer symbols are exported from the DSO;
>   * unused symbols can be better diagnosed.
>
> For example, as it turns out, `PipelineHandlerMaliC55::applyScalerCrop()`
> is not actually used, which was not diagnosed previously.
>
> Similarly, `ISICameraData::pipe()` is also not used, it is removed.
>
> This could be done with the other pipeline handlers, but is not done,
> due to the issues caused by the log category symbols and to avoid
> inconsistencies where half of a pipeline handler is in one namespace
> while the other half is somewhere else.

I'm just afraid once these pipelines grow to be larger than a single
file, whoever will modify this in future might be a bit confused ?

Can we comment that we can only do this for single file pipelines ?
But then, you know, some pipeline handlers are anonymized some are
not, not sure, I'm a bit in two minds here.

Let's see what others think..

>
> Signed-off-by: Barnabás Pőcze <barnabas.pocze at ideasonboard.com>
> ---
>  src/libcamera/pipeline/imx8-isi/imx8-isi.cpp |  13 +--
>  src/libcamera/pipeline/mali-c55/mali-c55.cpp | 104 +------------------
>  src/libcamera/pipeline/simple/simple.cpp     |   6 +-
>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp |   6 +-
>  src/libcamera/pipeline/vimc/vimc.cpp         |   6 +-
>  5 files changed, 20 insertions(+), 115 deletions(-)
>
> diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> index 4e66b3368..421328536 100644
> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> @@ -31,7 +31,9 @@
>
>  #include "linux/media-bus-format.h"
>
> -namespace libcamera {
> +namespace {
> +
> +using namespace libcamera;
>
>  LOG_DEFINE_CATEGORY(ISI)
>
> @@ -50,8 +52,6 @@ public:
>  		streams_.resize(2);
>  	}
>
> -	PipelineHandlerISI *pipe();
> -
>  	int init();
>
>  	unsigned int pipeIndex(const Stream *stream)
> @@ -149,11 +149,6 @@ private:
>   * Camera Data
>   */
>
> -PipelineHandlerISI *ISICameraData::pipe()
> -{
> -	return static_cast<PipelineHandlerISI *>(Camera::Private::pipe());
> -}
> -
>  /* Open and initialize pipe components. */
>  int ISICameraData::init()
>  {
> @@ -1113,4 +1108,4 @@ void PipelineHandlerISI::bufferReady(FrameBuffer *buffer)
>
>  REGISTER_PIPELINE_HANDLER(PipelineHandlerISI, "imx8-isi")
>
> -} /* namespace libcamera */
> +} /* namespace */
> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> index a05e11fcc..17cae9b96 100644
> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> @@ -52,7 +52,9 @@ bool isFormatRaw(const libcamera::PixelFormat &pixFmt)
>
>  } /* namespace */
>
> -namespace libcamera {
> +namespace {
> +
> +using namespace libcamera;
>
>  LOG_DEFINE_CATEGORY(MaliC55)
>
> @@ -677,8 +679,6 @@ private:
>  				     const StreamConfiguration &config,
>  				     V4L2SubdeviceFormat &subdevFormat);
>
> -	void applyScalerCrop(Camera *camera, const ControlList &controls);
> -
>  	bool registerMaliCamera(std::unique_ptr<MaliC55CameraData> data,
>  				const std::string &name);
>  	bool registerTPGCamera(MediaLink *link);
> @@ -1270,102 +1270,6 @@ void PipelineHandlerMaliC55::stopDevice(Camera *camera)
>  	freeBuffers(camera);
>  }
>
> -void PipelineHandlerMaliC55::applyScalerCrop(Camera *camera,
> -					     const ControlList &controls)
> -{
> -	MaliC55CameraData *data = cameraData(camera);
> -
> -	const auto &scalerCrop = controls.get<Rectangle>(controls::ScalerCrop);
> -	if (!scalerCrop)
> -		return;
> -
> -	if (!data->sensor_) {
> -		LOG(MaliC55, Error) << "ScalerCrop not supported for TPG";
> -		return;
> -	}
> -
> -	Rectangle nativeCrop = *scalerCrop;
> -
> -	IPACameraSensorInfo sensorInfo;
> -	int ret = data->sensor_->sensorInfo(&sensorInfo);
> -	if (ret) {
> -		LOG(MaliC55, Error) << "Failed to retrieve sensor info";
> -		return;
> -	}
> -
> -	/*
> -	 * The ScalerCrop rectangle re-scaling in the ISP crop rectangle
> -	 * comes straight from the RPi pipeline handler.
> -	 *
> -	 * Create a version of the crop rectangle aligned to the analogue crop
> -	 * rectangle top-left coordinates and scaled in the [analogue crop to
> -	 * output frame] ratio to take into account binning/skipping on the
> -	 * sensor.
> -	 */
> -	Rectangle ispCrop = nativeCrop.translatedBy(-sensorInfo.analogCrop
> -							       .topLeft());
> -	ispCrop.scaleBy(sensorInfo.outputSize, sensorInfo.analogCrop.size());
> -
> -	/*
> -	 * The crop rectangle should be:
> -	 * 1. At least as big as ispMinCropSize_, once that's been
> -	 *    enlarged to the same aspect ratio.
> -	 * 2. With the same mid-point, if possible.
> -	 * 3. But it can't go outside the sensor area.
> -	 */
> -	Rectangle ispMinCrop{ 0, 0, 640, 480 };
> -	Size minSize = ispMinCrop.size().expandedToAspectRatio(nativeCrop.size());
> -	Size size = ispCrop.size().expandedTo(minSize);
> -	ispCrop = size.centeredTo(ispCrop.center())
> -		      .enclosedIn(Rectangle(sensorInfo.outputSize));
> -
> -	/*
> -	 * As the resizer can't upscale, the crop rectangle has to be larger
> -	 * than the larger stream output size.
> -	 */
> -	Size maxYuvSize;
> -	for (MaliC55Pipe &pipe : pipes_) {
> -		if (!pipe.stream)
> -			continue;
> -
> -		const StreamConfiguration &config = pipe.stream->configuration();
> -		if (isFormatRaw(config.pixelFormat)) {
> -			LOG(MaliC55, Debug) << "Cannot crop with a RAW stream";
> -			return;
> -		}
> -
> -		Size streamSize = config.size;
> -		if (streamSize.width > maxYuvSize.width)
> -			maxYuvSize.width = streamSize.width;
> -		if (streamSize.height > maxYuvSize.height)
> -			maxYuvSize.height = streamSize.height;
> -	}
> -
> -	ispCrop.size().expandTo(maxYuvSize);
> -
> -	/*
> -	 * Now apply the scaler crop to each enabled output. This overrides the
> -	 * crop configuration performed at configure() time and can cause
> -	 * square pixels if the crop rectangle and scaler output FOV ratio are
> -	 * different.
> -	 */
> -	for (MaliC55Pipe &pipe : pipes_) {
> -		if (!pipe.stream)
> -			continue;
> -
> -		/* Create a copy to avoid setSelection() to modify ispCrop. */
> -		Rectangle pipeCrop = ispCrop;
> -		ret = pipe.resizer->setSelection(0, V4L2_SEL_TGT_CROP, &pipeCrop);
> -		if (ret) {
> -			LOG(MaliC55, Error)
> -				<< "Failed to apply crop to "
> -				<< (pipe.stream == &data->frStream_ ?
> -				    "FR" : "DS") << " pipe";
> -			return;
> -		}
> -	}
> -}
> -
>  int PipelineHandlerMaliC55::queueRequestDevice(Camera *camera, Request *request)
>  {
>  	MaliC55CameraData *data = cameraData(camera);
> @@ -1752,4 +1656,4 @@ bool PipelineHandlerMaliC55::match(DeviceEnumerator *enumerator)
>
>  REGISTER_PIPELINE_HANDLER(PipelineHandlerMaliC55, "mali-c55")
>
> -} /* namespace libcamera */
> +} /* namespace */
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 6e039bf35..4d9943d61 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -41,7 +41,9 @@
>  #include "libcamera/internal/v4l2_subdevice.h"
>  #include "libcamera/internal/v4l2_videodevice.h"
>
> -namespace libcamera {
> +namespace {
> +
> +using namespace libcamera;
>
>  LOG_DEFINE_CATEGORY(SimplePipeline)
>
> @@ -1751,4 +1753,4 @@ void SimplePipelineHandler::releasePipeline(SimpleCameraData *data)
>
>  REGISTER_PIPELINE_HANDLER(SimplePipelineHandler, "simple")
>
> -} /* namespace libcamera */
> +} /* namespace */
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index 7470b5627..dedcac89b 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -32,7 +32,9 @@
>  #include "libcamera/internal/sysfs.h"
>  #include "libcamera/internal/v4l2_videodevice.h"
>
> -namespace libcamera {
> +namespace {
> +
> +using namespace libcamera;
>
>  LOG_DEFINE_CATEGORY(UVC)
>
> @@ -804,4 +806,4 @@ void UVCCameraData::imageBufferReady(FrameBuffer *buffer)
>
>  REGISTER_PIPELINE_HANDLER(PipelineHandlerUVC, "uvcvideo")
>
> -} /* namespace libcamera */
> +} /* namespace */
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index 07273bd2b..f86ee5206 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -42,7 +42,9 @@
>  #include "libcamera/internal/v4l2_subdevice.h"
>  #include "libcamera/internal/v4l2_videodevice.h"
>
> -namespace libcamera {
> +namespace {
> +
> +using namespace libcamera;
>
>  LOG_DEFINE_CATEGORY(VIMC)
>
> @@ -646,4 +648,4 @@ void VimcCameraData::paramsComputed([[maybe_unused]] unsigned int id,
>
>  REGISTER_PIPELINE_HANDLER(PipelineHandlerVimc, "vimc")
>
> -} /* namespace libcamera */
> +} /* namespace */
> --
> 2.48.1