[libcamera-devel] [PATCH v8 4/8] libcamera: ipu3: Use roles in stream configuration
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri Apr 19 15:35:27 CEST 2019
Hi Jacopo,
Thank you for the patch.
On Fri, Apr 19, 2019 at 03:25:27PM +0200, Jacopo Mondi wrote:
> Use and inspect the stream roles provided by the application to
> streamConfiguration() to assign streams to their intended roles and
> return a default configuration associated with them.
>
> Support a limited number of usages, with the viewfinder stream able to
> capture both continuous video streams and still images, and the main
> output stream supporting still images only. This is an artificial
> limitation until we figure out the exact capabilities of the hardware.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> src/libcamera/pipeline/ipu3/ipu3.cpp | 124 +++++++++++++++++++++------
> 1 file changed, 98 insertions(+), 26 deletions(-)
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 46384d88dddd..0130a83973ca 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -5,6 +5,7 @@
> * ipu3.cpp - Pipeline handler for Intel IPU3
> */
>
> +#include <algorithm>
> #include <iomanip>
> #include <memory>
> #include <vector>
> @@ -221,34 +222,105 @@ CameraConfiguration
> PipelineHandlerIPU3::streamConfiguration(Camera *camera,
> const std::vector<StreamUsage> &usages)
> {
> - CameraConfiguration configs;
> IPU3CameraData *data = cameraData(camera);
> - StreamConfiguration config = {};
> + CameraConfiguration cameraConfig = {};
> + std::set<IPU3Stream *> streams = {
> + &data->outStream_,
> + &data->vfStream_,
> + };
>
> - /*
> - * FIXME: Soraka: the maximum resolution reported by both sensors
> - * (2592x1944 for ov5670 and 4224x3136 for ov13858) are returned as
> - * default configurations but they're not correctly processed by the
> - * ImgU. Resolutions up tp 2560x1920 have been validated.
> - *
> - * \todo Clarify ImgU alignement requirements.
> - */
> - config.width = 2560;
> - config.height = 1920;
> - config.pixelFormat = V4L2_PIX_FMT_NV12;
> - config.bufferCount = IPU3_BUFFER_COUNT;
> -
> - configs[&data->outStream_] = config;
> - LOG(IPU3, Debug)
> - << "Stream '" << data->outStream_.name_ << "' format set to "
> - << config.toString();
> -
> - configs[&data->vfStream_] = config;
> - LOG(IPU3, Debug)
> - << "Stream '" << data->vfStream_.name_ << "' format set to "
> - << config.toString();
> -
> - return configs;
> + for (const StreamUsage &usage : usages) {
> + StreamConfiguration streamConfig = {};
> + StreamUsage::Role role = usage.role();
> + IPU3Stream *stream = nullptr;
> +
> + switch (role) {
> + case StreamUsage::Role::StillCapture:
> + /*
> + * Don't allow viewfinder or video capture on the
> + * 'output' stream. This is an artificial limitation
> + * until we figure out the capabilities of the
> + * hardware.
This comment is a bit confusing. How about "Pick the output stream by
default as the Viewfinder and VideoRecording roles are not allowed on
the viewfinder stream." ?
> + */
> + if (streams.find(&data->outStream_) != streams.end()) {
> + stream = &data->outStream_;
> + } else if (streams.find(&data->vfStream_) != streams.end()) {
> + stream = &data->vfStream_;
> + } else {
> + LOG(IPU3, Error)
> + << "No stream available for requested role "
> + << role;
> + break;
> + }
> +
> + /*
> + * FIXME: Soraka: the maximum resolution reported by
> + * both sensors (2592x1944 for ov5670 and 4224x3136 for
> + * ov13858) are returned as default configurations but
> + * they're not correctly processed by the ImgU.
> + * Resolutions up tp 2560x1920 have been validated.
> + *
> + * \todo Clarify ImgU alignment requirements.
> + */
> + streamConfig.width = 2560;
> + streamConfig.height = 1920;
> +
> + break;
Please add a blank line here.
> + case StreamUsage::Role::Viewfinder:
> + case StreamUsage::Role::VideoRecording: {
> + /*
> + * We can't use the 'output' stream for viewfinder or
> + * video capture usages.
> + *
> + * \todo This is an artificial limitation until we
> + * figure out the exact capabilities of the hardware.
> + */
> + if (streams.find(&data->vfStream_) == streams.end()) {
> + LOG(IPU3, Error)
> + << "No stream available for requested role "
> + << role;
> + break;
> + }
> +
> + stream = &data->vfStream_;
> +
> + /*
> + * Align the requested viewfinder size to the
> + * maximum available sensor resolution and to the
> + * IPU3 alignment constraints.
> + */
> + const Size &res = data->cio2_.sensor_->resolution();
> + unsigned int width = std::min(usage.size().width,
> + res.width);
> + unsigned int height = std::min(usage.size().height,
> + res.height);
> + streamConfig.width = width & ~7;
> + streamConfig.height = height & ~3;
> +
> + break;
> + }
And a blank line here too.
> + default:
> + LOG(IPU3, Error)
> + << "Requested stream role not supported: " << role;
> + break;
> + }
> +
> + if (!stream)
> + return cameraConfig;
This will return a valid configuration if the first role was fulfilled
but the first one wasn't. You need to return CameraConfiguration{} here.
With this fixed,
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> +
> + streams.erase(stream);
> +
> + streamConfig.pixelFormat = V4L2_PIX_FMT_NV12;
> + streamConfig.bufferCount = IPU3_BUFFER_COUNT;
> +
> + cameraConfig[stream] = streamConfig;
> +
> + LOG(IPU3, Debug)
> + << "Stream '" << stream->name_ << "' format set to "
> + << streamConfig.toString();
> + }
> +
> + return cameraConfig;
> }
>
> int PipelineHandlerIPU3::configureStreams(Camera *camera,
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list