| Message ID | 20210521133054.274502-3-nfraprado@collabora.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Nícolas, Thank you for the patch. On Fri, May 21, 2021 at 10:30:51AM -0300, Nícolas F. R. A. Prado wrote: > Make SimpleCapture::stop() be able to be called multiple times and at > any point so that it can be called from the destructor and an assert > failure can return immediately. > > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > No changes in v4 > > No changes in v3 > > Changes in v2: > - Suggested by Laurent: > - Fixed code style > > src/lc-compliance/simple_capture.cpp | 33 +++++++++++----------------- > 1 file changed, 13 insertions(+), 20 deletions(-) > > diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp > index 64e862a08e3a..f90fe6d0f9aa 100644 > --- a/src/lc-compliance/simple_capture.cpp > +++ b/src/lc-compliance/simple_capture.cpp > @@ -17,6 +17,7 @@ SimpleCapture::SimpleCapture(std::shared_ptr<Camera> camera) > > SimpleCapture::~SimpleCapture() > { > + stop(); > } > > Results::Result SimpleCapture::configure(StreamRole role) > @@ -55,12 +56,17 @@ Results::Result SimpleCapture::start() > > void SimpleCapture::stop() > { > - Stream *stream = config_->at(0).stream(); > + if (!config_) > + return; > > camera_->stop(); > > camera_->requestCompleted.disconnect(this, &SimpleCapture::requestComplete); > > + if (!allocator_->allocated()) > + return; > + > + Stream *stream = config_->at(0).stream(); > allocator_->free(stream); > } > > @@ -84,7 +90,6 @@ Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests) > if (buffers.size() > numRequests) { > /* Cache buffers.size() before we destroy it in stop() */ > int buffers_size = buffers.size(); > - stop(); > > return { Results::Skip, "Camera needs " + std::to_string(buffers_size) > + " requests, can't test only " + std::to_string(numRequests) }; > @@ -98,20 +103,14 @@ Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests) > std::vector<std::unique_ptr<libcamera::Request>> requests; > for (const std::unique_ptr<FrameBuffer> &buffer : buffers) { > std::unique_ptr<Request> request = camera_->createRequest(); > - if (!request) { > - stop(); > + if (!request) > return { Results::Fail, "Can't create request" }; > - } > > - if (request->addBuffer(stream, buffer.get())) { > - stop(); > + if (request->addBuffer(stream, buffer.get())) > return { Results::Fail, "Can't set buffer for request" }; > - } > > - if (queueRequest(request.get()) < 0) { > - stop(); > + if (queueRequest(request.get()) < 0) > return { Results::Fail, "Failed to queue request" }; > - } > > requests.push_back(std::move(request)); > } > @@ -175,20 +174,14 @@ Results::Result SimpleCaptureUnbalanced::capture(unsigned int numRequests) > std::vector<std::unique_ptr<libcamera::Request>> requests; > for (const std::unique_ptr<FrameBuffer> &buffer : buffers) { > std::unique_ptr<Request> request = camera_->createRequest(); > - if (!request) { > - stop(); > + if (!request) > return { Results::Fail, "Can't create request" }; > - } > > - if (request->addBuffer(stream, buffer.get())) { > - stop(); > + if (request->addBuffer(stream, buffer.get())) > return { Results::Fail, "Can't set buffer for request" }; > - } > > - if (camera_->queueRequest(request.get()) < 0) { > - stop(); > + if (camera_->queueRequest(request.get()) < 0) > return { Results::Fail, "Failed to queue request" }; > - } > > requests.push_back(std::move(request)); > }
diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp index 64e862a08e3a..f90fe6d0f9aa 100644 --- a/src/lc-compliance/simple_capture.cpp +++ b/src/lc-compliance/simple_capture.cpp @@ -17,6 +17,7 @@ SimpleCapture::SimpleCapture(std::shared_ptr<Camera> camera) SimpleCapture::~SimpleCapture() { + stop(); } Results::Result SimpleCapture::configure(StreamRole role) @@ -55,12 +56,17 @@ Results::Result SimpleCapture::start() void SimpleCapture::stop() { - Stream *stream = config_->at(0).stream(); + if (!config_) + return; camera_->stop(); camera_->requestCompleted.disconnect(this, &SimpleCapture::requestComplete); + if (!allocator_->allocated()) + return; + + Stream *stream = config_->at(0).stream(); allocator_->free(stream); } @@ -84,7 +90,6 @@ Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests) if (buffers.size() > numRequests) { /* Cache buffers.size() before we destroy it in stop() */ int buffers_size = buffers.size(); - stop(); return { Results::Skip, "Camera needs " + std::to_string(buffers_size) + " requests, can't test only " + std::to_string(numRequests) }; @@ -98,20 +103,14 @@ Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests) std::vector<std::unique_ptr<libcamera::Request>> requests; for (const std::unique_ptr<FrameBuffer> &buffer : buffers) { std::unique_ptr<Request> request = camera_->createRequest(); - if (!request) { - stop(); + if (!request) return { Results::Fail, "Can't create request" }; - } - if (request->addBuffer(stream, buffer.get())) { - stop(); + if (request->addBuffer(stream, buffer.get())) return { Results::Fail, "Can't set buffer for request" }; - } - if (queueRequest(request.get()) < 0) { - stop(); + if (queueRequest(request.get()) < 0) return { Results::Fail, "Failed to queue request" }; - } requests.push_back(std::move(request)); } @@ -175,20 +174,14 @@ Results::Result SimpleCaptureUnbalanced::capture(unsigned int numRequests) std::vector<std::unique_ptr<libcamera::Request>> requests; for (const std::unique_ptr<FrameBuffer> &buffer : buffers) { std::unique_ptr<Request> request = camera_->createRequest(); - if (!request) { - stop(); + if (!request) return { Results::Fail, "Can't create request" }; - } - if (request->addBuffer(stream, buffer.get())) { - stop(); + if (request->addBuffer(stream, buffer.get())) return { Results::Fail, "Can't set buffer for request" }; - } - if (camera_->queueRequest(request.get()) < 0) { - stop(); + if (camera_->queueRequest(request.get()) < 0) return { Results::Fail, "Failed to queue request" }; - } requests.push_back(std::move(request)); }
Make SimpleCapture::stop() be able to be called multiple times and at any point so that it can be called from the destructor and an assert failure can return immediately. Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> --- No changes in v4 No changes in v3 Changes in v2: - Suggested by Laurent: - Fixed code style src/lc-compliance/simple_capture.cpp | 33 +++++++++++----------------- 1 file changed, 13 insertions(+), 20 deletions(-)