[libcamera-devel,v4,2/5] HACK: libcamera: Request: expose Camera from Request
diff mbox series

Message ID 20220204133814.303217-3-tomi.valkeinen@ideasonboard.com
State Superseded
Headers show
Series
  • Python bindings
Related show

Commit Message

Tomi Valkeinen Feb. 4, 2022, 1:38 p.m. UTC
Request has Camera as a private member. Expose this so that users can
more easily associate a received Request to a Camera.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 include/libcamera/request.h | 2 ++
 src/libcamera/request.cpp   | 5 +++++
 2 files changed, 7 insertions(+)

Comments

Kieran Bingham Feb. 8, 2022, 2:29 p.m. UTC | #1
Quoting Tomi Valkeinen (2022-02-04 13:38:11)
> Request has Camera as a private member. Expose this so that users can
> more easily associate a received Request to a Camera.

I still don't think this is a hack. I've wanted this in the past [0].

  [0] https://patchwork.libcamera.org/patch/9328/


I'm still fighting myself if shared ptr is appropriate here, as I don't
think we want to convey that the camera here could be stored or kept -
it should only be used as a convenience to check the camera while
dealing with the request - but I don't think that's really going to be
an issue ... so

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

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  include/libcamera/request.h | 2 ++
>  src/libcamera/request.cpp   | 5 +++++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> index 1eb537e9..29da397c 100644
> --- a/include/libcamera/request.h
> +++ b/include/libcamera/request.h
> @@ -64,6 +64,8 @@ public:
>  
>         std::string toString() const;
>  
> +       std::shared_ptr<Camera> camera() const;
> +
>  private:
>         LIBCAMERA_DISABLE_COPY(Request)
>  
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index 9c8da6ca..de3c1f80 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -513,6 +513,11 @@ FrameBuffer *Request::findBuffer(const Stream *stream) const
>         return it->second;
>  }
>  
> +std::shared_ptr<Camera> Request::camera() const
> +{
> +       return _d()->camera()->shared_from_this();
> +}
> +
>  /**
>   * \fn Request::metadata()
>   * \brief Retrieve the request's metadata
> -- 
> 2.25.1
>
Tomi Valkeinen Feb. 9, 2022, 7:49 a.m. UTC | #2
On 08/02/2022 16:29, Kieran Bingham wrote:
> Quoting Tomi Valkeinen (2022-02-04 13:38:11)
>> Request has Camera as a private member. Expose this so that users can
>> more easily associate a received Request to a Camera.
> 
> I still don't think this is a hack. I've wanted this in the past [0].
> 
>    [0] https://patchwork.libcamera.org/patch/9328/

And Laurent wants to get rid of the Camera field =)

> I'm still fighting myself if shared ptr is appropriate here, as I don't
> think we want to convey that the camera here could be stored or kept -
> it should only be used as a convenience to check the camera while
> dealing with the request - but I don't think that's really going to be
> an issue ... so

Plain pointers are a bit tricky wrt. Python. If the pointer means that 
the ownership is given to the caller, it's fine, Python then owns the 
object. But if the pointer means that the caller does not own the 
object, and can only use the pointer temporarily, it's a problem as we 
can't guarantee such (use only temporarily) thing on the Python side.

Cameras are handled with shared_ptrs in libcamera, so I don't quite see 
why you wouldn't use shared_ptr<Camera> everywhere.

  Tomi

Patch
diff mbox series

diff --git a/include/libcamera/request.h b/include/libcamera/request.h
index 1eb537e9..29da397c 100644
--- a/include/libcamera/request.h
+++ b/include/libcamera/request.h
@@ -64,6 +64,8 @@  public:
 
 	std::string toString() const;
 
+	std::shared_ptr<Camera> camera() const;
+
 private:
 	LIBCAMERA_DISABLE_COPY(Request)
 
diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
index 9c8da6ca..de3c1f80 100644
--- a/src/libcamera/request.cpp
+++ b/src/libcamera/request.cpp
@@ -513,6 +513,11 @@  FrameBuffer *Request::findBuffer(const Stream *stream) const
 	return it->second;
 }
 
+std::shared_ptr<Camera> Request::camera() const
+{
+	return _d()->camera()->shared_from_this();
+}
+
 /**
  * \fn Request::metadata()
  * \brief Retrieve the request's metadata