[libcamera-devel] [PATCH v8 09/17] libcamera: stream: Remove bufferCount
Nícolas F. R. A. Prado
nfraprado at collabora.com
Tue Aug 24 21:56:28 CEST 2021
Now that the number of buffers allocated by the FrameBufferAllocator
helper is passed through FrameBufferAllocator::allocate() and the
pipelines no longer use bufferCount for internal buffer or V4L2 buffer
slots allocation, we no longer need to have bufferCount in the
StreamConfiguration, so remove it.
Signed-off-by: Nícolas F. R. A. Prado <nfraprado at collabora.com>
---
Changes in v8:
- Updated the pipeline-handler guide to use MinimumRequests instead of
bufferCount
- Removed kNumInternalBuffers as it was unused
Changes in v6:
- Removed IPU3_BUFFER_COUNT as it was unused
Documentation/guides/pipeline-handler.rst | 15 +++++++++------
include/libcamera/stream.h | 2 --
src/android/camera_stream.cpp | 2 +-
src/libcamera/pipeline/ipu3/cio2.cpp | 1 -
src/libcamera/pipeline/ipu3/cio2.h | 2 --
src/libcamera/pipeline/ipu3/ipu3.cpp | 8 --------
.../pipeline/raspberrypi/raspberrypi.cpp | 6 ------
src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 2 --
src/libcamera/pipeline/rkisp1/rkisp1_path.h | 2 --
src/libcamera/pipeline/simple/converter.cpp | 3 ---
src/libcamera/pipeline/simple/converter.h | 3 ---
src/libcamera/pipeline/simple/simple.cpp | 6 +-----
src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 ---
src/libcamera/pipeline/vimc/vimc.cpp | 3 ---
src/libcamera/stream.cpp | 12 +++---------
test/camera/buffer_import.cpp | 10 +++++++---
test/libtest/buffer_source.cpp | 4 ++--
test/libtest/buffer_source.h | 2 +-
test/v4l2_videodevice/buffer_cache.cpp | 3 +--
19 files changed, 25 insertions(+), 64 deletions(-)
diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst
index 2a69ef7d7461..3ee79b98c4dc 100644
--- a/Documentation/guides/pipeline-handler.rst
+++ b/Documentation/guides/pipeline-handler.rst
@@ -826,14 +826,12 @@ As well as a list of supported StreamFormats, the StreamConfiguration is also
expected to provide an initialsed default configuration. This may be arbitrary,
but depending on use case you may which to select an output that matches the
Sensor output, or prefer a pixelformat which might provide higher performance on
-the hardware. The bufferCount represents the number of buffers required to
-support functional continuous processing on this stream.
+the hardware.
.. code-block:: cpp
cfg.pixelFormat = formats::BGR888;
cfg.size = { 1280, 720 };
- cfg.bufferCount = 4;
Finally add each ``StreamConfiguration`` generated to the
``CameraConfiguration``, and ensure that it has been validated before returning
@@ -899,8 +897,6 @@ Add the following function implementation to your file:
status = Adjusted;
}
- cfg.bufferCount = 4;
-
return status;
}
@@ -1144,13 +1140,20 @@ is performed by using the ``V4L2VideoDevice`` API, which provides an
.. _FrameBuffer: http://libcamera.org/api-html/classlibcamera_1_1FrameBuffer.html
+The number passed to ``importBuffers()`` should be at least equal to the value
+of the ``MinimumRequests`` property in order to be possible to queue enough
+buffers to the video device that frames won't be dropped during capture. A
+bigger value can be advantageous to reduce the thrashing of dma-buf file
+descriptor mappings in case the application queues more requests and therefore
+improve performance, but for simplicity we'll just use ``MinimumRequests``.
+
Implement the pipeline handler ``start()`` function by replacing the stub
version with the following code:
.. code-block:: c++
VividCameraData *data = cameraData(camera);
- unsigned int count = data->stream_.configuration().bufferCount;
+ unsigned int count = camera->properties().get(properties::MinimumRequests);
int ret = data->video_->importBuffers(count);
if (ret < 0)
diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
index 0c55e7164592..b25f0059f2f1 100644
--- a/include/libcamera/stream.h
+++ b/include/libcamera/stream.h
@@ -45,8 +45,6 @@ struct StreamConfiguration {
unsigned int stride;
unsigned int frameSize;
- unsigned int bufferCount;
-
Stream *stream() const { return stream_; }
void setStream(Stream *stream) { stream_ = stream; }
const StreamFormats &formats() const { return formats_; }
diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
index 29be1ac5ca4f..d9ee8842938f 100644
--- a/src/android/camera_stream.cpp
+++ b/src/android/camera_stream.cpp
@@ -96,7 +96,7 @@ int CameraStream::configure()
buffers_.push_back(frameBuffer.get());
}
- camera3Stream_->max_buffers = configuration().bufferCount;
+ camera3Stream_->max_buffers = bufferCount;
return 0;
}
diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
index b940a0f6d7d6..c555ef5ffd27 100644
--- a/src/libcamera/pipeline/ipu3/cio2.cpp
+++ b/src/libcamera/pipeline/ipu3/cio2.cpp
@@ -214,7 +214,6 @@ StreamConfiguration CIO2Device::generateConfiguration(Size size) const
cfg.size = sensorFormat.size;
cfg.pixelFormat = mbusCodesToPixelFormat.at(sensorFormat.mbus_code);
- cfg.bufferCount = CIO2_BUFFER_COUNT;
return cfg;
}
diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h
index ab915b6a16fa..50ccb20765d3 100644
--- a/src/libcamera/pipeline/ipu3/cio2.h
+++ b/src/libcamera/pipeline/ipu3/cio2.h
@@ -30,8 +30,6 @@ struct StreamConfiguration;
class CIO2Device
{
public:
- static constexpr unsigned int CIO2_BUFFER_COUNT = 4;
-
CIO2Device();
std::vector<PixelFormat> formats() const;
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index cc519ae6adbe..4a60f00ecf8b 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -39,7 +39,6 @@ namespace libcamera {
LOG_DEFINE_CATEGORY(IPU3)
-static constexpr unsigned int IPU3_BUFFER_COUNT = 4;
static constexpr unsigned int IPU3_MAX_STREAMS = 3;
static const Size IMGU_OUTPUT_MIN_SIZE = { 2, 2 };
static const Size IMGU_OUTPUT_MAX_SIZE = { 4480, 34004 };
@@ -313,7 +312,6 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
/* Initialize the RAW stream with the CIO2 configuration. */
cfg->size = cio2Configuration_.size;
cfg->pixelFormat = cio2Configuration_.pixelFormat;
- cfg->bufferCount = cio2Configuration_.bufferCount;
cfg->stride = info.stride(cfg->size.width, 0, 64);
cfg->frameSize = info.frameSize(cfg->size, 64);
cfg->setStream(const_cast<Stream *>(&data_->rawStream_));
@@ -357,7 +355,6 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
IMGU_OUTPUT_HEIGHT_ALIGN);
cfg->pixelFormat = formats::NV12;
- cfg->bufferCount = IPU3_BUFFER_COUNT;
cfg->stride = info.stride(cfg->size.width, 0, 1);
cfg->frameSize = info.frameSize(cfg->size, 1);
@@ -425,7 +422,6 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,
Size sensorResolution = data->cio2_.sensor()->resolution();
for (const StreamRole role : roles) {
std::map<PixelFormat, std::vector<SizeRange>> streamFormats;
- unsigned int bufferCount;
PixelFormat pixelFormat;
Size size;
@@ -446,7 +442,6 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,
size.height = utils::alignDown(size.height - 1,
IMGU_OUTPUT_HEIGHT_MARGIN);
pixelFormat = formats::NV12;
- bufferCount = IPU3_BUFFER_COUNT;
streamFormats[pixelFormat] = { { IMGU_OUTPUT_MIN_SIZE, size } };
break;
@@ -456,7 +451,6 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,
data->cio2_.generateConfiguration(sensorResolution);
pixelFormat = cio2Config.pixelFormat;
size = cio2Config.size;
- bufferCount = cio2Config.bufferCount;
for (const PixelFormat &format : data->cio2_.formats())
streamFormats[format] = data->cio2_.sizes();
@@ -475,7 +469,6 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,
.alignedDownTo(IMGU_OUTPUT_WIDTH_ALIGN,
IMGU_OUTPUT_HEIGHT_ALIGN);
pixelFormat = formats::NV12;
- bufferCount = IPU3_BUFFER_COUNT;
streamFormats[pixelFormat] = { { IMGU_OUTPUT_MIN_SIZE, size } };
break;
@@ -492,7 +485,6 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,
StreamConfiguration cfg(formats);
cfg.size = size;
cfg.pixelFormat = pixelFormat;
- cfg.bufferCount = bufferCount;
config->addConfiguration(cfg);
}
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index a7c1cc1d5001..12d6729044e6 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -492,7 +492,6 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
RPiCameraData *data = cameraData(camera);
CameraConfiguration *config = new RPiCameraConfiguration(data);
V4L2DeviceFormat sensorFormat;
- unsigned int bufferCount;
PixelFormat pixelFormat;
V4L2VideoDevice::Formats fmts;
Size size;
@@ -510,7 +509,6 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
sensorFormat = findBestMode(fmts, size);
pixelFormat = sensorFormat.fourcc.toPixelFormat();
ASSERT(pixelFormat.isValid());
- bufferCount = 2;
rawCount++;
break;
@@ -519,7 +517,6 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
pixelFormat = formats::NV12;
/* Return the largest sensor resolution. */
size = data->sensor_->resolution();
- bufferCount = 1;
outCount++;
break;
@@ -535,7 +532,6 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
fmts = data->isp_[Isp::Output0].dev()->formats();
pixelFormat = formats::YUV420;
size = { 1920, 1080 };
- bufferCount = 4;
outCount++;
break;
@@ -543,7 +539,6 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
fmts = data->isp_[Isp::Output0].dev()->formats();
pixelFormat = formats::ARGB8888;
size = { 800, 600 };
- bufferCount = 4;
outCount++;
break;
@@ -573,7 +568,6 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
StreamConfiguration cfg(formats);
cfg.size = size;
cfg.pixelFormat = pixelFormat;
- cfg.bufferCount = bufferCount;
config->addConfiguration(cfg);
}
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
index 515f4be16d7e..9bbdf951d9b6 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
@@ -61,7 +61,6 @@ StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution)
StreamConfiguration cfg(formats);
cfg.pixelFormat = formats::NV12;
cfg.size = maxResolution;
- cfg.bufferCount = RKISP1_BUFFER_COUNT;
return cfg;
}
@@ -77,7 +76,6 @@ CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)
cfg->size.boundTo(maxResolution_);
cfg->size.expandTo(minResolution_);
- cfg->bufferCount = RKISP1_BUFFER_COUNT;
V4L2DeviceFormat format;
format.fourcc = video_->toV4L2PixelFormat(cfg->pixelFormat);
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
index 267d8f988ace..dd54fc609da6 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h
+++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
@@ -57,8 +57,6 @@ public:
Signal<FrameBuffer *> &bufferReady() { return video_->bufferReady; }
private:
- static constexpr unsigned int RKISP1_BUFFER_COUNT = 4;
-
const char *name_;
bool running_;
diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp
index 3133b3dbda07..46ab503b7c38 100644
--- a/src/libcamera/pipeline/simple/converter.cpp
+++ b/src/libcamera/pipeline/simple/converter.cpp
@@ -89,9 +89,6 @@ int SimpleConverter::Stream::configure(const StreamConfiguration &inputCfg,
return -EINVAL;
}
- inputBufferCount_ = inputCfg.bufferCount;
- outputBufferCount_ = outputCfg.bufferCount;
-
return 0;
}
diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h
index deb3df0d08df..365b99e9853e 100644
--- a/src/libcamera/pipeline/simple/converter.h
+++ b/src/libcamera/pipeline/simple/converter.h
@@ -86,9 +86,6 @@ private:
SimpleConverter *converter_;
unsigned int index_;
std::unique_ptr<V4L2M2MDevice> m2m_;
-
- unsigned int inputBufferCount_;
- unsigned int outputBufferCount_;
};
std::string deviceNode_;
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index d0a658a23be8..6deba5d7dd61 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -252,7 +252,6 @@ protected:
int queueRequestDevice(Camera *camera, Request *request) override;
private:
- static constexpr unsigned int kNumInternalBuffers = 3;
static constexpr unsigned int kSimpleBufferSlotCount = 16;
SimpleCameraData *cameraData(Camera *camera)
@@ -638,7 +637,7 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()
cfg.size != pipeConfig_->captureSize)
needConversion_ = true;
- /* Set the stride, frameSize and bufferCount. */
+ /* Set the stride and frameSize. */
if (needConversion_) {
std::tie(cfg.stride, cfg.frameSize) =
converter->strideAndFrameSize(cfg.pixelFormat, cfg.size);
@@ -656,8 +655,6 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()
cfg.stride = format.planes[0].bpl;
cfg.frameSize = format.planes[0].size;
}
-
- cfg.bufferCount = 3;
}
return status;
@@ -780,7 +777,6 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
inputCfg.pixelFormat = pipeConfig->captureFormat;
inputCfg.size = pipeConfig->captureSize;
inputCfg.stride = captureFormat.planes[0].bpl;
- inputCfg.bufferCount = kNumInternalBuffers;
/* Set the MinimumRequests property. */
unsigned int minimumRequests;
diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
index c210cf57750f..5977312a795d 100644
--- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
+++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
@@ -150,8 +150,6 @@ CameraConfiguration::Status UVCCameraConfiguration::validate()
status = Adjusted;
}
- cfg.bufferCount = 4;
-
V4L2DeviceFormat format;
format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat);
format.size = cfg.size;
@@ -193,7 +191,6 @@ CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera,
cfg.pixelFormat = formats.pixelformats().front();
cfg.size = formats.sizes(cfg.pixelFormat).back();
- cfg.bufferCount = 4;
config->addConfiguration(cfg);
diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
index d2943f61a745..ad71bfc67228 100644
--- a/src/libcamera/pipeline/vimc/vimc.cpp
+++ b/src/libcamera/pipeline/vimc/vimc.cpp
@@ -170,8 +170,6 @@ CameraConfiguration::Status VimcCameraConfiguration::validate()
status = Adjusted;
}
- cfg.bufferCount = 4;
-
V4L2DeviceFormat format;
format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat);
format.size = cfg.size;
@@ -227,7 +225,6 @@ CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera,
cfg.pixelFormat = formats::BGR888;
cfg.size = { 1920, 1080 };
- cfg.bufferCount = 4;
config->addConfiguration(cfg);
diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
index b421e17ecb36..d4cc0fafb76f 100644
--- a/src/libcamera/stream.cpp
+++ b/src/libcamera/stream.cpp
@@ -280,8 +280,7 @@ SizeRange StreamFormats::range(const PixelFormat &pixelformat) const
* handlers provide StreamFormats.
*/
StreamConfiguration::StreamConfiguration()
- : pixelFormat(0), stride(0), frameSize(0), bufferCount(0),
- stream_(nullptr)
+ : pixelFormat(0), stride(0), frameSize(0), stream_(nullptr)
{
}
@@ -289,8 +288,8 @@ StreamConfiguration::StreamConfiguration()
* \brief Construct a configuration with stream formats
*/
StreamConfiguration::StreamConfiguration(const StreamFormats &formats)
- : pixelFormat(0), stride(0), frameSize(0), bufferCount(0),
- stream_(nullptr), formats_(formats)
+ : pixelFormat(0), stride(0), frameSize(0), stream_(nullptr),
+ formats_(formats)
{
}
@@ -324,11 +323,6 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats)
* validating the configuration with a call to CameraConfiguration::validate().
*/
-/**
- * \var StreamConfiguration::bufferCount
- * \brief Requested number of buffers to allocate for the stream
- */
-
/**
* \fn StreamConfiguration::stream()
* \brief Retrieve the stream associated with the configuration
diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp
index c504ea09e64b..67ac0ad20e15 100644
--- a/test/camera/buffer_import.cpp
+++ b/test/camera/buffer_import.cpp
@@ -16,6 +16,8 @@
#include <libcamera/base/thread.h>
#include <libcamera/base/timer.h>
+#include <libcamera/property_ids.h>
+
#include "libcamera/internal/device_enumerator.h"
#include "libcamera/internal/media_device.h"
#include "libcamera/internal/v4l2_videodevice.h"
@@ -92,10 +94,12 @@ protected:
return TestFail;
}
+ unsigned int bufferCount = camera_->properties().get(properties::MinimumRequests);
+
Stream *stream = cfg.stream();
BufferSource source;
- int ret = source.allocate(cfg);
+ int ret = source.allocate(cfg, bufferCount);
if (ret != TestPass)
return ret;
@@ -139,10 +143,10 @@ protected:
while (timer.isRunning())
dispatcher->processEvents();
- if (completeRequestsCount_ < cfg.bufferCount * 2) {
+ if (completeRequestsCount_ < bufferCount * 2) {
std::cout << "Failed to capture enough frames (got "
<< completeRequestsCount_ << " expected at least "
- << cfg.bufferCount * 2 << ")" << std::endl;
+ << bufferCount * 2 << ")" << std::endl;
return TestFail;
}
diff --git a/test/libtest/buffer_source.cpp b/test/libtest/buffer_source.cpp
index 73563f2fc39d..c3d5286a2462 100644
--- a/test/libtest/buffer_source.cpp
+++ b/test/libtest/buffer_source.cpp
@@ -24,7 +24,7 @@ BufferSource::~BufferSource()
media_->release();
}
-int BufferSource::allocate(const StreamConfiguration &config)
+int BufferSource::allocate(const StreamConfiguration &config, unsigned int count)
{
/* Locate and open the video device. */
std::string videoDeviceName = "vivid-000-vid-out";
@@ -77,7 +77,7 @@ int BufferSource::allocate(const StreamConfiguration &config)
return TestFail;
}
- if (video->allocateBuffers(config.bufferCount, &buffers_) < 0) {
+ if (video->allocateBuffers(count, &buffers_) < 0) {
std::cout << "Failed to allocate buffers" << std::endl;
return TestFail;
}
diff --git a/test/libtest/buffer_source.h b/test/libtest/buffer_source.h
index 14b4770e8d8a..6a18e269a575 100644
--- a/test/libtest/buffer_source.h
+++ b/test/libtest/buffer_source.h
@@ -20,7 +20,7 @@ public:
BufferSource();
~BufferSource();
- int allocate(const StreamConfiguration &config);
+ int allocate(const StreamConfiguration &config, unsigned int count);
const std::vector<std::unique_ptr<FrameBuffer>> &buffers();
private:
diff --git a/test/v4l2_videodevice/buffer_cache.cpp b/test/v4l2_videodevice/buffer_cache.cpp
index b3f2bec11783..6c8183800d0b 100644
--- a/test/v4l2_videodevice/buffer_cache.cpp
+++ b/test/v4l2_videodevice/buffer_cache.cpp
@@ -145,10 +145,9 @@ public:
StreamConfiguration cfg;
cfg.pixelFormat = formats::YUYV;
cfg.size = Size(600, 800);
- cfg.bufferCount = numBuffers;
BufferSource source;
- int ret = source.allocate(cfg);
+ int ret = source.allocate(cfg, numBuffers);
if (ret != TestPass)
return ret;
--
2.33.0
More information about the libcamera-devel
mailing list