[libcamera-devel] [PATCH v4 15/21] libcamera: raspberrypi: Fill stride and frameSize at config validation
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Jul 8 17:16:55 CEST 2020
Hi Paul,
Thank you for the patch.
On Wed, Jul 08, 2020 at 10:44:11PM +0900, Paul Elder wrote:
> Fill the stride and frameSize fields of the StreamConfiguration at
> configuration validation time instead of at camera configuration time.
> This allows applications to get the stride when trying a configuration
> without modifying the active configuration of the camera.
>
> Also set a default configuration size, in case the size is zero.
This would normally be split in a separate commit.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
>
> ---
> Changes in v4:
> - mention motivation in commit message
> - also fill stride and frameSize in the default format
>
> New in v3
> ---
> .../pipeline/raspberrypi/raspberrypi.cpp | 45 +++++++++++++++----
> 1 file changed, 37 insertions(+), 8 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 1822ac9..d0ce446 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -427,6 +427,10 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> */
> V4L2PixFmtMap fmts = data_->unicam_[Unicam::Image].dev()->formats();
> V4L2DeviceFormat sensorFormat = findBestMode(fmts, cfg.size);
> + int ret = data_->unicam_[Unicam::Image].dev()->tryFormat(&sensorFormat);
> + if (ret)
> + return Invalid;
> +
> PixelFormat sensorPixFormat = sensorFormat.fourcc.toPixelFormat();
> if (cfg.size != sensorFormat.size ||
> cfg.pixelFormat != sensorPixFormat) {
> @@ -434,6 +438,10 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> cfg.pixelFormat = sensorPixFormat;
> status = Adjusted;
> }
> +
> + cfg.stride = sensorFormat.planes[0].bpl;
> + cfg.frameSize = sensorFormat.planes[0].size;
> +
> rawCount++;
> } else {
> outSize[outCount] = std::make_pair(count, cfg.size);
> @@ -478,19 +486,43 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> * have that fixed up in the code above.
> *
> */
> - PixelFormat &cfgPixFmt = config_.at(outSize[i].first).pixelFormat;
> + StreamConfiguration &cfg = config_.at(outSize[i].first);
> + PixelFormat &cfgPixFmt = cfg.pixelFormat;
> + V4L2VideoDevice *dev;
> V4L2PixFmtMap fmts;
>
> - if (i == maxIndex)
> - fmts = data_->isp_[Isp::Output0].dev()->formats();
> - else
> - fmts = data_->isp_[Isp::Output1].dev()->formats();
> + if (i == maxIndex) {
> + dev = data_->isp_[Isp::Output0].dev();
> + fmts = dev->formats();
> + } else {
> + dev = data_->isp_[Isp::Output1].dev();
> + fmts = dev->formats();
> + }
>
If would remove fmts above and write here
V4L2PixFmtMap fmts = dev->formats();
> if (fmts.find(V4L2PixelFormat::fromPixelFormat(cfgPixFmt, false)) == fmts.end()) {
> /* If we cannot find a native format, use a default one. */
> cfgPixFmt = formats::NV12;
> +
> + const PixelFormatInfo &info = PixelFormatInfo::info(cfgPixFmt);
> + cfg.size.width = 1920;
> + cfg.size.height = 1080;
Why is this needed, isn't the code above supposed to pick a suitable
size ?
> + cfg.stride = info.stride(cfg.size.width, 0);
> + cfg.frameSize = info.frameSize(cfg.size);
I think you can drop those two lines, as you set the stride and
frameSize below.
> +
> status = Adjusted;
> }
> +
> + V4L2DeviceFormat format = {};
> + format.fourcc = dev->toV4L2PixelFormat(cfg.pixelFormat);
> + format.size = cfg.size;
> +
> + int ret = dev->tryFormat(&format);
> + if (ret)
> + return Invalid;
> +
> + cfg.stride = format.planes[0].bpl;
> + cfg.frameSize = format.planes[0].size;
> +
> }
>
> return status;
> @@ -655,7 +687,6 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
>
> if (isRaw(cfg.pixelFormat)) {
> cfg.setStream(&data->isp_[Isp::Input]);
> - cfg.stride = sensorFormat.planes[0].bpl;
> data->isp_[Isp::Input].setExternal(true);
> continue;
> }
> @@ -679,7 +710,6 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> }
>
> cfg.setStream(&data->isp_[Isp::Output0]);
> - cfg.stride = format.planes[0].bpl;
> data->isp_[Isp::Output0].setExternal(true);
> }
>
> @@ -705,7 +735,6 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> */
> if (!cfg.stream()) {
> cfg.setStream(&data->isp_[Isp::Output1]);
> - cfg.stride = format.planes[0].bpl;
> data->isp_[Isp::Output1].setExternal(true);
> }
> }
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list