[libcamera-devel,v3,16/22] libcamera: pipeline: rkisp1: Move path configuration to RkISP1Path
diff mbox series

Message ID 20200925014207.1455796-17-niklas.soderlund@ragnatech.se
State Accepted
Headers show
Series
  • libcamera: pipeline: rkisp1: Extend to support two streams
Related show

Commit Message

Niklas Söderlund Sept. 25, 2020, 1:42 a.m. UTC 
Move the path configuration to RkISP1Path to increase code reuse and
make the V4L2 subdevice resizer private to the path. There is no
functional change.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp     | 55 +++-----------------
 src/libcamera/pipeline/rkisp1/rkisp1path.cpp | 55 +++++++++++++++++++-
 src/libcamera/pipeline/rkisp1/rkisp1path.h   | 10 +++-
 3 files changed, 68 insertions(+), 52 deletions(-)

Comments

Laurent Pinchart Sept. 28, 2020, 10:19 p.m. UTC | #1 
Hi Niklas,

Thank you for the patch.

On Fri, Sep 25, 2020 at 03:42:01AM +0200, Niklas Söderlund wrote:
> Move the path configuration to RkISP1Path to increase code reuse and
> make the V4L2 subdevice resizer private to the path. There is no
> functional change.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

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

> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp     | 55 +++-----------------
>  src/libcamera/pipeline/rkisp1/rkisp1path.cpp | 55 +++++++++++++++++++-
>  src/libcamera/pipeline/rkisp1/rkisp1path.h   | 10 +++-
>  3 files changed, 68 insertions(+), 52 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index e738a7eb19264d79..2403eec4691b5ff6 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -810,60 +810,17 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>  	data->mainPathActive_ = false;
>  	data->selfPathActive_ = false;
>  	for (const StreamConfiguration &cfg : *config) {
> -		V4L2SubdeviceFormat ispFormat = format;
> -		V4L2Subdevice *resizer;
> -		V4L2VideoDevice *video;
> -
>  		if (cfg.stream() == &data->mainPathStream_) {
> -			resizer = mainPath_.resizer_;
> -			video = mainPath_.video_;
> +			ret = mainPath_.configure(cfg, format);
> +			if (ret)
> +				return ret;
>  			data->mainPathActive_ = true;
>  		} else {
> -			resizer = selfPath_.resizer_;
> -			video = selfPath_.video_;
> +			ret = selfPath_.configure(cfg, format);
> +			if (ret)
> +				return ret;
>  			data->selfPathActive_ = true;
>  		}
> -
> -		ret = resizer->setFormat(0, &ispFormat);
> -		if (ret < 0)
> -			return ret;
> -
> -		const char *name = resizer == mainPath_.resizer_ ? "main" : "self";
> -
> -		LOG(RkISP1, Debug)
> -			<< "Configured " << name << " resizer input pad with "
> -			<< ispFormat.toString();
> -
> -		ispFormat.size = cfg.size;
> -
> -		LOG(RkISP1, Debug)
> -			<< "Configuring " << name << " resizer output pad with "
> -			<< ispFormat.toString();
> -
> -		ret = resizer->setFormat(1, &ispFormat);
> -		if (ret < 0)
> -			return ret;
> -
> -		LOG(RkISP1, Debug)
> -			<< "Configured " << name << " resizer output pad with "
> -			<< ispFormat.toString();
> -
> -		const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);
> -		V4L2DeviceFormat outputFormat = {};
> -		outputFormat.fourcc = video->toV4L2PixelFormat(cfg.pixelFormat);
> -		outputFormat.size = cfg.size;
> -		outputFormat.planesCount = info.numPlanes();
> -
> -		ret = video->setFormat(&outputFormat);
> -		if (ret)
> -			return ret;
> -
> -		if (outputFormat.size != cfg.size ||
> -		    outputFormat.fourcc != video->toV4L2PixelFormat(cfg.pixelFormat)) {
> -			LOG(RkISP1, Error)
> -				<< "Unable to configure capture in " << cfg.toString();
> -			return -EINVAL;
> -		}
>  	}
>  
>  	V4L2DeviceFormat paramFormat = {};
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1path.cpp
> index 51a75df86baaaa7b..758580934817ed6a 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1path.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1path.cpp
> @@ -7,14 +7,18 @@
>  
>  #include "rkisp1path.h"
>  
> +#include <libcamera/stream.h>
> +
>  #include "libcamera/internal/media_device.h"
>  #include "libcamera/internal/v4l2_subdevice.h"
>  #include "libcamera/internal/v4l2_videodevice.h"
>  
>  namespace libcamera {
>  
> +LOG_DECLARE_CATEGORY(RkISP1)
> +
>  RkISP1Path::RkISP1Path(const char *name)
> -	: resizer_(nullptr), video_(nullptr), name_(name)
> +	: video_(nullptr), name_(name), resizer_(nullptr)
>  {
>  }
>  
> @@ -40,6 +44,55 @@ bool RkISP1Path::init(MediaDevice *media)
>  	return true;
>  }
>  
> +int RkISP1Path::configure(const StreamConfiguration &config,
> +			  const V4L2SubdeviceFormat &inputFormat)
> +{
> +	int ret;
> +
> +	V4L2SubdeviceFormat ispFormat = inputFormat;
> +
> +	ret = resizer_->setFormat(0, &ispFormat);
> +	if (ret < 0)
> +		return ret;
> +
> +	LOG(RkISP1, Debug)
> +		<< "Configured " << name_ << " resizer input pad with "
> +		<< ispFormat.toString();
> +
> +	ispFormat.size = config.size;
> +
> +	LOG(RkISP1, Debug)
> +		<< "Configuring " << name_ << " resizer output pad with "
> +		<< ispFormat.toString();
> +
> +	ret = resizer_->setFormat(1, &ispFormat);
> +	if (ret < 0)
> +		return ret;
> +
> +	LOG(RkISP1, Debug)
> +		<< "Configured " << name_ << " resizer output pad with "
> +		<< ispFormat.toString();
> +
> +	const PixelFormatInfo &info = PixelFormatInfo::info(config.pixelFormat);
> +	V4L2DeviceFormat outputFormat = {};
> +	outputFormat.fourcc = video_->toV4L2PixelFormat(config.pixelFormat);
> +	outputFormat.size = config.size;
> +	outputFormat.planesCount = info.numPlanes();
> +
> +	ret = video_->setFormat(&outputFormat);
> +	if (ret)
> +		return ret;
> +
> +	if (outputFormat.size != config.size ||
> +	    outputFormat.fourcc != video_->toV4L2PixelFormat(config.pixelFormat)) {
> +		LOG(RkISP1, Error)
> +			<< "Unable to configure capture in " << config.toString();
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  RkISP1MainPath::RkISP1MainPath()
>  	: RkISP1Path("main")
>  {
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1path.h b/src/libcamera/pipeline/rkisp1/rkisp1path.h
> index d3172e228a3f67bf..6eb01529d2fddb1c 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1path.h
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1path.h
> @@ -12,6 +12,8 @@ namespace libcamera {
>  class MediaDevice;
>  class V4L2Subdevice;
>  class V4L2VideoDevice;
> +struct StreamConfiguration;
> +struct V4L2SubdeviceFormat;
>  
>  class RkISP1Path
>  {
> @@ -21,12 +23,16 @@ public:
>  
>  	bool init(MediaDevice *media);
>  
> -	/* \todo Make resizer and video private. */
> -	V4L2Subdevice *resizer_;
> +	int configure(const StreamConfiguration &config,
> +		      const V4L2SubdeviceFormat &inputFormat);
> +
> +	/* \todo Make video private. */
>  	V4L2VideoDevice *video_;
>  
>  private:
>  	const char *name_;
> +
> +	V4L2Subdevice *resizer_;
>  };
>  
>  class RkISP1MainPath : public RkISP1Path

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index e738a7eb19264d79..2403eec4691b5ff6 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -810,60 +810,17 @@  int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
 	data->mainPathActive_ = false;
 	data->selfPathActive_ = false;
 	for (const StreamConfiguration &cfg : *config) {
-		V4L2SubdeviceFormat ispFormat = format;
-		V4L2Subdevice *resizer;
-		V4L2VideoDevice *video;
-
 		if (cfg.stream() == &data->mainPathStream_) {
-			resizer = mainPath_.resizer_;
-			video = mainPath_.video_;
+			ret = mainPath_.configure(cfg, format);
+			if (ret)
+				return ret;
 			data->mainPathActive_ = true;
 		} else {
-			resizer = selfPath_.resizer_;
-			video = selfPath_.video_;
+			ret = selfPath_.configure(cfg, format);
+			if (ret)
+				return ret;
 			data->selfPathActive_ = true;
 		}
-
-		ret = resizer->setFormat(0, &ispFormat);
-		if (ret < 0)
-			return ret;
-
-		const char *name = resizer == mainPath_.resizer_ ? "main" : "self";
-
-		LOG(RkISP1, Debug)
-			<< "Configured " << name << " resizer input pad with "
-			<< ispFormat.toString();
-
-		ispFormat.size = cfg.size;
-
-		LOG(RkISP1, Debug)
-			<< "Configuring " << name << " resizer output pad with "
-			<< ispFormat.toString();
-
-		ret = resizer->setFormat(1, &ispFormat);
-		if (ret < 0)
-			return ret;
-
-		LOG(RkISP1, Debug)
-			<< "Configured " << name << " resizer output pad with "
-			<< ispFormat.toString();
-
-		const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);
-		V4L2DeviceFormat outputFormat = {};
-		outputFormat.fourcc = video->toV4L2PixelFormat(cfg.pixelFormat);
-		outputFormat.size = cfg.size;
-		outputFormat.planesCount = info.numPlanes();
-
-		ret = video->setFormat(&outputFormat);
-		if (ret)
-			return ret;
-
-		if (outputFormat.size != cfg.size ||
-		    outputFormat.fourcc != video->toV4L2PixelFormat(cfg.pixelFormat)) {
-			LOG(RkISP1, Error)
-				<< "Unable to configure capture in " << cfg.toString();
-			return -EINVAL;
-		}
 	}
 
 	V4L2DeviceFormat paramFormat = {};
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1path.cpp
index 51a75df86baaaa7b..758580934817ed6a 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1path.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1path.cpp
@@ -7,14 +7,18 @@ 
 
 #include "rkisp1path.h"
 
+#include <libcamera/stream.h>
+
 #include "libcamera/internal/media_device.h"
 #include "libcamera/internal/v4l2_subdevice.h"
 #include "libcamera/internal/v4l2_videodevice.h"
 
 namespace libcamera {
 
+LOG_DECLARE_CATEGORY(RkISP1)
+
 RkISP1Path::RkISP1Path(const char *name)
-	: resizer_(nullptr), video_(nullptr), name_(name)
+	: video_(nullptr), name_(name), resizer_(nullptr)
 {
 }
 
@@ -40,6 +44,55 @@  bool RkISP1Path::init(MediaDevice *media)
 	return true;
 }
 
+int RkISP1Path::configure(const StreamConfiguration &config,
+			  const V4L2SubdeviceFormat &inputFormat)
+{
+	int ret;
+
+	V4L2SubdeviceFormat ispFormat = inputFormat;
+
+	ret = resizer_->setFormat(0, &ispFormat);
+	if (ret < 0)
+		return ret;
+
+	LOG(RkISP1, Debug)
+		<< "Configured " << name_ << " resizer input pad with "
+		<< ispFormat.toString();
+
+	ispFormat.size = config.size;
+
+	LOG(RkISP1, Debug)
+		<< "Configuring " << name_ << " resizer output pad with "
+		<< ispFormat.toString();
+
+	ret = resizer_->setFormat(1, &ispFormat);
+	if (ret < 0)
+		return ret;
+
+	LOG(RkISP1, Debug)
+		<< "Configured " << name_ << " resizer output pad with "
+		<< ispFormat.toString();
+
+	const PixelFormatInfo &info = PixelFormatInfo::info(config.pixelFormat);
+	V4L2DeviceFormat outputFormat = {};
+	outputFormat.fourcc = video_->toV4L2PixelFormat(config.pixelFormat);
+	outputFormat.size = config.size;
+	outputFormat.planesCount = info.numPlanes();
+
+	ret = video_->setFormat(&outputFormat);
+	if (ret)
+		return ret;
+
+	if (outputFormat.size != config.size ||
+	    outputFormat.fourcc != video_->toV4L2PixelFormat(config.pixelFormat)) {
+		LOG(RkISP1, Error)
+			<< "Unable to configure capture in " << config.toString();
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 RkISP1MainPath::RkISP1MainPath()
 	: RkISP1Path("main")
 {
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1path.h b/src/libcamera/pipeline/rkisp1/rkisp1path.h
index d3172e228a3f67bf..6eb01529d2fddb1c 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1path.h
+++ b/src/libcamera/pipeline/rkisp1/rkisp1path.h
@@ -12,6 +12,8 @@  namespace libcamera {
 class MediaDevice;
 class V4L2Subdevice;
 class V4L2VideoDevice;
+struct StreamConfiguration;
+struct V4L2SubdeviceFormat;
 
 class RkISP1Path
 {
@@ -21,12 +23,16 @@  public:
 
 	bool init(MediaDevice *media);
 
-	/* \todo Make resizer and video private. */
-	V4L2Subdevice *resizer_;
+	int configure(const StreamConfiguration &config,
+		      const V4L2SubdeviceFormat &inputFormat);
+
+	/* \todo Make video private. */
 	V4L2VideoDevice *video_;
 
 private:
 	const char *name_;
+
+	V4L2Subdevice *resizer_;
 };
 
 class RkISP1MainPath : public RkISP1Path