[libcamera-devel,v2,2/4] libcamera: pipeline: rkisp1: Don't hardcode NV12 in configureStreams()

Message ID 20190419102844.6838-2-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • [libcamera-devel,v2,1/4] libcamera: Document documentation style and update the code accordingly
Related show

Commit Message

Laurent Pinchart April 19, 2019, 10:28 a.m. UTC
Use the pixel format requested by the application in the
RkISP1PipelineHandler::configureStreams() method instead of hardcoding
NV12. The streamsConfiguration() method still proposes NV12 by default.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Jacopo Mondi April 19, 2019, 10:38 a.m. UTC | #1
I Laurent,

On Fri, Apr 19, 2019 at 01:28:42PM +0300, Laurent Pinchart wrote:
> Use the pixel format requested by the application in the
> RkISP1PipelineHandler::configureStreams() method instead of hardcoding
> NV12. The streamsConfiguration() method still proposes NV12 by default.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 8ed0ba84780a..51f00fb68402 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -223,7 +223,7 @@ int PipelineHandlerRkISP1::configureStreams(Camera *camera,
>  	V4L2DeviceFormat outputFormat = {};
>  	outputFormat.width = cfg.width;
>  	outputFormat.height = cfg.height;
> -	outputFormat.fourcc = V4L2_PIX_FMT_NV12;
> +	outputFormat.fourcc = cfg.pixelFormat;
>  	outputFormat.planesCount = 2;
>
>  	ret = video_->setFormat(&outputFormat);
> @@ -232,12 +232,12 @@ int PipelineHandlerRkISP1::configureStreams(Camera *camera,
>
>  	if (outputFormat.width != cfg.width ||
>  	    outputFormat.height != cfg.height ||
> -	    outputFormat.fourcc != V4L2_PIX_FMT_NV12) {
> +	    outputFormat.fourcc != cfg.pixelFormat) {
>  		LOG(RkISP1, Error)
>  			<< "Unable to configure capture in " << cfg.width
>  			<< "x" << cfg.height << "-0x"
>  			<< std::hex << std::setfill('0') << std::setw(8)
> -			<< V4L2_PIX_FMT_NV12;
> +			<< cfg.pixelFormat;

After the offline clarification on validating the received
pixelFormat, this seems fine to me. Not supported formats will be
catched by this check, so we're good!

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

>  		return -EINVAL;
>  	}
>
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch

diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 8ed0ba84780a..51f00fb68402 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -223,7 +223,7 @@  int PipelineHandlerRkISP1::configureStreams(Camera *camera,
 	V4L2DeviceFormat outputFormat = {};
 	outputFormat.width = cfg.width;
 	outputFormat.height = cfg.height;
-	outputFormat.fourcc = V4L2_PIX_FMT_NV12;
+	outputFormat.fourcc = cfg.pixelFormat;
 	outputFormat.planesCount = 2;
 
 	ret = video_->setFormat(&outputFormat);
@@ -232,12 +232,12 @@  int PipelineHandlerRkISP1::configureStreams(Camera *camera,
 
 	if (outputFormat.width != cfg.width ||
 	    outputFormat.height != cfg.height ||
-	    outputFormat.fourcc != V4L2_PIX_FMT_NV12) {
+	    outputFormat.fourcc != cfg.pixelFormat) {
 		LOG(RkISP1, Error)
 			<< "Unable to configure capture in " << cfg.width
 			<< "x" << cfg.height << "-0x"
 			<< std::hex << std::setfill('0') << std::setw(8)
-			<< V4L2_PIX_FMT_NV12;
+			<< cfg.pixelFormat;
 		return -EINVAL;
 	}