[libcamera-devel,v2,02/11] qcam: main_window: Explicitly name raw buffer
diff mbox series

Message ID 20201015223750.58563-3-kieran.bingham@ideasonboard.com
State Accepted
Headers show
Series
  • Shadowed Variables
Related show

Commit Message

Kieran Bingham Oct. 15, 2020, 10:37 p.m. UTC 
The buffer obtained for queuing to a raw stream aliases the buffer
passed in for the main view finder stream.

Prevent aliasing, and explicitly name the buffer as a separate variable.

Suggested-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 src/qcam/main_window.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Kieran Bingham Oct. 15, 2020, 10:39 p.m. UTC | #1 
On 15/10/2020 23:37, Kieran Bingham wrote:
> The buffer obtained for queuing to a raw stream aliases the buffer
> passed in for the main view finder stream.
> 
> Prevent aliasing, and explicitly name the buffer as a separate variable.
> 
> Suggested-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
>  src/qcam/main_window.cpp | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 0cbdab9a6bce..f140313ede8d 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -769,16 +769,16 @@ void MainWindow::queueRequest(FrameBuffer *buffer)
>  	request->addBuffer(vfStream_, buffer);
>  
>  	if (captureRaw_) {
> -		FrameBuffer *buffer = nullptr;
> +		FrameBuffer *rawBuffer = nullptr;
>  
>  		{
>  			QMutexLocker locker(&mutex_);
>  			if (!freeBuffers_[rawStream_].isEmpty())
> -				buffer = freeBuffers_[rawStream_].dequeue();
> +				rawBuffer = freeBuffers_[rawStream_].dequeue();
>  		}
>  
> -		if (buffer) {
> -			request->addBuffer(rawStream_, buffer);
> +		if (rawBuffer) {
> +			request->addBuffer(rawStream_, rawBuffer);
>  			captureRaw_ = false;
>  		} else {
>  			qWarning() << "No free buffer available for RAW capture";
>
Niklas Söderlund Oct. 15, 2020, 11:25 p.m. UTC | #2 
Hi Kieran,

On 2020-10-15 23:39:09 +0100, Kieran Bingham wrote:
> On 15/10/2020 23:37, Kieran Bingham wrote:
> > The buffer obtained for queuing to a raw stream aliases the buffer
> > passed in for the main view finder stream.
> > 
> > Prevent aliasing, and explicitly name the buffer as a separate variable.
> > 
> > Suggested-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

:-)

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

> 
> > ---
> >  src/qcam/main_window.cpp | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> > index 0cbdab9a6bce..f140313ede8d 100644
> > --- a/src/qcam/main_window.cpp
> > +++ b/src/qcam/main_window.cpp
> > @@ -769,16 +769,16 @@ void MainWindow::queueRequest(FrameBuffer *buffer)
> >  	request->addBuffer(vfStream_, buffer);
> >  
> >  	if (captureRaw_) {
> > -		FrameBuffer *buffer = nullptr;
> > +		FrameBuffer *rawBuffer = nullptr;
> >  
> >  		{
> >  			QMutexLocker locker(&mutex_);
> >  			if (!freeBuffers_[rawStream_].isEmpty())
> > -				buffer = freeBuffers_[rawStream_].dequeue();
> > +				rawBuffer = freeBuffers_[rawStream_].dequeue();
> >  		}
> >  
> > -		if (buffer) {
> > -			request->addBuffer(rawStream_, buffer);
> > +		if (rawBuffer) {
> > +			request->addBuffer(rawStream_, rawBuffer);
> >  			captureRaw_ = false;
> >  		} else {
> >  			qWarning() << "No free buffer available for RAW capture";
> > 
> 
> -- 
> Regards
> --
> Kieran
Laurent Pinchart Oct. 16, 2020, 1:12 a.m. UTC | #3 
Hi Kieran,

Thank you for the patch.

On Thu, Oct 15, 2020 at 11:37:41PM +0100, Kieran Bingham wrote:
> The buffer obtained for queuing to a raw stream aliases the buffer

s/aliases/shadows/

> passed in for the main view finder stream.
> 
> Prevent aliasing, and explicitly name the buffer as a separate variable.

s/aliasing/shadowing/

Could you double-check that no other patch in this series needs a
similar fix in its commit message ?

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

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  src/qcam/main_window.cpp | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 0cbdab9a6bce..f140313ede8d 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -769,16 +769,16 @@ void MainWindow::queueRequest(FrameBuffer *buffer)
>  	request->addBuffer(vfStream_, buffer);
>  
>  	if (captureRaw_) {
> -		FrameBuffer *buffer = nullptr;
> +		FrameBuffer *rawBuffer = nullptr;
>  
>  		{
>  			QMutexLocker locker(&mutex_);
>  			if (!freeBuffers_[rawStream_].isEmpty())
> -				buffer = freeBuffers_[rawStream_].dequeue();
> +				rawBuffer = freeBuffers_[rawStream_].dequeue();
>  		}
>  
> -		if (buffer) {
> -			request->addBuffer(rawStream_, buffer);
> +		if (rawBuffer) {
> +			request->addBuffer(rawStream_, rawBuffer);
>  			captureRaw_ = false;
>  		} else {
>  			qWarning() << "No free buffer available for RAW capture";

Patch
diff mbox series 

diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
index 0cbdab9a6bce..f140313ede8d 100644
--- a/src/qcam/main_window.cpp
+++ b/src/qcam/main_window.cpp
@@ -769,16 +769,16 @@  void MainWindow::queueRequest(FrameBuffer *buffer)
 	request->addBuffer(vfStream_, buffer);
 
 	if (captureRaw_) {
-		FrameBuffer *buffer = nullptr;
+		FrameBuffer *rawBuffer = nullptr;
 
 		{
 			QMutexLocker locker(&mutex_);
 			if (!freeBuffers_[rawStream_].isEmpty())
-				buffer = freeBuffers_[rawStream_].dequeue();
+				rawBuffer = freeBuffers_[rawStream_].dequeue();
 		}
 
-		if (buffer) {
-			request->addBuffer(rawStream_, buffer);
+		if (rawBuffer) {
+			request->addBuffer(rawStream_, rawBuffer);
 			captureRaw_ = false;
 		} else {
 			qWarning() << "No free buffer available for RAW capture";