[libcamera-devel,v5,13/23] libcamera: raspberrypi: Fill stride and frameSize at config validation
diff mbox series

Message ID 20200709132835.112593-14-paul.elder@ideasonboard.com
State Accepted
Headers show
Series
  • Clean up formats in v4l2-compat and pipeline handlers
Related show

Commit Message

Paul Elder July 9, 2020, 1:28 p.m. UTC 
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.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

---
Changes in v5:
- clean up code a bit
- move setting default size and width into separate patch

Changes in v4:
- mention motivation in commit message
- also fill stride and frameSize in the default format

New in v3
---
 .../pipeline/raspberrypi/raspberrypi.cpp      | 34 +++++++++++++++----
 1 file changed, 27 insertions(+), 7 deletions(-)

Comments

Laurent Pinchart July 9, 2020, 6:24 p.m. UTC | #1 
Hi Paul,

Thank you for the patch.

On Thu, Jul 09, 2020 at 10:28:25PM +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.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> Changes in v5:
> - clean up code a bit
> - move setting default size and width into separate patch
> 
> Changes in v4:
> - mention motivation in commit message
> - also fill stride and frameSize in the default format
> 
> New in v3
> ---
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 34 +++++++++++++++----
>  1 file changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 1822ac9..a08ad6a 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,34 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
>  		 * have that fixed up in the code above.
>  		 *
>  		 */
> -		PixelFormat &cfgPixFmt = config_.at(outSize[i].first).pixelFormat;
> -		V4L2PixFmtMap fmts;
> +		StreamConfiguration &cfg = config_.at(outSize[i].first);
> +		PixelFormat &cfgPixFmt = cfg.pixelFormat;
> +		V4L2VideoDevice *dev;
>  
>  		if (i == maxIndex)
> -			fmts = data_->isp_[Isp::Output0].dev()->formats();
> +			dev = data_->isp_[Isp::Output0].dev();
>  		else
> -			fmts = data_->isp_[Isp::Output1].dev()->formats();
> +			dev = data_->isp_[Isp::Output1].dev();
> +
> +		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;
>  			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 +678,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 +701,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 +726,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);
>  		}
>  	}
Naushir Patuck July 10, 2020, 6:56 a.m. UTC | #2 
Hi Paul,

Thank you for the patch.

On Thu, 9 Jul 2020 at 14:29, Paul Elder <paul.elder@ideasonboard.com> 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.
>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

Reviewed-by: Naushir Patuck <naush@raspberrypi.com>

>
> ---
> Changes in v5:
> - clean up code a bit
> - move setting default size and width into separate patch
>
> Changes in v4:
> - mention motivation in commit message
> - also fill stride and frameSize in the default format
>
> New in v3
> ---
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 34 +++++++++++++++----
>  1 file changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 1822ac9..a08ad6a 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,34 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
>                  * have that fixed up in the code above.
>                  *
>                  */
> -               PixelFormat &cfgPixFmt = config_.at(outSize[i].first).pixelFormat;
> -               V4L2PixFmtMap fmts;
> +               StreamConfiguration &cfg = config_.at(outSize[i].first);
> +               PixelFormat &cfgPixFmt = cfg.pixelFormat;
> +               V4L2VideoDevice *dev;
>
>                 if (i == maxIndex)
> -                       fmts = data_->isp_[Isp::Output0].dev()->formats();
> +                       dev = data_->isp_[Isp::Output0].dev();
>                 else
> -                       fmts = data_->isp_[Isp::Output1].dev()->formats();
> +                       dev = data_->isp_[Isp::Output1].dev();
> +
> +               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;
>                         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 +678,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 +701,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 +726,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);
>                 }
>         }
> --
> 2.27.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index 1822ac9..a08ad6a 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,34 @@  CameraConfiguration::Status RPiCameraConfiguration::validate()
 		 * have that fixed up in the code above.
 		 *
 		 */
-		PixelFormat &cfgPixFmt = config_.at(outSize[i].first).pixelFormat;
-		V4L2PixFmtMap fmts;
+		StreamConfiguration &cfg = config_.at(outSize[i].first);
+		PixelFormat &cfgPixFmt = cfg.pixelFormat;
+		V4L2VideoDevice *dev;
 
 		if (i == maxIndex)
-			fmts = data_->isp_[Isp::Output0].dev()->formats();
+			dev = data_->isp_[Isp::Output0].dev();
 		else
-			fmts = data_->isp_[Isp::Output1].dev()->formats();
+			dev = data_->isp_[Isp::Output1].dev();
+
+		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;
 			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 +678,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 +701,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 +726,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);
 		}
 	}