| Message ID | 20200426223126.17943-1-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | 96980e35ae64df51fa6830622e776506e9a4cf42 |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, On 26/04/2020 23:31, Laurent Pinchart wrote: > If the camera specified on the command line can't be opened, the > MainWindow constructor still proceeds to check the startStopAction_, > which results in MainWindow::startCapture() being called and trying to > use a null camera_ object. Fix this by returning from the constructor as > soon as the error is detected. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/qcam/main_window.cpp | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > index cf39ed7aceca..ed0cad417d62 100644 > --- a/src/qcam/main_window.cpp > +++ b/src/qcam/main_window.cpp > @@ -70,8 +70,10 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options) > > /* Open the camera and start capture. */ > ret = openCamera(); > - if (ret < 0) > + if (ret < 0) { > quit(); > + return; > + } Ah indeed, I see otherwise it would start the stream on the Checked action. I wonder if we fail to start the first camera we shouldn't stay running (displaying our invalid stream icon), rather than quit()ing. But that's a separate patch, and would also require changes in MainWindow::switchCamera(). Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > startStopAction_->setChecked(true); > } >
Hi, On Mon, Apr 27, 2020 at 01:31, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > If the camera specified on the command line can't be opened, the > MainWindow constructor still proceeds to check the startStopAction_, > which results in MainWindow::startCapture() being called and trying to > use a null camera_ object. Fix this by returning from the constructor > as > soon as the error is detected. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com > <mailto:laurent.pinchart@ideasonboard.com>> > --- > src/qcam/main_window.cpp | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > index cf39ed7aceca..ed0cad417d62 100644 > --- a/src/qcam/main_window.cpp > +++ b/src/qcam/main_window.cpp > @@ -70,8 +70,10 @@ MainWindow::MainWindow(CameraManager *cm, const > OptionsParser::Options &options) > > /* Open the camera and start capture. */ > ret = openCamera(); > - if (ret < 0) > + if (ret < 0) { > quit(); > + return; > + } > > startStopAction_->setChecked(true); > } This also fixes a segfault when the "Select Camera" dialog is closed[x] to exit QCam (i.e. no camera is selected). Thanks. Reviewed-by: Umang Jain <email@uajain.com> > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > <mailto:libcamera-devel@lists.libcamera.org> > <https://u15657259.ct.sendgrid.net/ls/click?upn=8H1KCc2bev8KdIveckpOEBeWjI3THEr-2F8W-2FrEpvXj1fUaD8nZSgfyCwFn-2BKX4QPmzP3yuD-2FqUTQ0p4eTt76rvA-3D-3D4tnk_C3wFy2Q4UgRsRLDAYieRZ5Z3EhAWyy0-2FkOzyYc6FPc1dn6ROcAJqKXb9hjP566uP1e5M4N-2F8GT19qzOCb8CuCYu-2FO2uDnfNqUr41Orvj3-2BchlJd76X1SPt3ovYqUp-2F9-2FaDOIC-2FhfNoe6Lbb7ZHsHnUFZTTRE4YObL2n8JsyeH-2BJhJmPjgWxHeMJSRMHDqt3rsxfHS2ZDJeZrSjIv1MKzTldqRXCzan6wC6uBls-2FdrTYEYuvB7fI1SBSh9mIh4SdXE-2FbIXDpIvLEsqVXetrbnNQ-3D-3D>
Hi Umang, On Mon, Apr 27, 2020 at 11:45:20AM +0000, Umang Jain wrote: > On Mon, Apr 27, 2020 at 01:31, Laurent Pinchart wrote: > > If the camera specified on the command line can't be opened, the > > MainWindow constructor still proceeds to check the startStopAction_, > > which results in MainWindow::startCapture() being called and trying to > > use a null camera_ object. Fix this by returning from the constructor > > as soon as the error is detected. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com > > <mailto:laurent.pinchart@ideasonboard.com>> > > --- > > src/qcam/main_window.cpp | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > > index cf39ed7aceca..ed0cad417d62 100644 > > --- a/src/qcam/main_window.cpp > > +++ b/src/qcam/main_window.cpp > > @@ -70,8 +70,10 @@ MainWindow::MainWindow(CameraManager *cm, const > > OptionsParser::Options &options) > > > > /* Open the camera and start capture. */ > > ret = openCamera(); > > - if (ret < 0) > > + if (ret < 0) { > > quit(); > > + return; > > + } > > > > startStopAction_->setChecked(true); > > } > > This also fixes a segfault when the "Select Camera" dialog is closed[x] > to exit QCam (i.e. no camera is selected). Good point. I'll add it to the commit message, thanks for reporting it. > Thanks. > > Reviewed-by: Umang Jain <email@uajain.com>
diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index cf39ed7aceca..ed0cad417d62 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -70,8 +70,10 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options) /* Open the camera and start capture. */ ret = openCamera(); - if (ret < 0) + if (ret < 0) { quit(); + return; + } startStopAction_->setChecked(true); }
If the camera specified on the command line can't be opened, the MainWindow constructor still proceeds to check the startStopAction_, which results in MainWindow::startCapture() being called and trying to use a null camera_ object. Fix this by returning from the constructor as soon as the error is detected. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/qcam/main_window.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)