Message ID | 20211027141623.422927-1-umang.jain@ideasonboard.com |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
Hi Umang, Thank you for the patch. On Wed, Oct 27, 2021 at 07:46:23PM +0530, Umang Jain wrote: > Provide a constructor for StreamBuffer and use that while populating > Camera3RequestDescriptor::buffers_ vector. Also provide the default > move-constructor (required as StreamBuffer is stored in a vector in > Camera3RequestDescriptor) and destructor for the StreamBuffer struct. > > Also declare a default move assignment operator and disable the > copy constructor and move operator explicitly with > LIBCAMERA_DISABLE_COPY(). > > While at it, initialize pointers members in the StreamBuffer struct > to nullptr, with StreamBuffer::status set to Status::Success. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > Changes in v2: > - Declare default move-assignment operator which is then explicitly > disabled by LIBCAMERA_DISABLE_MOVE() macro along with copy > constructor (enforced by the macro). > --- > src/android/camera_request.cpp | 16 +++++++++++++--- > src/android/camera_request.h | 16 +++++++++++++--- > 2 files changed, 26 insertions(+), 6 deletions(-) > > diff --git a/src/android/camera_request.cpp b/src/android/camera_request.cpp > index 5bac1b8f..0b2946fb 100644 > --- a/src/android/camera_request.cpp > +++ b/src/android/camera_request.cpp > @@ -37,9 +37,7 @@ Camera3RequestDescriptor::Camera3RequestDescriptor( > CameraStream *stream = > static_cast<CameraStream *>(buffer.stream->priv); > > - buffers_.push_back({ stream, buffer.buffer, nullptr, > - buffer.acquire_fence, Status::Success, > - nullptr, nullptr, nullptr, this }); > + buffers_.emplace_back(stream, buffer, this); > } > > /* Clone the controls associated with the camera3 request. */ > @@ -54,3 +52,15 @@ Camera3RequestDescriptor::Camera3RequestDescriptor( > } > > Camera3RequestDescriptor::~Camera3RequestDescriptor() = default; > + > +Camera3RequestDescriptor::StreamBuffer::StreamBuffer( > + CameraStream *stream, const camera3_stream_buffer_t &buffer, > + Camera3RequestDescriptor *request) > + : camera3Buffer(buffer.buffer), fence(buffer.acquire_fence), > + stream(stream), request(request) > +{ > +} > + > +Camera3RequestDescriptor::StreamBuffer::~StreamBuffer() = default; > + > +Camera3RequestDescriptor::StreamBuffer::StreamBuffer(StreamBuffer &&other) = default; > diff --git a/src/android/camera_request.h b/src/android/camera_request.h > index cfafa445..618e1fc1 100644 > --- a/src/android/camera_request.h > +++ b/src/android/camera_request.h > @@ -34,15 +34,25 @@ public: > }; > > struct StreamBuffer { > + StreamBuffer(CameraStream *stream, > + const camera3_stream_buffer_t &buffer, > + Camera3RequestDescriptor *request); > + ~StreamBuffer(); I'd add a blank line here. > + StreamBuffer(StreamBuffer &&other); > + StreamBuffer &operator=(StreamBuffer &&) = default; The = default should go to the .cpp file, the function doesn't need to be inlined. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + > CameraStream *stream; > buffer_handle_t *camera3Buffer; > std::unique_ptr<libcamera::FrameBuffer> frameBuffer; > int fence; > - Status status; > - libcamera::FrameBuffer *internalBuffer; > - const libcamera::FrameBuffer *srcBuffer; > + Status status = Status::Success; > + libcamera::FrameBuffer *internalBuffer = nullptr; > + const libcamera::FrameBuffer *srcBuffer = nullptr; > std::unique_ptr<CameraBuffer> dstBuffer; > Camera3RequestDescriptor *request; > + > + private: > + LIBCAMERA_DISABLE_COPY(StreamBuffer) > }; > > /* Keeps track of streams requiring post-processing. */
diff --git a/src/android/camera_request.cpp b/src/android/camera_request.cpp index 5bac1b8f..0b2946fb 100644 --- a/src/android/camera_request.cpp +++ b/src/android/camera_request.cpp @@ -37,9 +37,7 @@ Camera3RequestDescriptor::Camera3RequestDescriptor( CameraStream *stream = static_cast<CameraStream *>(buffer.stream->priv); - buffers_.push_back({ stream, buffer.buffer, nullptr, - buffer.acquire_fence, Status::Success, - nullptr, nullptr, nullptr, this }); + buffers_.emplace_back(stream, buffer, this); } /* Clone the controls associated with the camera3 request. */ @@ -54,3 +52,15 @@ Camera3RequestDescriptor::Camera3RequestDescriptor( } Camera3RequestDescriptor::~Camera3RequestDescriptor() = default; + +Camera3RequestDescriptor::StreamBuffer::StreamBuffer( + CameraStream *stream, const camera3_stream_buffer_t &buffer, + Camera3RequestDescriptor *request) + : camera3Buffer(buffer.buffer), fence(buffer.acquire_fence), + stream(stream), request(request) +{ +} + +Camera3RequestDescriptor::StreamBuffer::~StreamBuffer() = default; + +Camera3RequestDescriptor::StreamBuffer::StreamBuffer(StreamBuffer &&other) = default; diff --git a/src/android/camera_request.h b/src/android/camera_request.h index cfafa445..618e1fc1 100644 --- a/src/android/camera_request.h +++ b/src/android/camera_request.h @@ -34,15 +34,25 @@ public: }; struct StreamBuffer { + StreamBuffer(CameraStream *stream, + const camera3_stream_buffer_t &buffer, + Camera3RequestDescriptor *request); + ~StreamBuffer(); + StreamBuffer(StreamBuffer &&other); + StreamBuffer &operator=(StreamBuffer &&) = default; + CameraStream *stream; buffer_handle_t *camera3Buffer; std::unique_ptr<libcamera::FrameBuffer> frameBuffer; int fence; - Status status; - libcamera::FrameBuffer *internalBuffer; - const libcamera::FrameBuffer *srcBuffer; + Status status = Status::Success; + libcamera::FrameBuffer *internalBuffer = nullptr; + const libcamera::FrameBuffer *srcBuffer = nullptr; std::unique_ptr<CameraBuffer> dstBuffer; Camera3RequestDescriptor *request; + + private: + LIBCAMERA_DISABLE_COPY(StreamBuffer) }; /* Keeps track of streams requiring post-processing. */
Provide a constructor for StreamBuffer and use that while populating Camera3RequestDescriptor::buffers_ vector. Also provide the default move-constructor (required as StreamBuffer is stored in a vector in Camera3RequestDescriptor) and destructor for the StreamBuffer struct. Also declare a default move assignment operator and disable the copy constructor and move operator explicitly with LIBCAMERA_DISABLE_COPY(). While at it, initialize pointers members in the StreamBuffer struct to nullptr, with StreamBuffer::status set to Status::Success. Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> --- Changes in v2: - Declare default move-assignment operator which is then explicitly disabled by LIBCAMERA_DISABLE_MOVE() macro along with copy constructor (enforced by the macro). --- src/android/camera_request.cpp | 16 +++++++++++++--- src/android/camera_request.h | 16 +++++++++++++--- 2 files changed, 26 insertions(+), 6 deletions(-)