[libcamera-devel] [PATCH] libcamera: stream: Expose stride value

Nicolas Dufresne nicolas at ndufresne.ca
Fri May 1 17:05:47 CEST 2020


Le vendredi 01 mai 2020 à 16:30 +0200, Niklas Söderlund a écrit :
> 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
> assignment of video devices takes place at this point.
> 
> In the future video devices should be assign at configuration validation
> 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;

In my opinion, as we have full knowledge of it already, we should
directly start with an array of up to 4 strides. We can implement
extrapolation for non-mplane V4L2 formats. This way we don't need to
port the userspace bits twice.

>  
>  	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;
>  
> @@ -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
> + * \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



More information about the libcamera-devel mailing list