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

[Paul Elder]

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 at collabora.com>

Reviewed-by: Paul Elder <paul.elder at 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
>