[libcamera-devel] [RFC v2 2/4] HACK: expose Camera* from Request

[Kieran Bingham]

Hi Tomi,

On 27/11/2020 13:37, Tomi Valkeinen wrote:
> Py bindings need to find out which camera is the source of the completed
> Request. Request already has a private field for the Camera, so we can
> just expose it via a getter.

Interestingly, some time ago I posted a similar (but simpler?) patch for
this.

[PATCH] libcamera: request: Facilitate retrieval of the camera

https://lists.libcamera.org/pipermail/libcamera-devel/2020-August/012064.html

Which was partially rejected by Niklas, however he did give me an RB tag.

I sort of thought this was beneficial as it helps get directly to the
'correct' Camera in any callbacks.

Interesting that you've used a shared_ptr rather than where I directly
pass the pointer, and I think that's likely more correct as the expected
use might be to take the event in a callback, and immediately pass it on
to another thread - so a shared pointer makes a bit more sense then to
keep things safe.



> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ti.com>
> ---
>  include/libcamera/request.h | 1 +
>  src/libcamera/request.cpp   | 5 +++++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> index 655b1324..f98ef767 100644
> --- a/include/libcamera/request.h
> +++ b/include/libcamera/request.h
> @@ -56,6 +56,7 @@ public:
>  
>  	bool hasPendingBuffers() const { return !pending_.empty(); }
>  
> +        std::shared_ptr<Camera> camera() const;
>  private:
>  	friend class PipelineHandler;
>  
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index a68684ef..5a50ec6b 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -216,6 +216,11 @@ FrameBuffer *Request::findBuffer(const Stream *stream) const
>  	return it->second;
>  }
>  


Perhaps just add this ;-) (stolen/adapted from my patch)

+/**
+ * \fn Request::camera()
+ * \brief Retrieve the camera that owns the request
+ *
+ * \return A shared pointer to the camera associated with the request
+ */

With a version with that, I'd give this a:

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

But given previous push-back, I'll hope for further acceptance from the
others before we consider dropping the 'HACK:' prefix ;-)

--
Kieran


> +std::shared_ptr<Camera> Request::camera() const
> +{
> +	return camera_->shared_from_this();
> +}
> +
>  /**
>   * \fn Request::metadata()
>   * \brief Retrieve the request's metadata
> 

-- 
Regards
--
Kieran