[v5,1/2] libcamera: pipeline_handler: Add accessor for useCount_
diff mbox series

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

Commit Message

Antoine Bouyer Nov. 4, 2025, 7:56 a.m. UTC 
Add an accessor for useCount_ parameter, so that PipelineHandler
child classes can access it to verify whether the media device
is already locked or not.

Signed-off-by: Antoine Bouyer <antoine.bouyer@nxp.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 include/libcamera/internal/pipeline_handler.h |  1 +
 src/libcamera/pipeline_handler.cpp            | 14 ++++++++++++++
 2 files changed, 15 insertions(+)

Comments

Barnabás Pőcze Nov. 4, 2025, 11:31 a.m. UTC | #1 
Hi

2025. 11. 04. 8:56 keltezéssel, Antoine Bouyer írta:
> Add an accessor for useCount_ parameter, so that PipelineHandler
> child classes can access it to verify whether the media device
> is already locked or not.
> 
> Signed-off-by: Antoine Bouyer <antoine.bouyer@nxp.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>   include/libcamera/internal/pipeline_handler.h |  1 +
>   src/libcamera/pipeline_handler.cpp            | 14 ++++++++++++++
>   2 files changed, 15 insertions(+)
> 
> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> index e89d6a33e398..2ca210d0ae4f 100644
> --- a/include/libcamera/internal/pipeline_handler.h
> +++ b/include/libcamera/internal/pipeline_handler.h
> @@ -73,6 +73,7 @@ public:
>   protected:
>   	void registerCamera(std::shared_ptr<Camera> camera);
>   	void hotplugMediaDevice(MediaDevice *media);
> +	unsigned int useCount() const { return useCount_; };
>   
>   	virtual int queueRequestDevice(Camera *camera, Request *request) = 0;
>   	virtual void stopDevice(Camera *camera) = 0;
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index e5f9e55c9783..0279c21b691b 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -168,6 +168,10 @@ MediaDevice *PipelineHandler::acquireMediaDevice(DeviceEnumerator *enumerator,
>    */
>   bool PipelineHandler::acquire(Camera *camera)
>   {
> +	LOG(Pipeline, Debug)
> +		<< "Acquire camera " << camera->id()
> +		<< " useCount " << useCount_;
> +
>   	if (useCount_ == 0) {
>   		for (std::shared_ptr<MediaDevice> &media : mediaDevices_) {
>   			if (!media->lock()) {
> @@ -214,6 +218,10 @@ void PipelineHandler::release(Camera *camera)
>   		unlockMediaDevices();
>   
>   	--useCount_;
> +
> +	LOG(Pipeline, Debug)
> +		<< "release camera " << camera->id()

The casing does not match the previous log message, but this can be fixed when applying the change.
And maybe the log message additions should be separate.

But otherwise looks good to me.

Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>


> +		<< " useCount " << useCount_;
>   }
>   
>   /**
> @@ -811,6 +819,12 @@ void PipelineHandler::disconnect()
>    * \return The pipeline handler name
>    */
>   
> +/**
> + * \fn PipelineHandler::useCount()
> + * \brief Retrieve the pipeline handler's used camera count
> + * \return The number of acquired cameras of the pipeline handler
> + */
> +
>   /**
>    * \fn PipelineHandler::cameraManager() const
>    * \brief Retrieve the CameraManager that this pipeline handler belongs to
Antoine Bouyer Nov. 4, 2025, 1:39 p.m. UTC | #2 
Hi Barnabás

Thanks for your reviews.

On 11/4/25 12:31 PM, Barnabás Pőcze wrote:
> Caution: This is an external email. Please take care when clicking links 
> or opening attachments. When in doubt, report the message using the 
> 'Report this email' button
> 
> 
> Hi
> 
> 2025. 11. 04. 8:56 keltezéssel, Antoine Bouyer írta:
>> Add an accessor for useCount_ parameter, so that PipelineHandler
>> child classes can access it to verify whether the media device
>> is already locked or not.
>>
>> Signed-off-by: Antoine Bouyer <antoine.bouyer@nxp.com>
>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>> ---
>>   include/libcamera/internal/pipeline_handler.h |  1 +
>>   src/libcamera/pipeline_handler.cpp            | 14 ++++++++++++++
>>   2 files changed, 15 insertions(+)
>>
>> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/ 
>> libcamera/internal/pipeline_handler.h
>> index e89d6a33e398..2ca210d0ae4f 100644
>> --- a/include/libcamera/internal/pipeline_handler.h
>> +++ b/include/libcamera/internal/pipeline_handler.h
>> @@ -73,6 +73,7 @@ public:
>>   protected:
>>       void registerCamera(std::shared_ptr<Camera> camera);
>>       void hotplugMediaDevice(MediaDevice *media);
>> +     unsigned int useCount() const { return useCount_; };
>>
>>       virtual int queueRequestDevice(Camera *camera, Request *request) 
>> = 0;
>>       virtual void stopDevice(Camera *camera) = 0;
>> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/ 
>> pipeline_handler.cpp
>> index e5f9e55c9783..0279c21b691b 100644
>> --- a/src/libcamera/pipeline_handler.cpp
>> +++ b/src/libcamera/pipeline_handler.cpp
>> @@ -168,6 +168,10 @@ MediaDevice 
>> *PipelineHandler::acquireMediaDevice(DeviceEnumerator *enumerator,
>>    */
>>   bool PipelineHandler::acquire(Camera *camera)
>>   {
>> +     LOG(Pipeline, Debug)
>> +             << "Acquire camera " << camera->id()
>> +             << " useCount " << useCount_;
>> +
>>       if (useCount_ == 0) {
>>               for (std::shared_ptr<MediaDevice> &media : mediaDevices_) {
>>                       if (!media->lock()) {
>> @@ -214,6 +218,10 @@ void PipelineHandler::release(Camera *camera)
>>               unlockMediaDevices();
>>
>>       --useCount_;
>> +
>> +     LOG(Pipeline, Debug)
>> +             << "release camera " << camera->id()
> 
> The casing does not match the previous log message, but this can be 
> fixed when applying the change.
> And maybe the log message additions should be separate.

Indeed, preferable to log _after_ usecCount_ is increased, and _before_ 
it is decreased, or simply remove the logs. I'm not particularly 
motivated to keep them. Just moved them here since they were in ISI 
pipeline handler originally in v1.

Let me push a v6 without any logs.

> 
> But otherwise looks good to me.
> 
> Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

Thanks
Antoine

> 
> 
>> +             << " useCount " << useCount_;
>>   }
>>
>>   /**
>> @@ -811,6 +819,12 @@ void PipelineHandler::disconnect()
>>    * \return The pipeline handler name
>>    */
>>
>> +/**
>> + * \fn PipelineHandler::useCount()
>> + * \brief Retrieve the pipeline handler's used camera count
>> + * \return The number of acquired cameras of the pipeline handler
>> + */
>> +
>>   /**
>>    * \fn PipelineHandler::cameraManager() const
>>    * \brief Retrieve the CameraManager that this pipeline handler 
>> belongs to
>

Patch
diff mbox series 

diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
index e89d6a33e398..2ca210d0ae4f 100644
--- a/include/libcamera/internal/pipeline_handler.h
+++ b/include/libcamera/internal/pipeline_handler.h
@@ -73,6 +73,7 @@  public:
 protected:
 	void registerCamera(std::shared_ptr<Camera> camera);
 	void hotplugMediaDevice(MediaDevice *media);
+	unsigned int useCount() const { return useCount_; };
 
 	virtual int queueRequestDevice(Camera *camera, Request *request) = 0;
 	virtual void stopDevice(Camera *camera) = 0;
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
index e5f9e55c9783..0279c21b691b 100644
--- a/src/libcamera/pipeline_handler.cpp
+++ b/src/libcamera/pipeline_handler.cpp
@@ -168,6 +168,10 @@  MediaDevice *PipelineHandler::acquireMediaDevice(DeviceEnumerator *enumerator,
  */
 bool PipelineHandler::acquire(Camera *camera)
 {
+	LOG(Pipeline, Debug)
+		<< "Acquire camera " << camera->id()
+		<< " useCount " << useCount_;
+
 	if (useCount_ == 0) {
 		for (std::shared_ptr<MediaDevice> &media : mediaDevices_) {
 			if (!media->lock()) {
@@ -214,6 +218,10 @@  void PipelineHandler::release(Camera *camera)
 		unlockMediaDevices();
 
 	--useCount_;
+
+	LOG(Pipeline, Debug)
+		<< "release camera " << camera->id()
+		<< " useCount " << useCount_;
 }
 
 /**
@@ -811,6 +819,12 @@  void PipelineHandler::disconnect()
  * \return The pipeline handler name
  */
 
+/**
+ * \fn PipelineHandler::useCount()
+ * \brief Retrieve the pipeline handler's used camera count
+ * \return The number of acquired cameras of the pipeline handler
+ */
+
 /**
  * \fn PipelineHandler::cameraManager() const
  * \brief Retrieve the CameraManager that this pipeline handler belongs to