[libcamera-devel,v1,1/3] libcamera: rpi: pipeline_base: Remove populateSensorFormats()
diff mbox series

Message ID 20230712105510.14658-2-naush@raspberrypi.com
State Accepted
Headers show
Series
  • Raspberry Pi: Configuration simplifications
Related show

Commit Message

Naushir Patuck July 12, 2023, 10:55 a.m. UTC 
From: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

populateSensorFormats() is a static helper that is called from a single
place and performs a simple loop over the sensor camera formats.

Remove it and in-line it in the caller to remove one static helper from
the pipeline_base.cpp file.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 .../pipeline/rpi/common/pipeline_base.cpp         | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

Comments

Naushir Patuck July 12, 2023, 10:56 a.m. UTC | #1 
Hi Jacopo,

Thank you for your work.

On Wed, 12 Jul 2023 at 11:55, Naushir Patuck <naush@raspberrypi.com> wrote:
>
> From: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>
> populateSensorFormats() is a static helper that is called from a single
> place and performs a simple loop over the sensor camera formats.
>
> Remove it and in-line it in the caller to remove one static helper from
> the pipeline_base.cpp file.
>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Reviewed-by: Naushir Patuck <naush@raspberrypi.com>

> ---
>  .../pipeline/rpi/common/pipeline_base.cpp         | 15 ++++-----------
>  1 file changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> index 179a5b81a516..fb3756a47590 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> @@ -66,16 +66,6 @@ PixelFormat mbusCodeToPixelFormat(unsigned int mbus_code,
>         return pix;
>  }
>
> -SensorFormats populateSensorFormats(std::unique_ptr<CameraSensor> &sensor)
> -{
> -       SensorFormats formats;
> -
> -       for (auto const mbusCode : sensor->mbusCodes())
> -               formats.emplace(mbusCode, sensor->sizes(mbusCode));
> -
> -       return formats;
> -}
> -
>  bool isMonoSensor(std::unique_ptr<CameraSensor> &sensor)
>  {
>         unsigned int mbusCode = sensor->mbusCodes()[0];
> @@ -804,7 +794,10 @@ int PipelineHandlerBase::registerCamera(std::unique_ptr<RPi::CameraData> &camera
>         if (data->sensor_->init())
>                 return -EINVAL;
>
> -       data->sensorFormats_ = populateSensorFormats(data->sensor_);
> +       /* Populate the map of sensor supported formats and sizes. */
> +       for (auto const mbusCode : data->sensor_->mbusCodes())
> +               data->sensorFormats_.emplace(mbusCode,
> +                                            data->sensor_->sizes(mbusCode));
>
>         /*
>          * Enumerate all the Video Mux/Bridge devices across the sensor -> Fr
> --
> 2.34.1
>
Kieran Bingham July 14, 2023, 1:18 p.m. UTC | #2 
Quoting Naushir Patuck via libcamera-devel (2023-07-12 11:56:16)
> Hi Jacopo,
> 
> Thank you for your work.
> 
> On Wed, 12 Jul 2023 at 11:55, Naushir Patuck <naush@raspberrypi.com> wrote:
> >
> > From: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> >
> > populateSensorFormats() is a static helper that is called from a single
> > place and performs a simple loop over the sensor camera formats.
> >
> > Remove it and in-line it in the caller to remove one static helper from
> > the pipeline_base.cpp file.
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> 
> Reviewed-by: Naushir Patuck <naush@raspberrypi.com>

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> > ---
> >  .../pipeline/rpi/common/pipeline_base.cpp         | 15 ++++-----------
> >  1 file changed, 4 insertions(+), 11 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > index 179a5b81a516..fb3756a47590 100644
> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > @@ -66,16 +66,6 @@ PixelFormat mbusCodeToPixelFormat(unsigned int mbus_code,
> >         return pix;
> >  }
> >
> > -SensorFormats populateSensorFormats(std::unique_ptr<CameraSensor> &sensor)
> > -{
> > -       SensorFormats formats;
> > -
> > -       for (auto const mbusCode : sensor->mbusCodes())
> > -               formats.emplace(mbusCode, sensor->sizes(mbusCode));
> > -
> > -       return formats;
> > -}
> > -
> >  bool isMonoSensor(std::unique_ptr<CameraSensor> &sensor)
> >  {
> >         unsigned int mbusCode = sensor->mbusCodes()[0];
> > @@ -804,7 +794,10 @@ int PipelineHandlerBase::registerCamera(std::unique_ptr<RPi::CameraData> &camera
> >         if (data->sensor_->init())
> >                 return -EINVAL;
> >
> > -       data->sensorFormats_ = populateSensorFormats(data->sensor_);
> > +       /* Populate the map of sensor supported formats and sizes. */
> > +       for (auto const mbusCode : data->sensor_->mbusCodes())
> > +               data->sensorFormats_.emplace(mbusCode,
> > +                                            data->sensor_->sizes(mbusCode));
> >
> >         /*
> >          * Enumerate all the Video Mux/Bridge devices across the sensor -> Fr
> > --
> > 2.34.1
> >

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
index 179a5b81a516..fb3756a47590 100644
--- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
+++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
@@ -66,16 +66,6 @@  PixelFormat mbusCodeToPixelFormat(unsigned int mbus_code,
 	return pix;
 }
 
-SensorFormats populateSensorFormats(std::unique_ptr<CameraSensor> &sensor)
-{
-	SensorFormats formats;
-
-	for (auto const mbusCode : sensor->mbusCodes())
-		formats.emplace(mbusCode, sensor->sizes(mbusCode));
-
-	return formats;
-}
-
 bool isMonoSensor(std::unique_ptr<CameraSensor> &sensor)
 {
 	unsigned int mbusCode = sensor->mbusCodes()[0];
@@ -804,7 +794,10 @@  int PipelineHandlerBase::registerCamera(std::unique_ptr<RPi::CameraData> &camera
 	if (data->sensor_->init())
 		return -EINVAL;
 
-	data->sensorFormats_ = populateSensorFormats(data->sensor_);
+	/* Populate the map of sensor supported formats and sizes. */
+	for (auto const mbusCode : data->sensor_->mbusCodes())
+		data->sensorFormats_.emplace(mbusCode,
+					     data->sensor_->sizes(mbusCode));
 
 	/*
 	 * Enumerate all the Video Mux/Bridge devices across the sensor -> Fr