[libcamera-devel] libcamera: pipeline: simple: Setup links outside for format try loop

Message ID 20200628095204.23527-1-laurent.pinchart@ideasonboard.com
State Accepted
Commit 7283eff090f801ba97a198e4929e64d0d158e063
Headers show
Series
  • [libcamera-devel] libcamera: pipeline: simple: Setup links outside for format try loop
Related show

Commit Message

Laurent Pinchart June 28, 2020, 9:52 a.m. UTC
The SimpleCameraData::init() function needs to setup links along the
pipeline, but doesn't need to repeat that operation for each media bus
code it tries. Move the link setup before the loop.

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

Comments

Niklas Söderlund June 28, 2020, 2:01 p.m. UTC | #1
Hi Laurent,

Thanks for your work.

On 2020-06-28 12:52:04 +0300, Laurent Pinchart wrote:
> The SimpleCameraData::init() function needs to setup links along the
> pipeline, but doesn't need to repeat that operation for each media bus
> code it tries. Move the link setup before the loop.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  src/libcamera/pipeline/simple/simple.cpp | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 1ec8d0f7de03..9274e61462fc 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -250,6 +250,14 @@ int SimpleCameraData::init()
>  	SimpleConverter *converter = pipe->converter();
>  	int ret;
>  
> +	/*
> +	 * Setup links first as some subdev drivers take active links into
> +	 * account to propagate TRY formats. Such is life :-(
> +	 */
> +	ret = setupLinks();
> +	if (ret < 0)
> +		return ret;
> +
>  	/*
>  	 * Enumerate the possible pipeline configurations. For each media bus
>  	 * format supported by the sensor, propagate the formats through the
> @@ -259,14 +267,6 @@ int SimpleCameraData::init()
>  	for (unsigned int code : sensor_->mbusCodes()) {
>  		V4L2SubdeviceFormat format{ code, sensor_->resolution() };
>  
> -		/*
> -		 * Setup links first as some subdev drivers take active links
> -		 * into account to propagate TRY formats. Such is life :-(
> -		 */
> -		ret = setupLinks();
> -		if (ret < 0)
> -			return ret;
> -
>  		ret = setupFormats(&format, V4L2Subdevice::TryFormat);
>  		if (ret < 0) {
>  			LOG(SimplePipeline, Error)
> -- 
> 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/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index 1ec8d0f7de03..9274e61462fc 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -250,6 +250,14 @@  int SimpleCameraData::init()
 	SimpleConverter *converter = pipe->converter();
 	int ret;
 
+	/*
+	 * Setup links first as some subdev drivers take active links into
+	 * account to propagate TRY formats. Such is life :-(
+	 */
+	ret = setupLinks();
+	if (ret < 0)
+		return ret;
+
 	/*
 	 * Enumerate the possible pipeline configurations. For each media bus
 	 * format supported by the sensor, propagate the formats through the
@@ -259,14 +267,6 @@  int SimpleCameraData::init()
 	for (unsigned int code : sensor_->mbusCodes()) {
 		V4L2SubdeviceFormat format{ code, sensor_->resolution() };
 
-		/*
-		 * Setup links first as some subdev drivers take active links
-		 * into account to propagate TRY formats. Such is life :-(
-		 */
-		ret = setupLinks();
-		if (ret < 0)
-			return ret;
-
 		ret = setupFormats(&format, V4L2Subdevice::TryFormat);
 		if (ret < 0) {
 			LOG(SimplePipeline, Error)