[libcamera-devel] [PATCH v2 2/4] libcamera: rkisp1: Assign sizes to roles
Paul Elder
paul.elder at ideasonboard.com
Fri Mar 3 09:34:47 CET 2023
On Wed, Feb 22, 2023 at 04:19:15PM +0100, Jacopo Mondi via libcamera-devel wrote:
> Currently each RkISP1 path (main and self) generate a configuration
> by bounding the sensor's resolution to their respective maximum output
> aspect ratio and size.
>
> Size maxResolution = maxResolution_.boundedToAspectRatio(resolution)
> .boundedTo(resolution);
>
> In the case of self path, whose maximum size is 1920x1920, the generated
> configuration could get assigned unusual sizes, as the result of the
> above operation
>
> As an example, with the imx258 sensor whose resolution is 4208x3118, the
> resulting size for the Viewfinder use case is an unusual 1920x1423.
>
> Fix this by assigning to each role a desired output size:
> - Use the sensor's resolution for StillCapture and Raw
> - Use 1080p for Viewfinder and VideoRecording
>
> Signed-off-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 12 +++++++++++-
> src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 10 ++++++++--
> src/libcamera/pipeline/rkisp1/rkisp1_path.h | 1 +
> 3 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 569fb8ecb629..05a7ba03b2d2 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -162,6 +162,8 @@ public:
> bool match(DeviceEnumerator *enumerator) override;
>
> private:
> + static constexpr Size kRkISP1PreviewSize = { 1920, 1080 };
> +
> RkISP1CameraData *cameraData(Camera *camera)
> {
> return static_cast<RkISP1CameraData *>(camera->_d());
> @@ -634,12 +636,15 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera,
>
> for (const StreamRole role : roles) {
> bool useMainPath = mainPathAvailable;
> + Size size;
>
> switch (role) {
> case StreamRole::StillCapture:
> /* JPEG encoders typically expect sYCC. */
> if (!colorSpace)
> colorSpace = ColorSpace::Sycc;
> +
> + size = data->sensor_->resolution();
> break;
>
> case StreamRole::Viewfinder:
> @@ -649,12 +654,16 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera,
> */
> if (!colorSpace)
> colorSpace = ColorSpace::Sycc;
> +
> + size = kRkISP1PreviewSize;
> break;
>
> case StreamRole::VideoRecording:
> /* Rec. 709 is a good default for HD video recording. */
> if (!colorSpace)
> colorSpace = ColorSpace::Rec709;
> +
> + size = kRkISP1PreviewSize;
> break;
>
> case StreamRole::Raw:
> @@ -665,6 +674,7 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera,
> }
>
> colorSpace = ColorSpace::Raw;
> + size = data->sensor_->resolution();
> break;
>
> default:
> @@ -683,7 +693,7 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera,
> }
>
> StreamConfiguration cfg =
> - path->generateConfiguration(data->sensor_.get(), role);
> + path->generateConfiguration(data->sensor_.get(), size, role);
> if (!cfg.pixelFormat.isValid())
> return nullptr;
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> index 5079b268c464..a27ac6fc35cb 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> @@ -127,15 +127,21 @@ void RkISP1Path::populateFormats()
> }
>
> StreamConfiguration
> -RkISP1Path::generateConfiguration(const CameraSensor *sensor, StreamRole role)
> +RkISP1Path::generateConfiguration(const CameraSensor *sensor,
> + const Size &size,
> + StreamRole role)
> {
> const std::vector<unsigned int> &mbusCodes = sensor->mbusCodes();
> const Size &resolution = sensor->resolution();
>
> + /* Min and max resolutions to populate the available stream formats. */
> Size maxResolution = maxResolution_.boundedToAspectRatio(resolution)
> .boundedTo(resolution);
> Size minResolution = minResolution_.expandedToAspectRatio(resolution);
>
> + /* The desired stream size, bound to the max resolution. */
> + Size streamSize = size.boundedTo(maxResolution);
> +
> /* Create the list of supported stream formats. */
> std::map<PixelFormat, std::vector<SizeRange>> streamFormats;
> unsigned int rawBitsPerPixel = 0;
> @@ -189,7 +195,7 @@ RkISP1Path::generateConfiguration(const CameraSensor *sensor, StreamRole role)
> StreamFormats formats(streamFormats);
> StreamConfiguration cfg(formats);
> cfg.pixelFormat = format;
> - cfg.size = maxResolution;
> + cfg.size = streamSize;
> cfg.bufferCount = RKISP1_BUFFER_COUNT;
>
> return cfg;
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> index bdf3f95b95e1..cd77957ee4a6 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> @@ -41,6 +41,7 @@ public:
> bool isEnabled() const { return link_->flags() & MEDIA_LNK_FL_ENABLED; }
>
> StreamConfiguration generateConfiguration(const CameraSensor *sensor,
> + const Size &resolution,
> StreamRole role);
> CameraConfiguration::Status validate(const CameraSensor *sensor,
> StreamConfiguration *cfg);
> --
> 2.39.0
>
More information about the libcamera-devel
mailing list