[v3,1/2] libcamera: pipeline_handler: Move useCount_ to protected
diff mbox series

Message ID 20251028160453.3802642-2-antoine.bouyer@nxp.com
State Changes Requested
Headers show
Series
  • imx8-isi: Move isi routing into acquireDevice
Related show

Commit Message

Antoine Bouyer Oct. 28, 2025, 4:04 p.m. UTC
Move the useCount_ parameter to protected instead of private, so that
PipelineHandler child classes could access it to verify whether the
media device is already locked or not.

Signed-off-by: Antoine Bouyer <antoine.bouyer@nxp.com>
---
 include/libcamera/internal/pipeline_handler.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Kieran Bingham Oct. 28, 2025, 4:29 p.m. UTC | #1
Quoting Antoine Bouyer (2025-10-28 16:04:52)
> Move the useCount_ parameter to protected instead of private, so that
> PipelineHandler child classes could access it to verify whether the
> media device is already locked or not.
> 
> Signed-off-by: Antoine Bouyer <antoine.bouyer@nxp.com>
> ---
>  include/libcamera/internal/pipeline_handler.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> index e89d6a33e398..4cefa3c7aba2 100644
> --- a/include/libcamera/internal/pipeline_handler.h
> +++ b/include/libcamera/internal/pipeline_handler.h
> @@ -82,6 +82,7 @@ protected:
>  
>         CameraManager *manager_;
>         const unsigned int maxQueuedRequestsDevice_;
> +       unsigned int useCount_;

Part of me wonders if we should have an accessor instead but I don't
think that's completley required. Though I can't imagine why a derived
pipeline handler should ever modify this value ...

So for now:

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

>  
>  private:
>         void unlockMediaDevices();
> @@ -96,7 +97,6 @@ private:
>         std::vector<std::weak_ptr<Camera>> cameras_;
>  
>         const char *name_;
> -       unsigned int useCount_;
>  
>         friend class PipelineHandlerFactoryBase;
>  };
> -- 
> 2.34.1
>
Kieran Bingham Oct. 30, 2025, 2:41 p.m. UTC | #2
Quoting Kieran Bingham (2025-10-28 16:29:26)
> Quoting Antoine Bouyer (2025-10-28 16:04:52)
> > Move the useCount_ parameter to protected instead of private, so that
> > PipelineHandler child classes could access it to verify whether the
> > media device is already locked or not.
> > 
> > Signed-off-by: Antoine Bouyer <antoine.bouyer@nxp.com>
> > ---
> >  include/libcamera/internal/pipeline_handler.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> > index e89d6a33e398..4cefa3c7aba2 100644
> > --- a/include/libcamera/internal/pipeline_handler.h
> > +++ b/include/libcamera/internal/pipeline_handler.h
> > @@ -82,6 +82,7 @@ protected:
> >  
> >         CameraManager *manager_;
> >         const unsigned int maxQueuedRequestsDevice_;
> > +       unsigned int useCount_;
> 
> Part of me wonders if we should have an accessor instead but I don't
> think that's completley required. Though I can't imagine why a derived
> pipeline handler should ever modify this value ...
> 
> So for now:
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

By moving this from private to protected the following error is produced
so this doesn't get through CI:

https://gitlab.freedesktop.org/camera/libcamera/-/jobs/86929614#L845

FAILED: Documentation/internal-api
/usr/bin/doxygen Documentation/Doxyfile-internal
/builds/camera/libcamera/include/libcamera/internal/pipeline_handler.h:85: error: Member useCount_ (variable) of class libcamera::PipelineHandler is not documented. (warning treated as error, aborting now)

If you fix that up - I think making it just an accessor protects the
useCount_ ...

--
Kieran


> 
> >  
> >  private:
> >         void unlockMediaDevices();
> > @@ -96,7 +97,6 @@ private:
> >         std::vector<std::weak_ptr<Camera>> cameras_;
> >  
> >         const char *name_;
> > -       unsigned int useCount_;
> >  
> >         friend class PipelineHandlerFactoryBase;
> >  };
> > -- 
> > 2.34.1
> >

Patch
diff mbox series

diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
index e89d6a33e398..4cefa3c7aba2 100644
--- a/include/libcamera/internal/pipeline_handler.h
+++ b/include/libcamera/internal/pipeline_handler.h
@@ -82,6 +82,7 @@  protected:
 
 	CameraManager *manager_;
 	const unsigned int maxQueuedRequestsDevice_;
+	unsigned int useCount_;
 
 private:
 	void unlockMediaDevices();
@@ -96,7 +97,6 @@  private:
 	std::vector<std::weak_ptr<Camera>> cameras_;
 
 	const char *name_;
-	unsigned int useCount_;
 
 	friend class PipelineHandlerFactoryBase;
 };