[libcamera-devel,v5,18/33] ipa: Disable copy-construction of IPAContext
diff mbox series

Message ID 20220927023642.12341-19-laurent.pinchart@ideasonboard.com
State New
Headers show
Series
  • ipa: Frame context queue, IPU3 & RkISP consolidation, and RkISP1 improvements
Related show

Commit Message

Laurent Pinchart Sept. 27, 2022, 2:36 a.m. UTC 
The IPAContext is not meant to be copied. Prevent unintentional
pass-by-value by disabling its copy constructor.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/ipa/ipu3/ipa_context.h   | 4 ++++
 src/ipa/rkisp1/ipa_context.h | 4 ++++
 2 files changed, 8 insertions(+)

Comments

Kieran Bingham Sept. 27, 2022, 8:15 a.m. UTC | #1 
Quoting Laurent Pinchart via libcamera-devel (2022-09-27 03:36:27)
> The IPAContext is not meant to be copied. Prevent unintentional
> pass-by-value by disabling its copy constructor.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

I wonder if IPAContext should derive from an (empty) base struct which
has copy disabled... (would that actually work?)

But I doubt that's much better. This is fine I think.

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

> ---
>  src/ipa/ipu3/ipa_context.h   | 4 ++++
>  src/ipa/rkisp1/ipa_context.h | 4 ++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> index bdba3d07d13a..9766330f2f39 100644
> --- a/src/ipa/ipu3/ipa_context.h
> +++ b/src/ipa/ipu3/ipa_context.h
> @@ -10,6 +10,7 @@
>  
>  #include <linux/intel-ipu3.h>
>  
> +#include <libcamera/base/class.h>
>  #include <libcamera/base/utils.h>
>  
>  #include <libcamera/geometry.h>
> @@ -87,6 +88,9 @@ struct IPAContext {
>         IPAActiveState activeState;
>  
>         FCQueue<IPAFrameContext, kMaxFrameContexts> frameContexts;
> +
> +private:
> +       LIBCAMERA_DISABLE_COPY(IPAContext)
>  };
>  
>  } /* namespace ipa::ipu3 */
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index 4481bd2acde6..9a84b7adda45 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -10,6 +10,7 @@
>  
>  #include <linux/rkisp1-config.h>
>  
> +#include <libcamera/base/class.h>
>  #include <libcamera/base/utils.h>
>  
>  #include <libcamera/geometry.h>
> @@ -101,6 +102,9 @@ struct IPAContext {
>         IPAActiveState activeState;
>  
>         FCQueue<IPAFrameContext, kMaxFrameContexts> frameContexts;
> +
> +private:
> +       LIBCAMERA_DISABLE_COPY(IPAContext)
>  };
>  
>  } /* namespace ipa::rkisp1 */
> -- 
> Regards,
> 
> Laurent Pinchart
>
Jacopo Mondi Sept. 27, 2022, 8:19 a.m. UTC | #2 
Hi Laurent

On Tue, Sep 27, 2022 at 09:15:15AM +0100, Kieran Bingham via libcamera-devel wrote:
> Quoting Laurent Pinchart via libcamera-devel (2022-09-27 03:36:27)
> > The IPAContext is not meant to be copied. Prevent unintentional
> > pass-by-value by disabling its copy constructor.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> I wonder if IPAContext should derive from an (empty) base struct which
> has copy disabled... (would that actually work?)
>
> But I doubt that's much better. This is fine I think.
>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

>
> > ---
> >  src/ipa/ipu3/ipa_context.h   | 4 ++++
> >  src/ipa/rkisp1/ipa_context.h | 4 ++++
> >  2 files changed, 8 insertions(+)
> >
> > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> > index bdba3d07d13a..9766330f2f39 100644
> > --- a/src/ipa/ipu3/ipa_context.h
> > +++ b/src/ipa/ipu3/ipa_context.h
> > @@ -10,6 +10,7 @@
> >
> >  #include <linux/intel-ipu3.h>
> >
> > +#include <libcamera/base/class.h>
> >  #include <libcamera/base/utils.h>
> >
> >  #include <libcamera/geometry.h>
> > @@ -87,6 +88,9 @@ struct IPAContext {
> >         IPAActiveState activeState;
> >
> >         FCQueue<IPAFrameContext, kMaxFrameContexts> frameContexts;
> > +
> > +private:
> > +       LIBCAMERA_DISABLE_COPY(IPAContext)
> >  };
> >
> >  } /* namespace ipa::ipu3 */
> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> > index 4481bd2acde6..9a84b7adda45 100644
> > --- a/src/ipa/rkisp1/ipa_context.h
> > +++ b/src/ipa/rkisp1/ipa_context.h
> > @@ -10,6 +10,7 @@
> >
> >  #include <linux/rkisp1-config.h>
> >
> > +#include <libcamera/base/class.h>
> >  #include <libcamera/base/utils.h>
> >
> >  #include <libcamera/geometry.h>
> > @@ -101,6 +102,9 @@ struct IPAContext {
> >         IPAActiveState activeState;
> >
> >         FCQueue<IPAFrameContext, kMaxFrameContexts> frameContexts;
> > +
> > +private:
> > +       LIBCAMERA_DISABLE_COPY(IPAContext)
> >  };
> >
> >  } /* namespace ipa::rkisp1 */
> > --
> > Regards,
> >
> > Laurent Pinchart
> >
Laurent Pinchart Sept. 27, 2022, 10:10 a.m. UTC | #3 
On Tue, Sep 27, 2022 at 09:15:15AM +0100, Kieran Bingham wrote:
> Quoting Laurent Pinchart via libcamera-devel (2022-09-27 03:36:27)
> > The IPAContext is not meant to be copied. Prevent unintentional
> > pass-by-value by disabling its copy constructor.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> I wonder if IPAContext should derive from an (empty) base struct which
> has copy disabled... (would that actually work?)

