| Message ID | 20201013151241.3557005-3-kieran.bingham@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, Thank you for the patch. On Tue, Oct 13, 2020 at 04:12:33PM +0100, Kieran Bingham wrote: > The queueRequest function creates FrameBuffer pointer which aliases the > existing and passed in buffer pointer. > > When the captureRaw_ scope is executed, the original FrameBuffer *buffer > is not used any more. > > Re-use it rather than creating a new variable with the same name. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/qcam/main_window.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > index 0cbdab9a6bce..0e50768aa386 100644 > --- a/src/qcam/main_window.cpp > +++ b/src/qcam/main_window.cpp > @@ -769,7 +769,7 @@ void MainWindow::queueRequest(FrameBuffer *buffer) > request->addBuffer(vfStream_, buffer); > > if (captureRaw_) { > - FrameBuffer *buffer = nullptr; > + buffer = nullptr; > > { > QMutexLocker locker(&mutex_);
Hi Kieran, On 2020-10-13 16:12:33 +0100, Kieran Bingham wrote: > The queueRequest function creates FrameBuffer pointer which aliases the > existing and passed in buffer pointer. > > When the captureRaw_ scope is executed, the original FrameBuffer *buffer > is not used any more. > > Re-use it rather than creating a new variable with the same name. I know this is bikeshedding, but I really don't like reusing a function argument. Specially not for a new use-case like this where the 'buffer' argument passed in represents the viewfinder buffer ands the 'buffer' variable used here is the raw buffer. Would it not make more sens to rename this buffer rawBuffer or something similar? > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/qcam/main_window.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > index 0cbdab9a6bce..0e50768aa386 100644 > --- a/src/qcam/main_window.cpp > +++ b/src/qcam/main_window.cpp > @@ -769,7 +769,7 @@ void MainWindow::queueRequest(FrameBuffer *buffer) > request->addBuffer(vfStream_, buffer); > > if (captureRaw_) { > - FrameBuffer *buffer = nullptr; > + buffer = nullptr; > > { > QMutexLocker locker(&mutex_); > -- > 2.25.1 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Niklas, On 14/10/2020 13:22, Niklas Söderlund wrote: > Hi Kieran, > > On 2020-10-13 16:12:33 +0100, Kieran Bingham wrote: >> The queueRequest function creates FrameBuffer pointer which aliases the >> existing and passed in buffer pointer. >> >> When the captureRaw_ scope is executed, the original FrameBuffer *buffer >> is not used any more. >> >> Re-use it rather than creating a new variable with the same name. > > I know this is bikeshedding, but I really don't like reusing a function > argument. Specially not for a new use-case like this where the 'buffer' > argument passed in represents the viewfinder buffer ands the 'buffer' > variable used here is the raw buffer. Would it not make more sens to > rename this buffer rawBuffer or something similar? Not bikeshedding at all I don't think. That looks like a good idea to me. It better clarifies the use of the variable. I'll update. >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> --- >> src/qcam/main_window.cpp | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp >> index 0cbdab9a6bce..0e50768aa386 100644 >> --- a/src/qcam/main_window.cpp >> +++ b/src/qcam/main_window.cpp >> @@ -769,7 +769,7 @@ void MainWindow::queueRequest(FrameBuffer *buffer) >> request->addBuffer(vfStream_, buffer); >> >> if (captureRaw_) { >> - FrameBuffer *buffer = nullptr; >> + buffer = nullptr; >> >> { >> QMutexLocker locker(&mutex_); >> -- >> 2.25.1 >> >> _______________________________________________ >> libcamera-devel mailing list >> libcamera-devel@lists.libcamera.org >> https://lists.libcamera.org/listinfo/libcamera-devel >
diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index 0cbdab9a6bce..0e50768aa386 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -769,7 +769,7 @@ void MainWindow::queueRequest(FrameBuffer *buffer) request->addBuffer(vfStream_, buffer); if (captureRaw_) { - FrameBuffer *buffer = nullptr; + buffer = nullptr; { QMutexLocker locker(&mutex_);
The queueRequest function creates FrameBuffer pointer which aliases the existing and passed in buffer pointer. When the captureRaw_ scope is executed, the original FrameBuffer *buffer is not used any more. Re-use it rather than creating a new variable with the same name. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- src/qcam/main_window.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)