[libcamera-devel] [PATCH] libcamera: stream: Expose stride value
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri May 1 16:46:07 CEST 2020
Hi Niklas,
Thank you for the patch.
On Fri, May 01, 2020 at 04:30:21PM +0200, Niklas Söderlund wrote:
> Expose the image stride which may be retrieved after a video device
> have been configured. It may only be retrieved at that point as the
s/have/has/
> assignment of video devices takes place at this point.
>
> In the future video devices should be assign at configuration validation
s/assign/assigned/
> time and the stride value retrieved at that point.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> include/libcamera/stream.h | 1 +
> src/libcamera/pipeline/ipu3/ipu3.cpp | 42 ++++++++++++--------
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 1 +
> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 1 +
> src/libcamera/pipeline/vimc/vimc.cpp | 1 +
> src/libcamera/stream.cpp | 7 ++++
> 6 files changed, 36 insertions(+), 17 deletions(-)
>
> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
> index 18142dc997bb8885..b3cbea3d52294846 100644
> --- a/include/libcamera/stream.h
> +++ b/include/libcamera/stream.h
> @@ -42,6 +42,7 @@ struct StreamConfiguration {
>
> PixelFormat pixelFormat;
> Size size;
> + unsigned int stride;
>
> unsigned int bufferCount;
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index ff33f15f9eac3b68..0f9ffd411a680ad9 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -77,7 +77,8 @@ public:
> int configureInput(const Size &size,
> V4L2DeviceFormat *inputFormat);
> int configureOutput(ImgUOutput *output,
> - const StreamConfiguration &cfg);
> + const StreamConfiguration &cfg,
> + V4L2DeviceFormat *outputFormat);
>
> int allocateBuffers(IPU3CameraData *data, unsigned int bufferCount);
> void freeBuffers(IPU3CameraData *data);
> @@ -546,6 +547,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> IPU3Stream *vfStream = &data->vfStream_;
> CIO2Device *cio2 = &data->cio2_;
> ImgUDevice *imgu = data->imgu_;
> + V4L2DeviceFormat outputFormat;
> int ret;
>
> /*
> @@ -625,12 +627,15 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> * The RAW still capture stream just copies buffers from the
> * internal queue and doesn't need any specific configuration.
> */
> - if (stream->raw_)
> - continue;
> -
> - ret = imgu->configureOutput(stream->device_, cfg);
> - if (ret)
> - return ret;
> + if (stream->raw_) {
> + cfg.stride = cio2Format.planes[0].bpl;
> + } else {
> + ret = imgu->configureOutput(stream->device_, cfg,
> + &outputFormat);
> + if (ret)
> + return ret;
> + cfg.stride = outputFormat.planes[0].bpl;
> + }
> }
>
> /*
> @@ -639,13 +644,15 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> * be at least one active stream in the configuration request).
> */
> if (!outStream->active_) {
> - ret = imgu->configureOutput(outStream->device_, config->at(0));
> + ret = imgu->configureOutput(outStream->device_, config->at(0),
> + &outputFormat);
> if (ret)
> return ret;
> }
>
> if (!vfStream->active_) {
> - ret = imgu->configureOutput(vfStream->device_, config->at(0));
> + ret = imgu->configureOutput(vfStream->device_, config->at(0),
> + &outputFormat);
> if (ret)
> return ret;
> }
> @@ -657,7 +664,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> StreamConfiguration statCfg = {};
> statCfg.size = cio2Format.size;
>
> - ret = imgu->configureOutput(&imgu->stat_, statCfg);
> + ret = imgu->configureOutput(&imgu->stat_, statCfg, &outputFormat);
> if (ret)
> return ret;
>
This isn't very nice, but will do for now as we know we'll have to
rework the code anyway.
> @@ -1166,7 +1173,8 @@ int ImgUDevice::configureInput(const Size &size,
> * \return 0 on success or a negative error code otherwise
> */
> int ImgUDevice::configureOutput(ImgUOutput *output,
> - const StreamConfiguration &cfg)
> + const StreamConfiguration &cfg,
> + V4L2DeviceFormat *outputFormat)
> {
> V4L2VideoDevice *dev = output->dev;
> unsigned int pad = output->pad;
> @@ -1183,17 +1191,17 @@ int ImgUDevice::configureOutput(ImgUOutput *output,
> if (output == &stat_)
> return 0;
>
> - V4L2DeviceFormat outputFormat = {};
> - outputFormat.fourcc = dev->toV4L2PixelFormat(PixelFormat(DRM_FORMAT_NV12));
> - outputFormat.size = cfg.size;
> - outputFormat.planesCount = 2;
> + *outputFormat = {};
> + outputFormat->fourcc = dev->toV4L2PixelFormat(PixelFormat(DRM_FORMAT_NV12));
> + outputFormat->size = cfg.size;
> + outputFormat->planesCount = 2;
>
> - ret = dev->setFormat(&outputFormat);
> + ret = dev->setFormat(outputFormat);
> if (ret)
> return ret;
>
> LOG(IPU3, Debug) << "ImgU " << output->name << " format = "
> - << outputFormat.toString();
> + << outputFormat->toString();
>
> return 0;
> }
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 6aa3178669b40a37..1e81a0048f093d8f 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -681,6 +681,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> return ret;
>
> cfg.setStream(&data->stream_);
> + cfg.stride = outputFormat.planes[0].bpl;
>
> return 0;
> }
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index 455693264c2e3e67..f0c1337de8623a6b 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -204,6 +204,7 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)
> return -EINVAL;
>
> cfg.setStream(&data->stream_);
> + cfg.stride = format.planes[0].bpl;
>
> return 0;
> }
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index ccfd7f86d15860c5..7019ac322e6c2c59 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -253,6 +253,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)
> return ret;
>
> cfg.setStream(&data->stream_);
> + cfg.stride = format.planes[0].bpl;
>
> return 0;
> }
> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> index ef16aaa1b2f4586a..0a2468d92039676c 100644
> --- a/src/libcamera/stream.cpp
> +++ b/src/libcamera/stream.cpp
> @@ -301,6 +301,13 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats)
> * \brief Stream pixel format
> */
>
> +/**
> + * \var StreamConfiguration::stride
> + * \brief Stream stride in bytes
Maybe "Image stride for the stream, in bytes" ?
I would add
* The stride value reports the number of bytes between the beginning of
* successive lines in an image buffer for this stream. The value is
* valid after successfully configuring the camera with this
* configuration with a call to Camera::Configure().
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> + * \todo Update this value when configuration is validated instead of when
> + * the camera is configured.
> + */
> +
> /**
> * \var StreamConfiguration::bufferCount
> * \brief Requested number of buffers to allocate for the stream
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list