[libcamera-devel] [PATCH 02/13] libcamera: pipeline: rkisp1: Breakout mainpath size and format constraints
Jacopo Mondi
jacopo at jmondi.org
Thu Aug 20 10:11:35 CEST 2020
Hi Niklas,
On Thu, Aug 13, 2020 at 02:52:35AM +0200, Niklas Söderlund wrote:
> Breakout the mainpath size and format constrains as it will be used in
> more places then just validate(). While at it use the new helpers to
> validate Size.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 32 +++++++++++++-----------
> 1 file changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index e2b703fc09f7afda..a0e36a44d8d91260 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -37,6 +37,19 @@ namespace libcamera {
>
> LOG_DEFINE_CATEGORY(RkISP1)
>
> +static const Size RKISP1_RSZ_MP_SRC_MIN = { 32, 16 };
> +static const Size RKISP1_RSZ_MP_SRC_MAX = { 4416, 3312 };
You could list-initialize these
> +static const std::array<PixelFormat, 7> RKISP1_RSZ_MP_FORMATS{
All of these can be made constexprs
Also, we usually prefer an anonymous namespace in place of static
Also, are you sure about the ALL_CAPITAL_NAME of the array of formats ?
> + formats::YUYV,
> + formats::YVYU,
> + formats::VYUY,
> + formats::NV16,
> + formats::NV61,
> + formats::NV21,
> + formats::NV12,
> + /* \todo Add support for 8-bit greyscale to DRM formats */
> +};
> +
> class PipelineHandlerRkISP1;
> class RkISP1ActionQueueBuffers;
>
> @@ -461,17 +474,6 @@ RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera,
>
> CameraConfiguration::Status RkISP1CameraConfiguration::validate()
> {
> - static const std::array<PixelFormat, 7> formats{
> - formats::YUYV,
> - formats::YVYU,
> - formats::VYUY,
> - formats::NV16,
> - formats::NV61,
> - formats::NV21,
> - formats::NV12,
> - /* \todo Add support for 8-bit greyscale to DRM formats */
> - };
> -
> const CameraSensor *sensor = data_->sensor_;
> Status status = Valid;
>
> @@ -487,8 +489,8 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
> StreamConfiguration &cfg = config_[0];
>
> /* Adjust the pixel format. */
> - if (std::find(formats.begin(), formats.end(), cfg.pixelFormat) ==
> - formats.end()) {
> + if (std::find(RKISP1_RSZ_MP_FORMATS.begin(), RKISP1_RSZ_MP_FORMATS.end(),
> + cfg.pixelFormat) == RKISP1_RSZ_MP_FORMATS.end()) {
> LOG(RkISP1, Debug) << "Adjusting format to NV12";
> cfg.pixelFormat = formats::NV12,
> status = Adjusted;
> @@ -525,8 +527,8 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
> / sensorFormat_.size.width;
> }
>
> - cfg.size.width = std::max(32U, std::min(4416U, cfg.size.width));
> - cfg.size.height = std::max(16U, std::min(3312U, cfg.size.height));
> + cfg.size.boundTo(RKISP1_RSZ_MP_SRC_MAX);
> + cfg.size.expandTo(RKISP1_RSZ_MP_SRC_MIN);
You could concatenate the two
cfg.size.boundTo().expandTo();
Nits apart, the patch itself looks good!
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
Thanks
j
>
> if (cfg.size != size) {
> LOG(RkISP1, Debug)
> --
> 2.28.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
More information about the libcamera-devel
mailing list