| Message ID | 20231113165244.480925-1-bbrotherton@google.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Brett, Thank you for your first libcamera patch :-) On Mon, Nov 13, 2023 at 09:52:44AM -0700, Brett Brotherton via libcamera-devel wrote: > fix -Wdeprecated-this-capture error when building with c++20 by > explicity naming this in the capture Nitpicking, s/fix/Fix/ and s/capture$/capture./ No need to resend the patch for this, I'll update the commit message before pushing. Is this the only occurrence of implicit *this capture ? I'm happy we're not doing that bad :-) > Signed-off-by: Brett Brotherton <bbrotherton@google.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> I will run this through build testing and push the patch if no issues are found. > --- > src/libcamera/pipeline_handler.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > index 9c74c6cf..2220cb92 100644 > --- a/src/libcamera/pipeline_handler.cpp > +++ b/src/libcamera/pipeline_handler.cpp > @@ -649,7 +649,7 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera) > */ > void PipelineHandler::hotplugMediaDevice(MediaDevice *media) > { > - media->disconnected.connect(this, [=]() { mediaDeviceDisconnected(media); }); > + media->disconnected.connect(this, [=, this]() { mediaDeviceDisconnected(media); }); > } > > /**
Hi Laurent, Thanks for the review, and for making the tweaks to the commit message. I believe that was the only instance, with that patch I was able to build with C++20 for ChromeOS. On Tue, Nov 14, 2023 at 3:52 AM Laurent Pinchart < laurent.pinchart@ideasonboard.com> wrote: > Hi Brett, > > Thank you for your first libcamera patch :-) > > On Mon, Nov 13, 2023 at 09:52:44AM -0700, Brett Brotherton via > libcamera-devel wrote: > > fix -Wdeprecated-this-capture error when building with c++20 by > > explicity naming this in the capture > > Nitpicking, s/fix/Fix/ and s/capture$/capture./ > > No need to resend the patch for this, I'll update the commit message > before pushing. > > Is this the only occurrence of implicit *this capture ? I'm happy we're > not doing that bad :-) > > > Signed-off-by: Brett Brotherton <bbrotherton@google.com> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > I will run this through build testing and push the patch if no issues > are found. > > > --- > > src/libcamera/pipeline_handler.cpp | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/src/libcamera/pipeline_handler.cpp > b/src/libcamera/pipeline_handler.cpp > > index 9c74c6cf..2220cb92 100644 > > --- a/src/libcamera/pipeline_handler.cpp > > +++ b/src/libcamera/pipeline_handler.cpp > > @@ -649,7 +649,7 @@ void > PipelineHandler::registerCamera(std::shared_ptr<Camera> camera) > > */ > > void PipelineHandler::hotplugMediaDevice(MediaDevice *media) > > { > > - media->disconnected.connect(this, [=]() { > mediaDeviceDisconnected(media); }); > > + media->disconnected.connect(this, [=, this]() { > mediaDeviceDisconnected(media); }); > > } > > > > /** > > -- > Regards, > > Laurent Pinchart >
On Tue, Nov 14, 2023 at 12:53:04PM +0200, Laurent Pinchart via libcamera-devel wrote: > Hi Brett, > > Thank you for your first libcamera patch :-) > > On Mon, Nov 13, 2023 at 09:52:44AM -0700, Brett Brotherton via libcamera-devel wrote: > > fix -Wdeprecated-this-capture error when building with c++20 by > > explicity naming this in the capture > > Nitpicking, s/fix/Fix/ and s/capture$/capture./ > > No need to resend the patch for this, I'll update the commit message > before pushing. > > Is this the only occurrence of implicit *this capture ? I'm happy we're > not doing that bad :-) > > > Signed-off-by: Brett Brotherton <bbrotherton@google.com> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > I will run this through build testing and push the patch if no issues > are found. Unfortunately problems have been found :-( ../../src/libcamera/pipeline_handler.cpp:652:40: error: explicit capture of 'this' with a capture default of '=' is a C++20 extension [-Werror,-Wc++20-extensions] media->disconnected.connect(this, [=, this]() { mediaDeviceDisconnected(media); }); ^ > > --- > > src/libcamera/pipeline_handler.cpp | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > > index 9c74c6cf..2220cb92 100644 > > --- a/src/libcamera/pipeline_handler.cpp > > +++ b/src/libcamera/pipeline_handler.cpp > > @@ -649,7 +649,7 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera) > > */ > > void PipelineHandler::hotplugMediaDevice(MediaDevice *media) > > { > > - media->disconnected.connect(this, [=]() { mediaDeviceDisconnected(media); }); > > + media->disconnected.connect(this, [=, this]() { mediaDeviceDisconnected(media); }); > > } > > > > /**
Hi 2023. november 14., kedd 14:42 keltezéssel, Laurent Pinchart via libcamera-devel <libcamera-devel@lists.libcamera.org> írta: > On Tue, Nov 14, 2023 at 12:53:04PM +0200, Laurent Pinchart via libcamera-devel wrote: > > > Hi Brett, > > > > Thank you for your first libcamera patch :-) > > > > On Mon, Nov 13, 2023 at 09:52:44AM -0700, Brett Brotherton via libcamera-devel wrote: > > > > > fix -Wdeprecated-this-capture error when building with c++20 by > > > explicity naming this in the capture > > > > Nitpicking, s/fix/Fix/ and s/capture$/capture./ > > > > No need to resend the patch for this, I'll update the commit message > > before pushing. > > > > Is this the only occurrence of implicit *this capture ? I'm happy we're > > not doing that bad :-) > > > > > Signed-off-by: Brett Brotherton bbrotherton@google.com > > > > Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com > > > > I will run this through build testing and push the patch if no issues > > are found. > > > Unfortunately problems have been found :-( > > ../../src/libcamera/pipeline_handler.cpp:652:40: error: explicit capture of 'this' with a capture default of '=' is a C++20 extension [-Werror,-Wc++20-extensions] > media->disconnected.connect(this, =, this { mediaDeviceDisconnected(media); }); > > ^ > I imagine explicitly capturing everything needed should fix that. diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 9c74c6cf..29e0c98a 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -649,7 +649,7 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera) */ void PipelineHandler::hotplugMediaDevice(MediaDevice *media) { - media->disconnected.connect(this, [=]() { mediaDeviceDisconnected(media); }); + media->disconnected.connect(this, [this, media] { mediaDeviceDisconnected(media); }); } /** > > > --- > > > src/libcamera/pipeline_handler.cpp | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > > > index 9c74c6cf..2220cb92 100644 > > > --- a/src/libcamera/pipeline_handler.cpp > > > +++ b/src/libcamera/pipeline_handler.cpp > > > @@ -649,7 +649,7 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera) > > > */ > > > void PipelineHandler::hotplugMediaDevice(MediaDevice *media) > > > { > > > - media->disconnected.connect(this, = { mediaDeviceDisconnected(media); }); > > > + media->disconnected.connect(this, =, this { mediaDeviceDisconnected(media); }); > > > } > [...] Regards, Barnabás Pőcze
Hello, On Tue, Nov 14, 2023 at 04:05:16PM +0000, Barnabás Pőcze wrote: > 2023. november 14., kedd 14:42 keltezéssel, Laurent Pinchart via libcamera-devel írta: > > On Tue, Nov 14, 2023 at 12:53:04PM +0200, Laurent Pinchart via libcamera-devel wrote: > > > Hi Brett, > > > > > > Thank you for your first libcamera patch :-) > > > > > > On Mon, Nov 13, 2023 at 09:52:44AM -0700, Brett Brotherton via libcamera-devel wrote: > > > > > > > fix -Wdeprecated-this-capture error when building with c++20 by > > > > explicity naming this in the capture > > > > > > Nitpicking, s/fix/Fix/ and s/capture$/capture./ > > > > > > No need to resend the patch for this, I'll update the commit message > > > before pushing. > > > > > > Is this the only occurrence of implicit *this capture ? I'm happy we're > > > not doing that bad :-) > > > > > > > Signed-off-by: Brett Brotherton bbrotherton@google.com > > > > > > Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com > > > > > > I will run this through build testing and push the patch if no issues > > > are found. > > > > Unfortunately problems have been found :-( > > > > ../../src/libcamera/pipeline_handler.cpp:652:40: error: explicit capture of 'this' with a capture default of '=' is a C++20 extension [-Werror,-Wc++20-extensions] > > media->disconnected.connect(this, =, this { mediaDeviceDisconnected(media); }); > > > > ^ > > I imagine explicitly capturing everything needed should fix that. > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > index 9c74c6cf..29e0c98a 100644 > --- a/src/libcamera/pipeline_handler.cpp > +++ b/src/libcamera/pipeline_handler.cpp > @@ -649,7 +649,7 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera) > */ > void PipelineHandler::hotplugMediaDevice(MediaDevice *media) > { > - media->disconnected.connect(this, [=]() { mediaDeviceDisconnected(media); }); > + media->disconnected.connect(this, [this, media] { mediaDeviceDisconnected(media); }); > } > > /** This seems fine. Brett, could you please test this (both with C++20 and C++17) and send a new version of the patch ? > > > > --- > > > > src/libcamera/pipeline_handler.cpp | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > > > > index 9c74c6cf..2220cb92 100644 > > > > --- a/src/libcamera/pipeline_handler.cpp > > > > +++ b/src/libcamera/pipeline_handler.cpp > > > > @@ -649,7 +649,7 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera) > > > > */ > > > > void PipelineHandler::hotplugMediaDevice(MediaDevice *media) > > > > { > > > > - media->disconnected.connect(this, = { mediaDeviceDisconnected(media); }); > > > > + media->disconnected.connect(this, =, this { mediaDeviceDisconnected(media); }); > > > > } > > [...]
Yes, I will try this and send a new version of the patch once confirmed. On Sun, Nov 19, 2023 at 6:07 PM Laurent Pinchart < laurent.pinchart@ideasonboard.com> wrote: > Hello, > > On Tue, Nov 14, 2023 at 04:05:16PM +0000, Barnabás Pőcze wrote: > > 2023. november 14., kedd 14:42 keltezéssel, Laurent Pinchart via > libcamera-devel írta: > > > On Tue, Nov 14, 2023 at 12:53:04PM +0200, Laurent Pinchart via > libcamera-devel wrote: > > > > Hi Brett, > > > > > > > > Thank you for your first libcamera patch :-) > > > > > > > > On Mon, Nov 13, 2023 at 09:52:44AM -0700, Brett Brotherton via > libcamera-devel wrote: > > > > > > > > > fix -Wdeprecated-this-capture error when building with c++20 by > > > > > explicity naming this in the capture > > > > > > > > Nitpicking, s/fix/Fix/ and s/capture$/capture./ > > > > > > > > No need to resend the patch for this, I'll update the commit message > > > > before pushing. > > > > > > > > Is this the only occurrence of implicit *this capture ? I'm happy > we're > > > > not doing that bad :-) > > > > > > > > > Signed-off-by: Brett Brotherton bbrotherton@google.com > > > > > > > > Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com > > > > > > > > I will run this through build testing and push the patch if no issues > > > > are found. > > > > > > Unfortunately problems have been found :-( > > > > > > ../../src/libcamera/pipeline_handler.cpp:652:40: error: explicit > capture of 'this' with a capture default of '=' is a C++20 extension > [-Werror,-Wc++20-extensions] > > > media->disconnected.connect(this, =, this { > mediaDeviceDisconnected(media); }); > > > > > > ^ > > > > I imagine explicitly capturing everything needed should fix that. > > > > diff --git a/src/libcamera/pipeline_handler.cpp > b/src/libcamera/pipeline_handler.cpp > > index 9c74c6cf..29e0c98a 100644 > > --- a/src/libcamera/pipeline_handler.cpp > > +++ b/src/libcamera/pipeline_handler.cpp > > @@ -649,7 +649,7 @@ void > PipelineHandler::registerCamera(std::shared_ptr<Camera> camera) > > */ > > void PipelineHandler::hotplugMediaDevice(MediaDevice *media) > > { > > - media->disconnected.connect(this, [=]() { > mediaDeviceDisconnected(media); }); > > + media->disconnected.connect(this, [this, media] { > mediaDeviceDisconnected(media); }); > > } > > > > /** > > This seems fine. Brett, could you please test this (both with C++20 and > C++17) and send a new version of the patch ? > > > > > > --- > > > > > src/libcamera/pipeline_handler.cpp | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/src/libcamera/pipeline_handler.cpp > b/src/libcamera/pipeline_handler.cpp > > > > > index 9c74c6cf..2220cb92 100644 > > > > > --- a/src/libcamera/pipeline_handler.cpp > > > > > +++ b/src/libcamera/pipeline_handler.cpp > > > > > @@ -649,7 +649,7 @@ void > PipelineHandler::registerCamera(std::shared_ptr<Camera> camera) > > > > > */ > > > > > void PipelineHandler::hotplugMediaDevice(MediaDevice *media) > > > > > { > > > > > - media->disconnected.connect(this, = { > mediaDeviceDisconnected(media); }); > > > > > + media->disconnected.connect(this, =, this { > mediaDeviceDisconnected(media); }); > > > > > } > > > [...] > > -- > Regards, > > Laurent Pinchart >
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 9c74c6cf..2220cb92 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -649,7 +649,7 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera) */ void PipelineHandler::hotplugMediaDevice(MediaDevice *media) { - media->disconnected.connect(this, [=]() { mediaDeviceDisconnected(media); }); + media->disconnected.connect(this, [=, this]() { mediaDeviceDisconnected(media); }); } /**
fix -Wdeprecated-this-capture error when building with c++20 by explicity naming this in the capture Signed-off-by: Brett Brotherton <bbrotherton@google.com> --- src/libcamera/pipeline_handler.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)