| Message ID | 20210920173752.1346190-2-umang.jain@ideasonboard.com |
|---|---|
| State | RFC |
| Delegated to: | Umang Jain |
| Headers | show |
| Series |
|
| Related | show |
Hi Umang, Thank you for the patch. On Mon, Sep 20, 2021 at 11:07:43PM +0530, Umang Jain wrote: > Use Camera3RequestDescriptor as cookie for the Capture Request. > The cookie is used to lookup descriptors map in > CameraDevice::requestComplete(). The map will be transformed to a > queue in subsequent commit. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > src/android/camera_device.cpp | 3 ++- > src/android/camera_worker.cpp | 6 +++--- > src/android/camera_worker.h | 5 +++-- > 3 files changed, 8 insertions(+), 6 deletions(-) > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp > index ab31bdda..f461e14c 100644 > --- a/src/android/camera_device.cpp > +++ b/src/android/camera_device.cpp > @@ -244,7 +244,8 @@ CameraDevice::Camera3RequestDescriptor::Camera3RequestDescriptor( > * Create the CaptureRequest, stored as a unique_ptr<> to tie its > * lifetime to the descriptor. > */ > - request_ = std::make_unique<CaptureRequest>(camera); > + request_ = std::make_unique<CaptureRequest>(camera, > + reinterpret_cast<uint64_t>(this)); > } > > /* > diff --git a/src/android/camera_worker.cpp b/src/android/camera_worker.cpp > index 91313183..3e18de21 100644 > --- a/src/android/camera_worker.cpp > +++ b/src/android/camera_worker.cpp > @@ -27,10 +27,10 @@ LOG_DECLARE_CATEGORY(HAL) > * by the CameraWorker which queues it to the libcamera::Camera after handling > * fences. > */ > -CaptureRequest::CaptureRequest(Camera *camera) > - : camera_(camera) > +CaptureRequest::CaptureRequest(Camera *camera, uint64_t cookie) > + : camera_(camera), cookie_(cookie) > { > - request_ = camera_->createRequest(reinterpret_cast<uint64_t>(this)); > + request_ = camera_->createRequest(cookie_); > } > > void CaptureRequest::addBuffer(Stream *stream, FrameBuffer *buffer, int fence) > diff --git a/src/android/camera_worker.h b/src/android/camera_worker.h > index 67ae50bd..3432d1fd 100644 > --- a/src/android/camera_worker.h > +++ b/src/android/camera_worker.h > @@ -22,7 +22,7 @@ class CameraDevice; You need to include stdint.h. > public: > - CaptureRequest(libcamera::Camera *camera); > + CaptureRequest(libcamera::Camera *camera, uint64_t cookie); > > const std::vector<int> &fences() const { return acquireFences_; } > libcamera::ControlList &controls() { return request_->controls(); } > @@ -30,7 +30,7 @@ public: > { > return request_->metadata(); > } > - unsigned long cookie() const { return request_->cookie(); } > + unsigned long cookie() const { return cookie_; } > > void addBuffer(libcamera::Stream *stream, > libcamera::FrameBuffer *buffer, int fence); > @@ -40,6 +40,7 @@ private: > libcamera::Camera *camera_; > std::vector<int> acquireFences_; > std::unique_ptr<libcamera::Request> request_; > + uint64_t cookie_; Any reason for adding this member ? I think you can keep the current implementation of cookie() and drop cookie_, unless another patch in this series requires it. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > }; > > class CameraWorker : private libcamera::Thread
Hi Umang, thank you for the patch. On Tue, Sep 21, 2021 at 3:35 AM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Umang, > > Thank you for the patch. > > On Mon, Sep 20, 2021 at 11:07:43PM +0530, Umang Jain wrote: > > Use Camera3RequestDescriptor as cookie for the Capture Request. > > The cookie is used to lookup descriptors map in > > CameraDevice::requestComplete(). The map will be transformed to a > > queue in subsequent commit. > > > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > > --- > > src/android/camera_device.cpp | 3 ++- > > src/android/camera_worker.cpp | 6 +++--- > > src/android/camera_worker.h | 5 +++-- > > 3 files changed, 8 insertions(+), 6 deletions(-) > > > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp > > index ab31bdda..f461e14c 100644 > > --- a/src/android/camera_device.cpp > > +++ b/src/android/camera_device.cpp > > @@ -244,7 +244,8 @@ CameraDevice::Camera3RequestDescriptor::Camera3RequestDescriptor( > > * Create the CaptureRequest, stored as a unique_ptr<> to tie its > > * lifetime to the descriptor. > > */ > > - request_ = std::make_unique<CaptureRequest>(camera); > > + request_ = std::make_unique<CaptureRequest>(camera, > > + reinterpret_cast<uint64_t>(this)); > > } > > > > /* > > diff --git a/src/android/camera_worker.cpp b/src/android/camera_worker.cpp > > index 91313183..3e18de21 100644 > > --- a/src/android/camera_worker.cpp > > +++ b/src/android/camera_worker.cpp > > @@ -27,10 +27,10 @@ LOG_DECLARE_CATEGORY(HAL) > > * by the CameraWorker which queues it to the libcamera::Camera after handling > > * fences. > > */ > > -CaptureRequest::CaptureRequest(Camera *camera) > > - : camera_(camera) > > +CaptureRequest::CaptureRequest(Camera *camera, uint64_t cookie) > > + : camera_(camera), cookie_(cookie) > > { > > - request_ = camera_->createRequest(reinterpret_cast<uint64_t>(this)); > > + request_ = camera_->createRequest(cookie_); > > } > > > > void CaptureRequest::addBuffer(Stream *stream, FrameBuffer *buffer, int fence) > > diff --git a/src/android/camera_worker.h b/src/android/camera_worker.h > > index 67ae50bd..3432d1fd 100644 > > --- a/src/android/camera_worker.h > > +++ b/src/android/camera_worker.h > > @@ -22,7 +22,7 @@ class CameraDevice; > > You need to include stdint.h. > > > public: > > - CaptureRequest(libcamera::Camera *camera); > > + CaptureRequest(libcamera::Camera *camera, uint64_t cookie); > > > > const std::vector<int> &fences() const { return acquireFences_; } > > libcamera::ControlList &controls() { return request_->controls(); } > > @@ -30,7 +30,7 @@ public: > > { > > return request_->metadata(); > > } > > - unsigned long cookie() const { return request_->cookie(); } > > + unsigned long cookie() const { return cookie_; } I think we would rather change CaptureRequest::cookie() return type to uint64_t to follow Request::cookie() return type. But this problem exists before your patch. So I am fine to fix this after this patch series. > > > > void addBuffer(libcamera::Stream *stream, > > libcamera::FrameBuffer *buffer, int fence); > > @@ -40,6 +40,7 @@ private: > > libcamera::Camera *camera_; > > std::vector<int> acquireFences_; > > std::unique_ptr<libcamera::Request> request_; > > + uint64_t cookie_; > > Any reason for adding this member ? I think you can keep the current > implementation of cookie() and drop cookie_, unless another patch in > this series requires it. > +1. request_->cookie() must be cookie_. So having cookie_ is not needed. Reviewed-by: Hirokazu Honda <hiroh@chromium.org> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > }; > > > > class CameraWorker : private libcamera::Thread > > -- > Regards, > > Laurent Pinchart
diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index ab31bdda..f461e14c 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -244,7 +244,8 @@ CameraDevice::Camera3RequestDescriptor::Camera3RequestDescriptor( * Create the CaptureRequest, stored as a unique_ptr<> to tie its * lifetime to the descriptor. */ - request_ = std::make_unique<CaptureRequest>(camera); + request_ = std::make_unique<CaptureRequest>(camera, + reinterpret_cast<uint64_t>(this)); } /* diff --git a/src/android/camera_worker.cpp b/src/android/camera_worker.cpp index 91313183..3e18de21 100644 --- a/src/android/camera_worker.cpp +++ b/src/android/camera_worker.cpp @@ -27,10 +27,10 @@ LOG_DECLARE_CATEGORY(HAL) * by the CameraWorker which queues it to the libcamera::Camera after handling * fences. */ -CaptureRequest::CaptureRequest(Camera *camera) - : camera_(camera) +CaptureRequest::CaptureRequest(Camera *camera, uint64_t cookie) + : camera_(camera), cookie_(cookie) { - request_ = camera_->createRequest(reinterpret_cast<uint64_t>(this)); + request_ = camera_->createRequest(cookie_); } void CaptureRequest::addBuffer(Stream *stream, FrameBuffer *buffer, int fence) diff --git a/src/android/camera_worker.h b/src/android/camera_worker.h index 67ae50bd..3432d1fd 100644 --- a/src/android/camera_worker.h +++ b/src/android/camera_worker.h @@ -22,7 +22,7 @@ class CameraDevice; class CaptureRequest { public: - CaptureRequest(libcamera::Camera *camera); + CaptureRequest(libcamera::Camera *camera, uint64_t cookie); const std::vector<int> &fences() const { return acquireFences_; } libcamera::ControlList &controls() { return request_->controls(); } @@ -30,7 +30,7 @@ public: { return request_->metadata(); } - unsigned long cookie() const { return request_->cookie(); } + unsigned long cookie() const { return cookie_; } void addBuffer(libcamera::Stream *stream, libcamera::FrameBuffer *buffer, int fence); @@ -40,6 +40,7 @@ private: libcamera::Camera *camera_; std::vector<int> acquireFences_; std::unique_ptr<libcamera::Request> request_; + uint64_t cookie_; }; class CameraWorker : private libcamera::Thread
Use Camera3RequestDescriptor as cookie for the Capture Request. The cookie is used to lookup descriptors map in CameraDevice::requestComplete(). The map will be transformed to a queue in subsequent commit. Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> --- src/android/camera_device.cpp | 3 ++- src/android/camera_worker.cpp | 6 +++--- src/android/camera_worker.h | 5 +++-- 3 files changed, 8 insertions(+), 6 deletions(-)