[libcamera-devel] [RFC PATCH] qcam: Fix IPU3 camera switching

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Wed, Jul 06, 2022 at 10:22:18AM +0100, Kieran Bingham via libcamera-devel wrote:
> On the IPU3 we will disallow acquiring a second camera while one is
> active.  This prevents us from doing a camera switch between the
> available cameras.
> 
> Re-instate this facility by ensuring we release the current camera
> before attempting to acquire the new camera.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> 
> QCam is broken on IPU3 in that once one camera is running, we can't swap
> to the other without closing qcam (or swapping to a non IPU3 camera).
> 
> This seems a bit of a pain - but here's a potential fix.
> 
> Since writing this patch (some time ago) I know think that trying to
> reaquire the previous camera if the switch fails might be wrong.
> 
> I wonder if we should just leave the camera halted, with our 'no-image'
> logo displayed, and wait. But the only way to 'fix' this would be for
> the user to choose a new camera, there's no current option to retry
> acquiring a camera...

This looks like a problem on the pipeline handler side, not in qcam. It
turns out that I have a patch in a branch that has been sitting idle
there for way too long, I've posted it ([1]), let's move the discussion
there (and it would be nice if you could test it).

[1] https://lists.libcamera.org/pipermail/libcamera-devel/2022-July/032507.html

>  src/qcam/main_window.cpp | 26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 7433d647e8a0..487f522748ca 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -269,11 +269,6 @@ void MainWindow::switchCamera(int index)
>  
>  	const std::shared_ptr<Camera> &cam = cameras[index];
>  
> -	if (cam->acquire()) {
> -		qInfo() << "Failed to acquire camera" << cam->id().c_str();
> -		return;
> -	}
> -
>  	qInfo() << "Switching to camera" << cam->id().c_str();
>  
>  	/*
> @@ -283,7 +278,26 @@ void MainWindow::switchCamera(int index)
>  	startStopAction_->setChecked(false);
>  
>  	camera_->release();
> -	camera_ = cam;
> +
> +	/*
> +	 * Only attempt to acquire after releasing to ensure we support
> +	 * pipelines which can only stream from one device at a time.
> +	 *
> +	 * Todo: Consider how we could acquire more than one camera
> +	 *       but ensure only one could stream at a time.
> +	 */
> +	if (cam->acquire()) {
> +		qInfo() << "Failed to acquire camera" << cam->id().c_str();
> +		qInfo() << "Switching back to " << camera_->id().c_str();
> +
> +		if (camera_->acquire()) {
> +			qInfo() << "Failed to reacquire " << camera_->id().c_str();
> +			return;
> +		}
> +	} else {
> +		/* Acquire was successful, Switch to this camera */
> +		camera_ = cam;
> +	}
>  
>  	startStopAction_->setChecked(true);
>  }

-- 
Regards,

Laurent Pinchart