[libcamera-devel] [PATCH 14/30] libcamera: request: In addBuffer() do not fetch stream from Buffer

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Wed, Nov 27, 2019 at 12:36:04AM +0100, Niklas Söderlund wrote:
> In the FrameBuffer interface the stream will not be available from the
> buffer object as the buffer might be allocated externally. The
> application needs to explicitly state which stream the buffer is being
> added for to the request.
> 
> Extend the addBuffer() function to get this information explicitly from
> the caller.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> ---
>  include/libcamera/request.h   | 2 +-
>  src/android/camera_device.cpp | 2 +-
>  src/cam/capture.cpp           | 4 ++--
>  src/libcamera/request.cpp     | 4 ++--
>  src/qcam/main_window.cpp      | 4 ++--
>  test/camera/buffer_import.cpp | 2 +-
>  test/camera/capture.cpp       | 4 ++--
>  test/camera/statemachine.cpp  | 2 +-
>  8 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> index 2d5a5964e99eb75f..a8708010ec1247a2 100644
> --- a/include/libcamera/request.h
> +++ b/include/libcamera/request.h
> @@ -39,7 +39,7 @@ public:
>  	ControlList &controls() { return *controls_; }
>  	ControlList &metadata() { return *metadata_; }
>  	const std::map<Stream *, Buffer *> &buffers() const { return bufferMap_; }
> -	int addBuffer(std::unique_ptr<Buffer> buffer);
> +	int addBuffer(Stream *stream, std::unique_ptr<Buffer> buffer);
>  	Buffer *findBuffer(Stream *stream) const;
>  
>  	uint64_t cookie() const { return cookie_; }
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 065e0292e186c2ad..09588c16a6301649 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -754,7 +754,7 @@ void CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reque
>  
>  	Request *request =
>  		camera_->createRequest(reinterpret_cast<uint64_t>(descriptor));
> -	request->addBuffer(std::move(buffer));
> +	request->addBuffer(stream, std::move(buffer));
>  
>  	int ret = camera_->queueRequest(request);
>  	if (ret) {
> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
> index 4b65b1d0a2dbed35..1a4dbe7ce4a15a2d 100644
> --- a/src/cam/capture.cpp
> +++ b/src/cam/capture.cpp
> @@ -95,7 +95,7 @@ int Capture::capture(EventLoop *loop)
>  			Stream *stream = cfg.stream();
>  			std::unique_ptr<Buffer> buffer = stream->createBuffer(i);
>  
> -			ret = request->addBuffer(std::move(buffer));
> +			ret = request->addBuffer(stream, std::move(buffer));
>  			if (ret < 0) {
>  				std::cerr << "Can't set buffer for request"
>  					  << std::endl;
> @@ -185,7 +185,7 @@ void Capture::requestComplete(Request *request)
>  			return;
>  		}
>  
> -		request->addBuffer(std::move(newBuffer));
> +		request->addBuffer(stream, std::move(newBuffer));
>  	}
>  
>  	camera_->queueRequest(request);
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index c2854dc2e8caab2e..3b49e52510918eee 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -113,6 +113,7 @@ Request::~Request()
>  
>  /**
>   * \brief Store a Buffer with its associated Stream in the Request
> + * \param[in] stream The stream the buffer belongs to
>   * \param[in] buffer The Buffer to store in the request
>   *
>   * Ownership of the buffer is passed to the request. It will be deleted when
> @@ -125,9 +126,8 @@ Request::~Request()
>   * \retval -EEXIST The request already contains a buffer for the stream
>   * \retval -EINVAL The buffer does not reference a valid Stream
>   */
> -int Request::addBuffer(std::unique_ptr<Buffer> buffer)
> +int Request::addBuffer(Stream *stream, std::unique_ptr<Buffer> buffer)
>  {
> -	Stream *stream = buffer->stream();
>  	if (!stream) {
>  		LOG(Request, Error) << "Invalid stream reference";
>  		return -EINVAL;
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index a828a176cbbf4854..4cecf7e351214f3d 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -190,7 +190,7 @@ int MainWindow::startCapture()
>  			goto error;
>  		}
>  
> -		ret = request->addBuffer(std::move(buffer));
> +		ret = request->addBuffer(stream, std::move(buffer));
>  		if (ret < 0) {
>  			std::cerr << "Can't set buffer for request" << std::endl;
>  			goto error;
> @@ -288,7 +288,7 @@ void MainWindow::requestComplete(Request *request)
>  			return;
>  		}
>  
> -		request->addBuffer(std::move(newBuffer));
> +		request->addBuffer(stream, std::move(newBuffer));
>  	}
>  
>  	camera_->queueRequest(request);
> diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp
> index 8b0d1c167bd2bbe8..dae907f9f41841c9 100644
> --- a/test/camera/buffer_import.cpp
> +++ b/test/camera/buffer_import.cpp
> @@ -268,7 +268,7 @@ public:
>  		Request *request = camera_->createRequest(cookie);
>  
>  		std::unique_ptr<Buffer> buffer = stream_->createBuffer({ dmabuf, -1, -1 });
> -		request->addBuffer(move(buffer));
> +		request->addBuffer(stream_, move(buffer));
>  		camera_->queueRequest(request);
>  	}
>  
> diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
> index 7cb76038f557d461..ca1ebe419946dd4d 100644
> --- a/test/camera/capture.cpp
> +++ b/test/camera/capture.cpp
> @@ -49,7 +49,7 @@ protected:
>  		std::unique_ptr<Buffer> newBuffer = stream->createBuffer(buffer->index());
>  
>  		request = camera_->createRequest();
> -		request->addBuffer(std::move(newBuffer));
> +		request->addBuffer(stream, std::move(newBuffer));
>  		camera_->queueRequest(request);
>  	}
>  
> @@ -101,7 +101,7 @@ protected:
>  				return TestFail;
>  			}
>  
> -			if (request->addBuffer(std::move(buffer))) {
> +			if (request->addBuffer(stream, std::move(buffer))) {
>  				cout << "Failed to associating buffer with request" << endl;
>  				return TestFail;
>  			}
> diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp
> index afa13ba77b0b7fe0..f627b8f37422350e 100644
> --- a/test/camera/statemachine.cpp
> +++ b/test/camera/statemachine.cpp
> @@ -219,7 +219,7 @@ protected:
>  
>  		Stream *stream = *camera_->streams().begin();
>  		std::unique_ptr<Buffer> buffer = stream->createBuffer(0);
> -		if (request->addBuffer(std::move(buffer)))
> +		if (request->addBuffer(stream, std::move(buffer)))
>  			return TestFail;
>  
>  		if (camera_->queueRequest(request))

-- 
Regards,

Laurent Pinchart