[libcamera-devel] [PATCH v2 1/3] qcam: Allow for a second raw stream to be configured

Laurent Pinchart laurent.pinchart at ideasonboard.com
Fri May 1 18:50:31 CEST 2020


Hi Niklas,

Thank you for the patch.

On Fri, May 01, 2020 at 05:27:43PM +0200, Niklas Söderlund wrote:
> Allow a second stream to be configured for raw capture. This change only
> adds support for configuring and allocating buffers for the second
> stream. Later changes are needed to queue the allocated buffers to the
> camera when the user wishes to capture a raw frame.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> * Changes since v1
> - Keep doneQueue_ as a QQueue and keep the original processing
>   granularity.
> ---
>  src/qcam/main_window.cpp | 125 +++++++++++++++++++++++++++------------
>  src/qcam/main_window.h   |   8 ++-
>  2 files changed, 94 insertions(+), 39 deletions(-)
> 
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index b9348111dfa28914..dc8824dae4669a7e 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -281,17 +281,30 @@ void MainWindow::toggleCapture(bool start)
>  int MainWindow::startCapture()
>  {
>  	StreamRoles roles = StreamKeyValueParser::roles(options_[OptStream]);
> +	std::vector<Request *> requests;
>  	int ret;
>  
>  	/* Verify roles are supported. */
> -	if (roles.size() != 1) {
> -		qCritical() << "Only one stream supported";
> -		return -EINVAL;
> -	}
> -
> -	if (roles[0] != StreamRole::Viewfinder) {
> -		qCritical() << "Only viewfinder supported";
> -		return -EINVAL;
> +	switch (roles.size()) {
> +	case 1:
> +		if (roles[0] != StreamRole::Viewfinder) {
> +			qWarning() << "Only viewfinder supported for single stream";
> +			return -EINVAL;
> +		}
> +		break;
> +	case 2:
> +		if (roles[0] != StreamRole::Viewfinder ||
> +		    roles[1] != StreamRole::StillCaptureRaw) {
> +			qWarning() << "Only viewfinder + raw supported for dual streams";
> +			return -EINVAL;
> +		}
> +		break;
> +	default:
> +		if (roles.size() != 1) {
> +			qWarning() << "Unsuported stream configuration";
> +			return -EINVAL;
> +		}
> +		break;
>  	}
>  
>  	/* Configure the camera. */
> @@ -301,17 +314,17 @@ int MainWindow::startCapture()
>  		return -EINVAL;
>  	}
>  
> -	StreamConfiguration &cfg = config_->at(0);
> +	StreamConfiguration &vfConfig = config_->at(0);
>  
>  	/* Use a format supported by the viewfinder if available. */
> -	std::vector<PixelFormat> formats = cfg.formats().pixelformats();
> +	std::vector<PixelFormat> formats = vfConfig.formats().pixelformats();
>  	for (const PixelFormat &format : viewfinder_->nativeFormats()) {
>  		auto match = std::find_if(formats.begin(), formats.end(),
>  					  [&](const PixelFormat &f) {
>  						  return f == format;
>  					  });
>  		if (match != formats.end()) {
> -			cfg.pixelFormat = format;
> +			vfConfig.pixelFormat = format;
>  			break;
>  		}
>  	}
> @@ -331,7 +344,7 @@ int MainWindow::startCapture()
>  
>  	if (validation == CameraConfiguration::Adjusted)
>  		qInfo() << "Stream configuration adjusted to "
> -			<< cfg.toString().c_str();
> +			<< vfConfig.toString().c_str();
>  
>  	ret = camera_->configure(config_.get());
>  	if (ret < 0) {
> @@ -339,10 +352,16 @@ int MainWindow::startCapture()
>  		return ret;
>  	}
>  
> +	/* Store stream allocation. */
> +	vfStream_ = config_->at(0).stream();
> +	if (config_->size() == 2)
> +		rawStream_ = config_->at(1).stream();
> +	else
> +		rawStream_ = nullptr;
> +
>  	/* Configure the viewfinder. */
> -	Stream *stream = cfg.stream();
> -	ret = viewfinder_->setFormat(cfg.pixelFormat,
> -				     QSize(cfg.size.width, cfg.size.height));
> +	ret = viewfinder_->setFormat(vfConfig.pixelFormat,
> +				     QSize(vfConfig.size.width, vfConfig.size.height));
>  	if (ret < 0) {
>  		qInfo() << "Failed to set viewfinder format";
>  		return ret;
> @@ -350,16 +369,33 @@ int MainWindow::startCapture()
>  
>  	adjustSize();
>  
> -	/* Allocate buffers and requests. */
> +	/* Allocate and map buffers. */
>  	allocator_ = new FrameBufferAllocator(camera_);
> -	ret = allocator_->allocate(stream);
> -	if (ret < 0) {
> -		qWarning() << "Failed to allocate capture buffers";
> -		return ret;
> +	for (StreamConfiguration &config : *config_) {
> +		Stream *stream = config.stream();
> +
> +		ret = allocator_->allocate(stream);
> +		if (ret < 0) {
> +			qWarning() << "Failed to allocate capture buffers";
> +			goto error;
> +		}
> +
> +		for (const std::unique_ptr<FrameBuffer> &buffer : allocator_->buffers(stream)) {
> +			/* Map memory buffers and cache the mappings. */
> +			const FrameBuffer::Plane &plane = buffer->planes().front();
> +			void *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED,
> +					    plane.fd.fd(), 0);
> +			mappedBuffers_[buffer.get()] = { memory, plane.length };
> +
> +			/* Store buffers on the free list. */
> +			freeBuffers_[stream].enqueue(buffer.get());
> +		}
>  	}
>  
> -	std::vector<Request *> requests;
> -	for (const std::unique_ptr<FrameBuffer> &buffer : allocator_->buffers(stream)) {
> +	/* Create requests and fill it with buffers from the viewfinder. */

s/it/them/

> +	while (!freeBuffers_[vfStream_].isEmpty()) {
> +		FrameBuffer *buffer = freeBuffers_[vfStream_].dequeue();
> +
>  		Request *request = camera_->createRequest();
>  		if (!request) {
>  			qWarning() << "Can't create request";
> @@ -367,19 +403,13 @@ int MainWindow::startCapture()
>  			goto error;
>  		}
>  
> -		ret = request->addBuffer(stream, buffer.get());
> +		ret = request->addBuffer(vfStream_, buffer);
>  		if (ret < 0) {
>  			qWarning() << "Can't set buffer for request";
>  			goto error;
>  		}
>  
>  		requests.push_back(request);
> -
> -		/* Map memory buffers and cache the mappings. */
> -		const FrameBuffer::Plane &plane = buffer->planes().front();
> -		void *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED,
> -				    plane.fd.fd(), 0);
> -		mappedBuffers_[buffer.get()] = { memory, plane.length };
>  	}
>  
>  	/* Start the title timer and the camera. */
> @@ -424,6 +454,8 @@ error:
>  	}
>  	mappedBuffers_.clear();
>  
> +	freeBuffers_.clear();
> +
>  	delete allocator_;
>  	allocator_ = nullptr;
>  
> @@ -466,6 +498,7 @@ void MainWindow::stopCapture()
>  	 * but not processed yet. Clear the queue of done buffers to avoid
>  	 * racing with the event handler.
>  	 */
> +	freeBuffers_.clear();
>  	doneQueue_.clear();
>  
>  	titleTimer_.stop();
> @@ -505,12 +538,9 @@ void MainWindow::requestComplete(Request *request)
>  	 * are not allowed. Add the buffer to the done queue and post a
>  	 * CaptureEvent for the application thread to handle.
>  	 */
> -	const std::map<Stream *, FrameBuffer *> &buffers = request->buffers();
> -	FrameBuffer *buffer = buffers.begin()->second;
> -
>  	{
>  		QMutexLocker locker(&mutex_);
> -		doneQueue_.enqueue(buffer);
> +		doneQueue_.enqueue(request->buffers());
>  	}
>  
>  	QCoreApplication::postEvent(this, new CaptureEvent);
> @@ -523,16 +553,38 @@ void MainWindow::processCapture()
>  	 * if stopCapture() has been called while a CaptureEvent was posted but
>  	 * not processed yet. Return immediately in that case.
>  	 */
> -	FrameBuffer *buffer;
> +	std::map<Stream *, FrameBuffer *> buffers;
>  
>  	{
>  		QMutexLocker locker(&mutex_);
>  		if (doneQueue_.isEmpty())
>  			return;
>  
> -		buffer = doneQueue_.dequeue();
> +		buffers = doneQueue_.dequeue();
>  	}
>  
> +	/* Process buffers. */
> +	if (buffers.count(vfStream_))
> +		processViewfinder(buffers[vfStream_]);
> +
> +	/*
> +	 * Return buffers so they can be reused. No processing involving
> +	 * a buffer can happen after they are returned to the free list.
> +	 */
> +	for (auto &it : buffers) {
> +		Stream *stream = it.first;
> +		FrameBuffer *buffer = it.second;
> +
> +		/* The ViewFinder manages the viewfinder buffers. */
> +		if (stream == vfStream_)
> +			continue;
> +
> +		freeBuffers_[stream].enqueue(buffer);
> +	}

This looks a bit dodgy as the free buffers list is only used when
starting the stream, but it will get used for raw capture in the next
patches, so it's fine.

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

> +}
> +
> +void MainWindow::processViewfinder(FrameBuffer *buffer)
> +{
>  	framesCaptured_++;
>  
>  	const FrameMetadata &metadata = buffer->metadata();
> @@ -559,8 +611,7 @@ void MainWindow::queueRequest(FrameBuffer *buffer)
>  		return;
>  	}
>  
> -	Stream *stream = config_->at(0).stream();
> -	request->addBuffer(stream, buffer);
> +	request->addBuffer(vfStream_, buffer);
>  
>  	camera_->queueRequest(request);
>  }
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index aea1f1dee20fcbb6..4856ecc10729159c 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -69,6 +69,7 @@ private:
>  
>  	void requestComplete(Request *request);
>  	void processCapture();
> +	void processViewfinder(FrameBuffer *buffer);
>  
>  	/* UI elements */
>  	QToolBar *toolbar_;
> @@ -95,8 +96,11 @@ private:
>  
>  	/* Capture state, buffers queue and statistics */
>  	bool isCapturing_;
> -	QQueue<FrameBuffer *> doneQueue_;
> -	QMutex mutex_;	/* Protects doneQueue_ */
> +	Stream *vfStream_;
> +	Stream *rawStream_;
> +	std::map<Stream *, QQueue<FrameBuffer *>> freeBuffers_;
> +	QQueue<std::map<Stream *, FrameBuffer *>> doneQueue_;
> +	QMutex mutex_; /* Protects freeBuffers_ and doneQueue_ */
>  
>  	uint64_t lastBufferTime_;
>  	QElapsedTimer frameRateInterval_;

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list