[libcamera-devel,v7,03/11] lc-compliance: Move buffer allocation to separate function
diff mbox series

Message ID 20210722232851.747614-4-nfraprado@collabora.com
State Superseded
Headers show
Series
  • lc-compliance: Add test to queue more requests than hardware depth
Related show

Commit Message

Nícolas F. R. A. Prado July 22, 2021, 11:28 p.m. UTC
Move buffer allocation to its own function and with a count argument so
tests can specify how many buffers to allocate.

Also add an overloaded function with no arguments and that allocates
MinNumRequests buffers for easier use by current tests.

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

No changes in v7

No changes in v6

Added in v5

 src/lc-compliance/capture_test.cpp   |  8 +++++++-
 src/lc-compliance/simple_capture.cpp | 15 +++++++++++----
 src/lc-compliance/simple_capture.h   |  2 ++
 3 files changed, 20 insertions(+), 5 deletions(-)

Comments

Laurent Pinchart Aug. 1, 2021, 8:41 p.m. UTC | #1
Hi Nícolas,

On Thu, Jul 22, 2021 at 08:28:43PM -0300, Nícolas F. R. A. Prado wrote:
> Move buffer allocation to its own function and with a count argument so
> tests can specify how many buffers to allocate.
> 
> Also add an overloaded function with no arguments and that allocates
> MinNumRequests buffers for easier use by current tests.

It's now called MinimumRequests.

> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
> 
> No changes in v7
> 
> No changes in v6
> 
> Added in v5
> 
>  src/lc-compliance/capture_test.cpp   |  8 +++++++-
>  src/lc-compliance/simple_capture.cpp | 15 +++++++++++----
>  src/lc-compliance/simple_capture.h   |  2 ++
>  3 files changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/src/lc-compliance/capture_test.cpp b/src/lc-compliance/capture_test.cpp
> index 52578207c11f..6439cbd88f8e 100644
> --- a/src/lc-compliance/capture_test.cpp
> +++ b/src/lc-compliance/capture_test.cpp
> @@ -80,6 +80,8 @@ TEST_P(SingleStream, Capture)
>  
>  	capture.configure(role);
>  
> +	capture.allocateBuffers();
> +
>  	capture.capture(numRequests);
>  }
>  
> @@ -99,8 +101,10 @@ TEST_P(SingleStream, CaptureStartStop)
>  
>  	capture.configure(role);
>  
> -	for (unsigned int starts = 0; starts < numRepeats; starts++)
> +	for (unsigned int starts = 0; starts < numRepeats; starts++) {
> +		capture.allocateBuffers();
>  		capture.capture(numRequests);
> +	}
>  }
>  
>  /*
> @@ -118,6 +122,8 @@ TEST_P(SingleStream, UnbalancedStop)
>  
>  	capture.configure(role);
>  
> +	capture.allocateBuffers();
> +
>  	capture.capture(numRequests);
>  }
>  
> diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp
> index 6444203547be..8683d9050806 100644
> --- a/src/lc-compliance/simple_capture.cpp
> +++ b/src/lc-compliance/simple_capture.cpp
> @@ -44,15 +44,22 @@ void SimpleCapture::configure(StreamRole role)
>  	}
>  }
>  
> -void SimpleCapture::start()
> +void SimpleCapture::allocateBuffers()
> +{
> +	allocateBuffers(camera_->properties().get(properties::MinimumRequests));
> +}
> +
> +void SimpleCapture::allocateBuffers(unsigned int count)
>  {
> -	unsigned int bufferCount = camera_->properties().get(properties::MinNumRequests);
>  	Stream *stream = config_->at(0).stream();
> -	int count = allocator_->allocate(stream, bufferCount);
> +	int allocatedCount = allocator_->allocate(stream, count);

You could also declare a single function with a default value for the
count parameter

	void allocateBuffers(unsigned int count = 0);

and have

	if (!count)
		count = camera_->properties().get(properties::MinimumRequests);

here.

>  
>  	ASSERT_GE(count, 0) << "Failed to allocate buffers";
> -	EXPECT_EQ(static_cast<unsigned int>(count), bufferCount) << "Allocated less buffers than expected";
> +	EXPECT_EQ(static_cast<unsigned int>(allocatedCount), count) << "Allocated less buffers than expected";
> +}
>  
> +void SimpleCapture::start()
> +{
>  	camera_->requestCompleted.connect(this, &SimpleCapture::requestComplete);
>  
>  	ASSERT_EQ(camera_->start(), 0) << "Failed to start camera";
> diff --git a/src/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h
> index 100ffd6637ad..1a1e874a528c 100644
> --- a/src/lc-compliance/simple_capture.h
> +++ b/src/lc-compliance/simple_capture.h
> @@ -17,6 +17,8 @@ class SimpleCapture
>  {
>  public:
>  	void configure(libcamera::StreamRole role);
> +	void allocateBuffers();
> +	void allocateBuffers(unsigned int count);
>  
>  protected:
>  	SimpleCapture(std::shared_ptr<libcamera::Camera> camera);

Patch
diff mbox series

diff --git a/src/lc-compliance/capture_test.cpp b/src/lc-compliance/capture_test.cpp
index 52578207c11f..6439cbd88f8e 100644
--- a/src/lc-compliance/capture_test.cpp
+++ b/src/lc-compliance/capture_test.cpp
@@ -80,6 +80,8 @@  TEST_P(SingleStream, Capture)
 
 	capture.configure(role);
 
+	capture.allocateBuffers();
+
 	capture.capture(numRequests);
 }
 
@@ -99,8 +101,10 @@  TEST_P(SingleStream, CaptureStartStop)
 
 	capture.configure(role);
 
-	for (unsigned int starts = 0; starts < numRepeats; starts++)
+	for (unsigned int starts = 0; starts < numRepeats; starts++) {
+		capture.allocateBuffers();
 		capture.capture(numRequests);
+	}
 }
 
 /*
@@ -118,6 +122,8 @@  TEST_P(SingleStream, UnbalancedStop)
 
 	capture.configure(role);
 
+	capture.allocateBuffers();
+
 	capture.capture(numRequests);
 }
 
diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp
index 6444203547be..8683d9050806 100644
--- a/src/lc-compliance/simple_capture.cpp
+++ b/src/lc-compliance/simple_capture.cpp
@@ -44,15 +44,22 @@  void SimpleCapture::configure(StreamRole role)
 	}
 }
 
-void SimpleCapture::start()
+void SimpleCapture::allocateBuffers()
+{
+	allocateBuffers(camera_->properties().get(properties::MinimumRequests));
+}
+
+void SimpleCapture::allocateBuffers(unsigned int count)
 {
-	unsigned int bufferCount = camera_->properties().get(properties::MinNumRequests);
 	Stream *stream = config_->at(0).stream();
-	int count = allocator_->allocate(stream, bufferCount);
+	int allocatedCount = allocator_->allocate(stream, count);
 
 	ASSERT_GE(count, 0) << "Failed to allocate buffers";
-	EXPECT_EQ(static_cast<unsigned int>(count), bufferCount) << "Allocated less buffers than expected";
+	EXPECT_EQ(static_cast<unsigned int>(allocatedCount), count) << "Allocated less buffers than expected";
+}
 
+void SimpleCapture::start()
+{
 	camera_->requestCompleted.connect(this, &SimpleCapture::requestComplete);
 
 	ASSERT_EQ(camera_->start(), 0) << "Failed to start camera";
diff --git a/src/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h
index 100ffd6637ad..1a1e874a528c 100644
--- a/src/lc-compliance/simple_capture.h
+++ b/src/lc-compliance/simple_capture.h
@@ -17,6 +17,8 @@  class SimpleCapture
 {
 public:
 	void configure(libcamera::StreamRole role);
+	void allocateBuffers();
+	void allocateBuffers(unsigned int count);
 
 protected:
 	SimpleCapture(std::shared_ptr<libcamera::Camera> camera);