That would work. We may also want to disable copying of the active
state. I'll have a look on top of this.

> But I doubt that's much better. This is fine I think.
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> > ---
> >  src/ipa/ipu3/ipa_context.h   | 4 ++++
> >  src/ipa/rkisp1/ipa_context.h | 4 ++++
> >  2 files changed, 8 insertions(+)
> > 
> > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> > index bdba3d07d13a..9766330f2f39 100644
> > --- a/src/ipa/ipu3/ipa_context.h
> > +++ b/src/ipa/ipu3/ipa_context.h
> > @@ -10,6 +10,7 @@
> >  
> >  #include <linux/intel-ipu3.h>
> >  
> > +#include <libcamera/base/class.h>
> >  #include <libcamera/base/utils.h>
> >  
> >  #include <libcamera/geometry.h>
> > @@ -87,6 +88,9 @@ struct IPAContext {
> >         IPAActiveState activeState;
> >  
> >         FCQueue<IPAFrameContext, kMaxFrameContexts> frameContexts;
> > +
> > +private:
> > +       LIBCAMERA_DISABLE_COPY(IPAContext)
> >  };
> >  
> >  } /* namespace ipa::ipu3 */
> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> > index 4481bd2acde6..9a84b7adda45 100644
> > --- a/src/ipa/rkisp1/ipa_context.h
> > +++ b/src/ipa/rkisp1/ipa_context.h
> > @@ -10,6 +10,7 @@
> >  
> >  #include <linux/rkisp1-config.h>
> >  
> > +#include <libcamera/base/class.h>
> >  #include <libcamera/base/utils.h>
> >  
> >  #include <libcamera/geometry.h>
> > @@ -101,6 +102,9 @@ struct IPAContext {
> >         IPAActiveState activeState;
> >  
> >         FCQueue<IPAFrameContext, kMaxFrameContexts> frameContexts;
> > +
> > +private:
> > +       LIBCAMERA_DISABLE_COPY(IPAContext)
> >  };
> >  
> >  } /* namespace ipa::rkisp1 */

Patch
diff mbox series 

diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
index bdba3d07d13a..9766330f2f39 100644
--- a/src/ipa/ipu3/ipa_context.h
+++ b/src/ipa/ipu3/ipa_context.h
@@ -10,6 +10,7 @@ 
 
 #include <linux/intel-ipu3.h>
 
+#include <libcamera/base/class.h>
 #include <libcamera/base/utils.h>
 
 #include <libcamera/geometry.h>
@@ -87,6 +88,9 @@  struct IPAContext {
 	IPAActiveState activeState;
 
 	FCQueue<IPAFrameContext, kMaxFrameContexts> frameContexts;
+
+private:
+	LIBCAMERA_DISABLE_COPY(IPAContext)
 };
 
 } /* namespace ipa::ipu3 */
diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
index 4481bd2acde6..9a84b7adda45 100644
--- a/src/ipa/rkisp1/ipa_context.h
+++ b/src/ipa/rkisp1/ipa_context.h
@@ -10,6 +10,7 @@ 
 
 #include <linux/rkisp1-config.h>
 
+#include <libcamera/base/class.h>
 #include <libcamera/base/utils.h>
 
 #include <libcamera/geometry.h>
@@ -101,6 +102,9 @@  struct IPAContext {
 	IPAActiveState activeState;
 
 	FCQueue<IPAFrameContext, kMaxFrameContexts> frameContexts;
+
+private:
+	LIBCAMERA_DISABLE_COPY(IPAContext)
 };
 
 } /* namespace ipa::rkisp1 */