[libcamera-devel,v8,15/17] lc-compliance: Add test to queue more requests than hardware depth

13476 diff mbox series

A pipeline handler should be able to handle an arbitrary number of
simultaneous requests by submitting what it can to the video device and
queuing the rest internally until resources are available. This isn't
currently done by some pipeline handlers however [1].

Add a new test to lc-compliance that submits a lot of requests at once
to check if the pipeline handler is behaving well in this situation.

[1] https://bugs.libcamera.org/show_bug.cgi?id=24

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

---

(no changes since v6)

Changes in v6:
- Made MAX_SIMULTANEOUS_REQUESTS a constexpr

Changes in v5:
- Updated to use googletest, and CameraHolder and roleToString from previous
  patches

 src/lc-compliance/capture_test.cpp   | 45 ++++++++++++++++++++++++++++
 src/lc-compliance/simple_capture.cpp | 29 ++++++++++++++++++
 src/lc-compliance/simple_capture.h   | 11 +++++++
 3 files changed, 85 insertions(+)

On Tue, Aug 24, 2021 at 04:56:34PM -0300, Nícolas F. R. A. Prado wrote:
> A pipeline handler should be able to handle an arbitrary number of
> simultaneous requests by submitting what it can to the video device and
> queuing the rest internally until resources are available. This isn't
> currently done by some pipeline handlers however [1].
> 
> Add a new test to lc-compliance that submits a lot of requests at once
> to check if the pipeline handler is behaving well in this situation.
> 
> [1] https://bugs.libcamera.org/show_bug.cgi?id=24
> 
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

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

> 
> ---
> 
> (no changes since v6)
> 
> Changes in v6:
> - Made MAX_SIMULTANEOUS_REQUESTS a constexpr
> 
> Changes in v5:
> - Updated to use googletest, and CameraHolder and roleToString from previous
>   patches
> 
>  src/lc-compliance/capture_test.cpp   | 45 ++++++++++++++++++++++++++++
>  src/lc-compliance/simple_capture.cpp | 29 ++++++++++++++++++
>  src/lc-compliance/simple_capture.h   | 11 +++++++
>  3 files changed, 85 insertions(+)
> 
> diff --git a/src/lc-compliance/capture_test.cpp b/src/lc-compliance/capture_test.cpp
> index 40242fbbc0ba..2e495810e40f 100644
> --- a/src/lc-compliance/capture_test.cpp
> +++ b/src/lc-compliance/capture_test.cpp
> @@ -18,6 +18,8 @@ using namespace libcamera;
>  const std::vector<int> NUMREQUESTS = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 };
>  const std::vector<StreamRole> ROLES = { Raw, StillCapture, VideoRecording, Viewfinder };
>  
> +static constexpr unsigned int MAX_SIMULTANEOUS_REQUESTS = 8;
> +
>  static const std::string &roleToString(const StreamRole &role)
>  {
>  	static const std::map<StreamRole, std::string> rolesMap = {
> @@ -54,12 +56,37 @@ void SingleStream::TearDown()
>  	releaseCamera();
>  }
>  
> +class RoleParametrizedTest : public testing::TestWithParam<StreamRole>, public CameraHolder
> +{
> +public:
> +	static std::string nameParameters(const testing::TestParamInfo<RoleParametrizedTest::ParamType> &info);
> +
> +protected:
> +	void SetUp() override;
> +	void TearDown() override;
> +};
> +
> +void RoleParametrizedTest::SetUp()
> +{
> +	acquireCamera();
> +}
> +
> +void RoleParametrizedTest::TearDown()
> +{
> +	releaseCamera();
> +}
> +
>  std::string SingleStream::nameParameters(const testing::TestParamInfo<SingleStream::ParamType> &info)
>  {
>  	return roleToString(std::get<0>(info.param)) + "_" +
>  	       std::to_string(std::get<1>(info.param));
>  }
>  
> +std::string RoleParametrizedTest::nameParameters(const testing::TestParamInfo<RoleParametrizedTest::ParamType> &info)
> +{
> +	return roleToString(info.param);
> +}
> +
>  /*
>   * Test single capture cycles
>   *
> @@ -122,8 +149,26 @@ TEST_P(SingleStream, UnbalancedStop)
>  	capture.capture(numRequests);
>  }
>  
> +TEST_P(RoleParametrizedTest, Overflow)
> +{
> +	auto role = GetParam();
> +
> +	SimpleCaptureOverflow capture(camera_);
> +
> +	capture.configure(role);
> +
> +	capture.allocateBuffers(MAX_SIMULTANEOUS_REQUESTS);
> +
> +	capture.capture();
> +}
> +
>  INSTANTIATE_TEST_SUITE_P(CaptureTests,
>  			 SingleStream,
>  			 testing::Combine(testing::ValuesIn(ROLES),
>  					  testing::ValuesIn(NUMREQUESTS)),
>  			 SingleStream::nameParameters);
> +
> +INSTANTIATE_TEST_SUITE_P(CaptureTests,
> +			 RoleParametrizedTest,
> +			 testing::ValuesIn(ROLES),
> +			 RoleParametrizedTest::nameParameters);
> diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp
> index e4f70974dd2c..d4805ff3403c 100644
> --- a/src/lc-compliance/simple_capture.cpp
> +++ b/src/lc-compliance/simple_capture.cpp
> @@ -206,3 +206,32 @@ void SimpleCaptureUnbalanced::requestComplete(Request *request)
>  	if (camera_->queueRequest(request))
>  		loop_->exit(-EINVAL);
>  }
> +
> +/* SimpleCaptureOverflow */
> +
> +SimpleCaptureOverflow::SimpleCaptureOverflow(std::shared_ptr<Camera> camera)
> +	: SimpleCapture(camera)
> +{
> +}
> +
> +void SimpleCaptureOverflow::capture()
> +{
> +	start();
> +
> +	Stream *stream = config_->at(0).stream();
> +	const std::vector<std::unique_ptr<FrameBuffer>> &buffers = allocator_->buffers(stream);
> +
> +	captureCount_ = 0;
> +	captureLimit_ = buffers.size();
> +
> +	queueRequests(stream, buffers);
> +
> +	runCaptureSession();
> +
> +	ASSERT_EQ(captureCount_, captureLimit_);
> +}
> +
> +void SimpleCaptureOverflow::requestComplete([[maybe_unused]] Request *request)
> +{
> +	captureCompleted();
> +}
> diff --git a/src/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h
> index 8a6db2a355f6..9d86fb32e9e0 100644
> --- a/src/lc-compliance/simple_capture.h
> +++ b/src/lc-compliance/simple_capture.h
> @@ -69,4 +69,15 @@ private:
>  	void requestComplete(libcamera::Request *request) override;
>  };
>  
> +class SimpleCaptureOverflow : public SimpleCapture
> +{
> +public:
> +	SimpleCaptureOverflow(std::shared_ptr<libcamera::Camera> camera);
> +
> +	void capture();
> +
> +private:
> +	void requestComplete(libcamera::Request *request) override;
> +};
> +
>  #endif /* __LC_COMPLIANCE_SIMPLE_CAPTURE_H__ */
> -- 
> 2.33.0
>