[libcamera-devel,v8,12/17] lc-compliance: Factor common capture code into SimpleCapture

13472 diff mbox series

Factor common code from SimpleCapture* subclasses into the SimpleCapture
parent class in order to avoid duplicated code.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

---

Changes in v8:
- Added requests_ member to SimpleCapture to hold ownership of queued
  requests during capture
- Made queueRequest() virtual and removed SimpleCaptureBalanced::queueRequests()

Changes in v6:
- Switched queueRequests()'s 'buffers' and 'requests' parameters order, since
  'requests' is an output variable
- Added comment to runCaptureSession()
- Added missing blank line before return in captureCompleted()
- Added blank line to runCaptureSession()

 src/lc-compliance/simple_capture.cpp | 93 +++++++++++++++-------------
 src/lc-compliance/simple_capture.h   | 16 +++--
 2 files changed, 61 insertions(+), 48 deletions(-)

On Tue, Aug 24, 2021 at 04:56:31PM -0300, Nícolas F. R. A. Prado wrote:
> Factor common code from SimpleCapture* subclasses into the SimpleCapture
> parent class in order to avoid duplicated code.
> 
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> 
> ---
> 
> Changes in v8:
> - Added requests_ member to SimpleCapture to hold ownership of queued
>   requests during capture
> - Made queueRequest() virtual and removed SimpleCaptureBalanced::queueRequests()
> 
> Changes in v6:
> - Switched queueRequests()'s 'buffers' and 'requests' parameters order, since
>   'requests' is an output variable
> - Added comment to runCaptureSession()
> - Added missing blank line before return in captureCompleted()
> - Added blank line to runCaptureSession()
> 
>  src/lc-compliance/simple_capture.cpp | 93 +++++++++++++++-------------
>  src/lc-compliance/simple_capture.h   | 16 +++--
>  2 files changed, 61 insertions(+), 48 deletions(-)
> 
> diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp
> index 041a2a4cbb5a..e4f70974dd2c 100644
> --- a/src/lc-compliance/simple_capture.cpp
> +++ b/src/lc-compliance/simple_capture.cpp
> @@ -70,12 +70,57 @@ void SimpleCapture::stop()
>  
>  	camera_->stop();
>  
> +	requests_.clear();
> +
>  	camera_->requestCompleted.disconnect(this, &SimpleCapture::requestComplete);
>  
>  	Stream *stream = config_->at(0).stream();
>  	allocator_->free(stream);
>  }
>  
> +bool SimpleCapture::captureCompleted()
> +{
> +	captureCount_++;
> +	if (captureCount_ >= captureLimit_) {
> +		loop_->exit(0);
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
> +int SimpleCapture::queueRequest(Request *request)
> +{
> +	return camera_->queueRequest(request);
> +}
> +
> +void SimpleCapture::queueRequests(Stream *stream,
> +				  const std::vector<std::unique_ptr<FrameBuffer>> &buffers)
> +{
> +	for (const std::unique_ptr<FrameBuffer> &buffer : buffers) {
> +		std::unique_ptr<Request> request = camera_->createRequest();
> +		ASSERT_TRUE(request) << "Can't create request";
> +
> +		ASSERT_EQ(request->addBuffer(stream, buffer.get()), 0) << "Can't set buffer for request";
> +
> +		ASSERT_EQ(queueRequest(request.get()), 0) << "Failed to queue request";
> +
> +		requests_.push_back(std::move(request));
> +	}
> +}
> +
> +int SimpleCapture::runCaptureSession()
> +{
> +	loop_ = new EventLoop();
> +	int status = loop_->exec();
> +
> +	/* After session ended */
> +	stop();
> +	delete loop_;
> +
> +	return status;
> +}
> +
>  /* SimpleCaptureBalanced */
>  
>  SimpleCaptureBalanced::SimpleCaptureBalanced(std::shared_ptr<Camera> camera)
> @@ -102,24 +147,9 @@ void SimpleCaptureBalanced::capture(unsigned int numRequests)
>  	captureCount_ = 0;
>  	captureLimit_ = numRequests;
>  
> -	/* Queue the recommended number of reqeuests. */
> -	std::vector<std::unique_ptr<libcamera::Request>> requests;
> -	for (const std::unique_ptr<FrameBuffer> &buffer : buffers) {
> -		std::unique_ptr<Request> request = camera_->createRequest();
> -		ASSERT_TRUE(request) << "Can't create request";
> +	queueRequests(stream, buffers);
>  
> -		ASSERT_EQ(request->addBuffer(stream, buffer.get()), 0) << "Can't set buffer for request";
> -
> -		ASSERT_EQ(queueRequest(request.get()), 0) << "Failed to queue request";
> -
> -		requests.push_back(std::move(request));
> -	}
> -
> -	/* Run capture session. */
> -	loop_ = new EventLoop();
> -	loop_->exec();
> -	stop();
> -	delete loop_;
> +	runCaptureSession();
>  
>  	ASSERT_EQ(captureCount_, captureLimit_);
>  }
> @@ -135,11 +165,8 @@ int SimpleCaptureBalanced::queueRequest(Request *request)
>  
>  void SimpleCaptureBalanced::requestComplete(Request *request)
>  {
> -	captureCount_++;
> -	if (captureCount_ >= captureLimit_) {
> -		loop_->exit(0);
> +	if (captureCompleted())
>  		return;
> -	}
>  
>  	request->reuse(Request::ReuseBuffers);
>  	if (queueRequest(request))
> @@ -163,35 +190,17 @@ void SimpleCaptureUnbalanced::capture(unsigned int numRequests)
>  	captureCount_ = 0;
>  	captureLimit_ = numRequests;
>  
> -	/* Queue the recommended number of reqeuests. */
> -	std::vector<std::unique_ptr<libcamera::Request>> requests;
> -	for (const std::unique_ptr<FrameBuffer> &buffer : buffers) {
> -		std::unique_ptr<Request> request = camera_->createRequest();
> -		ASSERT_TRUE(request) << "Can't create request";
> -
> -		ASSERT_EQ(request->addBuffer(stream, buffer.get()), 0) << "Can't set buffer for request";
> -
> -		ASSERT_EQ(camera_->queueRequest(request.get()), 0) << "Failed to queue request";
> -
> -		requests.push_back(std::move(request));
> -	}
> +	queueRequests(stream, buffers);
>  
> -	/* Run capture session. */
> -	loop_ = new EventLoop();
> -	int status = loop_->exec();
> -	stop();
> -	delete loop_;
> +	int status = runCaptureSession();
>  
>  	ASSERT_EQ(status, 0);
>  }
>  
>  void SimpleCaptureUnbalanced::requestComplete(Request *request)
>  {
> -	captureCount_++;
> -	if (captureCount_ >= captureLimit_) {
> -		loop_->exit(0);
> +	if (captureCompleted())
>  		return;
> -	}
>  
>  	request->reuse(Request::ReuseBuffers);
>  	if (camera_->queueRequest(request))
> diff --git a/src/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h
> index 401ba8273da8..8a6db2a355f6 100644
> --- a/src/lc-compliance/simple_capture.h
> +++ b/src/lc-compliance/simple_capture.h
> @@ -25,14 +25,23 @@ protected:
>  
>  	void start();
>  	void stop();
> +	virtual int queueRequest(libcamera::Request *request);
> +	void queueRequests(libcamera::Stream *stream,
> +			   const std::vector<std::unique_ptr<libcamera::FrameBuffer>> &buffers);
> +	bool captureCompleted();
> +	int runCaptureSession();
>  
>  	virtual void requestComplete(libcamera::Request *request) = 0;
>  
>  	EventLoop *loop_;
>  
> +	unsigned int captureCount_;
> +	unsigned int captureLimit_;
> +
>  	std::shared_ptr<libcamera::Camera> camera_;
>  	std::unique_ptr<libcamera::FrameBufferAllocator> allocator_;
>  	std::unique_ptr<libcamera::CameraConfiguration> config_;
> +	std::vector<std::unique_ptr<libcamera::Request>> requests_;
>  };
>  
>  class SimpleCaptureBalanced : public SimpleCapture
> @@ -43,12 +52,10 @@ public:
>  	void capture(unsigned int numRequests);
>  
>  private:
> -	int queueRequest(libcamera::Request *request);
> +	int queueRequest(libcamera::Request *request) override;
>  	void requestComplete(libcamera::Request *request) override;
>  
>  	unsigned int queueCount_;
> -	unsigned int captureCount_;
> -	unsigned int captureLimit_;
>  };
>  
>  class SimpleCaptureUnbalanced : public SimpleCapture
> @@ -60,9 +67,6 @@ public:
>  
>  private:
>  	void requestComplete(libcamera::Request *request) override;
> -
> -	unsigned int captureCount_;
> -	unsigned int captureLimit_;
>  };
>  
>  #endif /* __LC_COMPLIANCE_SIMPLE_CAPTURE_H__ */
> -- 
> 2.33.0
>