| Message ID | 20220706092218.2761216-1-kieran.bingham@ideasonboard.com |
|---|---|
| State | Rejected |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series |
|
| Related | show |
Quoting Kieran Bingham (2022-07-06 10:22:18) > 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@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... Does anyone have any thoughts or insight on this topic? > 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); > } > -- > 2.34.1 >
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@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); > }
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); }
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@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... src/qcam/main_window.cpp | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-)