| Message ID | 20221216122939.256534-11-paul.elder@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
On Fri, Dec 16, 2022 at 09:29:31PM +0900, Paul Elder via libcamera-devel wrote: > From: Nícolas F. R. A. Prado <nfraprado@collabora.com> > > 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@collabora.com> > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > Changes in v9: > - rebased > > 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 +++++++++------ > .../internal/converter/converter_v4l2_m2m.h | 3 --- > include/libcamera/stream.h | 2 -- > src/android/camera_stream.cpp | 2 +- > src/libcamera/converter/converter_v4l2_m2m.cpp | 3 --- > src/libcamera/pipeline/ipu3/cio2.cpp | 1 - > 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/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 | 11 ++++++++--- > test/libtest/buffer_source.cpp | 4 ++-- > test/libtest/buffer_source.h | 2 +- > test/v4l2_videodevice/buffer_cache.cpp | 3 +-- > 18 files changed, 26 insertions(+), 62 deletions(-) > > diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst > index 70de82af..1d4314bb 100644 > --- a/Documentation/guides/pipeline-handler.rst > +++ b/Documentation/guides/pipeline-handler.rst > @@ -827,14 +827,12 @@ As well as a list of supported StreamFormats, the StreamConfiguration is also > expected to provide an initialised default configuration. This may be arbitrary, > but depending on use case you may wish 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 > @@ -900,8 +898,6 @@ Add the following function implementation to your file: > status = Adjusted; > } > > - cfg.bufferCount = 4; > - > return status; > } > > @@ -1145,13 +1141,20 @@ is performed by using the ``V4L2VideoDevice`` API, which provides an > > .. _FrameBuffer: https://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/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h > index 1f471071..bcc03347 100644 > --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h > +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h > @@ -86,9 +86,6 @@ private: > V4L2M2MConverter *converter_; > unsigned int index_; > std::unique_ptr<V4L2M2MDevice> m2m_; > - > - unsigned int inputBufferCount_; > - unsigned int outputBufferCount_; > }; > > std::unique_ptr<V4L2M2MDevice> m2m_; > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h > index 29235ddf..dba9b453 100644 > --- a/include/libcamera/stream.h > +++ b/include/libcamera/stream.h > @@ -47,8 +47,6 @@ struct StreamConfiguration { > unsigned int stride; > unsigned int frameSize; > > - unsigned int bufferCount; > - > std::optional<ColorSpace> colorSpace; > > Stream *stream() const { return stream_; } > diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp > index 045e6006..f51f0c3a 100644 > --- a/src/android/camera_stream.cpp > +++ b/src/android/camera_stream.cpp > @@ -131,7 +131,7 @@ int CameraStream::configure() > allocator_ = std::make_unique<PlatformFrameBufferAllocator>(cameraDevice_); > mutex_ = std::make_unique<Mutex>(); > > - camera3Stream_->max_buffers = configuration().bufferCount; > + camera3Stream_->max_buffers = bufferCount; Where is bufferCount defined ?? Have you compliled the HAL ? > > return 0; > } > diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp > index 9d25f25a..b9372804 100644 > --- a/src/libcamera/converter/converter_v4l2_m2m.cpp > +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp > @@ -95,9 +95,6 @@ int V4L2M2MConverter::Stream::configure(const StreamConfiguration &inputCfg, > return -EINVAL; > } > > - inputBufferCount_ = inputCfg.bufferCount; > - outputBufferCount_ = outputCfg.bufferCount; > - > return 0; > } > > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp > index feb69991..02dc9406 100644 > --- a/src/libcamera/pipeline/ipu3/cio2.cpp > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp > @@ -234,7 +234,6 @@ StreamConfiguration CIO2Device::generateConfiguration(Size size) const > > cfg.size = sensorFormat.size; > cfg.pixelFormat = mbusCodesToPixelFormat.at(sensorFormat.mbus_code); > - cfg.bufferCount = kBufferCount; > > return cfg; > } > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 4d8fcfeb..4511adde 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -98,7 +98,6 @@ private: > class IPU3CameraConfiguration : public CameraConfiguration > { > public: > - static constexpr unsigned int kBufferCount = 4; > static constexpr unsigned int kMaxStreams = 3; > > IPU3CameraConfiguration(IPU3CameraData *data); > @@ -326,7 +325,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_)); > @@ -370,7 +368,6 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() > ImgUDevice::kOutputAlignHeight); > > cfg->pixelFormat = formats::NV12; > - cfg->bufferCount = kBufferCount; > cfg->stride = info.stride(cfg->size.width, 0, 1); > cfg->frameSize = info.frameSize(cfg->size, 1); > > @@ -439,7 +436,6 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &ro > 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; > > @@ -459,7 +455,6 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &ro > .alignedDownTo(ImgUDevice::kOutputMarginWidth, > ImgUDevice::kOutputMarginHeight); > pixelFormat = formats::NV12; > - bufferCount = IPU3CameraConfiguration::kBufferCount; > streamFormats[pixelFormat] = { { ImgUDevice::kOutputMinSize, size } }; > > break; > @@ -469,7 +464,6 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &ro > 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(format); > @@ -488,7 +482,6 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &ro > .alignedDownTo(ImgUDevice::kOutputAlignWidth, > ImgUDevice::kOutputAlignHeight); > pixelFormat = formats::NV12; > - bufferCount = IPU3CameraConfiguration::kBufferCount; > streamFormats[pixelFormat] = { { ImgUDevice::kOutputMinSize, size } }; > > break; > @@ -504,7 +497,6 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &ro > 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 72502c36..c0d96024 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -591,7 +591,6 @@ PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &rol > std::unique_ptr<CameraConfiguration> config = > std::make_unique<RPiCameraConfiguration>(data); > V4L2SubdeviceFormat sensorFormat; > - unsigned int bufferCount; > PixelFormat pixelFormat; > V4L2VideoDevice::Formats fmts; > Size size; > @@ -612,7 +611,6 @@ PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &rol > BayerFormat::Packing::CSI2); > ASSERT(pixelFormat.isValid()); > colorSpace = ColorSpace::Raw; > - bufferCount = 2; > rawCount++; > break; > > @@ -627,7 +625,6 @@ PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &rol > colorSpace = ColorSpace::Sycc; > /* Return the largest sensor resolution. */ > size = sensorSize; > - bufferCount = 1; > outCount++; > break; > > @@ -648,7 +645,6 @@ PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &rol > */ > colorSpace = ColorSpace::Rec709; > size = { 1920, 1080 }; > - bufferCount = 4; > outCount++; > break; > > @@ -657,7 +653,6 @@ PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &rol > pixelFormat = formats::ARGB8888; > colorSpace = ColorSpace::Sycc; > size = { 800, 600 }; > - bufferCount = 4; > outCount++; > break; > > @@ -704,7 +699,6 @@ PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &rol > cfg.size = size; > cfg.pixelFormat = pixelFormat; > cfg.colorSpace = colorSpace; > - 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 a168e0ad..9ef9c3e2 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp > @@ -190,7 +190,6 @@ RkISP1Path::generateConfiguration(const CameraSensor *sensor, StreamRole role) > StreamConfiguration cfg(formats); > cfg.pixelFormat = format; > cfg.size = maxResolution; > - cfg.bufferCount = RKISP1_BUFFER_COUNT; > > return cfg; > } > @@ -280,7 +279,6 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, > > 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 5b53783c..366720de 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h > @@ -63,8 +63,6 @@ public: > private: > void populateFormats(); > > - static constexpr unsigned int RKISP1_BUFFER_COUNT = 4; > - > const char *name_; > bool running_; > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index 196e5252..5d309777 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -338,7 +338,6 @@ protected: > int queueRequestDevice(Camera *camera, Request *request) override; > > private: > - static constexpr unsigned int kNumInternalBuffers = 3; > static constexpr unsigned int kSimpleBufferSlotCount = 16; > > struct EntityData { > @@ -1010,7 +1009,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) = > data_->converter_->strideAndFrameSize(cfg.pixelFormat, > @@ -1029,8 +1028,6 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() > cfg.stride = format.planes[0].bpl; > cfg.frameSize = format.planes[0].size; > } > - > - cfg.bufferCount = 3; > } > > return status; > @@ -1174,7 +1171,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 18966d01..5d88e6f3 100644 > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > @@ -154,8 +154,6 @@ CameraConfiguration::Status UVCCameraConfiguration::validate() > status = Adjusted; > } > > - cfg.bufferCount = 4; > - > V4L2DeviceFormat format; > format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat); > format.size = cfg.size; > @@ -196,7 +194,6 @@ 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 eaa6ebf7..ef5146f9 100644 > --- a/src/libcamera/pipeline/vimc/vimc.cpp > +++ b/src/libcamera/pipeline/vimc/vimc.cpp > @@ -171,8 +171,6 @@ CameraConfiguration::Status VimcCameraConfiguration::validate() > status = Adjusted; > } > > - cfg.bufferCount = 4; > - > V4L2DeviceFormat format; > format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat); > format.size = cfg.size; > @@ -230,7 +228,6 @@ 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 67f30815..aa61c855 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 > - */ > - > /** > * \var StreamConfiguration::colorSpace > * \brief The ColorSpace for this stream > diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp > index 92884004..cb7a0cef 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" > @@ -93,10 +95,13 @@ protected: > return TestFail; > } > > + unsigned int bufferCount = > + camera_->properties().get(properties::MinimumRequests).value(); > + > Stream *stream = cfg.stream(); > > BufferSource source; > - int ret = source.allocate(cfg); > + int ret = source.allocate(cfg, bufferCount); > if (ret != TestPass) > return ret; > > @@ -140,10 +145,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 dde11f36..c1bc45db 100644 > --- a/test/libtest/buffer_source.cpp > +++ b/test/libtest/buffer_source.cpp > @@ -26,7 +26,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"; > @@ -78,7 +78,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 0cc71aa5..f430a7f3 100644 > --- a/test/libtest/buffer_source.h > +++ b/test/libtest/buffer_source.h > @@ -18,7 +18,7 @@ public: > BufferSource(); > ~BufferSource(); > > - int allocate(const libcamera::StreamConfiguration &config); > + int allocate(const libcamera::StreamConfiguration &config, unsigned int count); > const std::vector<std::unique_ptr<libcamera::FrameBuffer>> &buffers(); > > private: > diff --git a/test/v4l2_videodevice/buffer_cache.cpp b/test/v4l2_videodevice/buffer_cache.cpp > index 5a9aa219..8d2cf6d1 100644 > --- a/test/v4l2_videodevice/buffer_cache.cpp > +++ b/test/v4l2_videodevice/buffer_cache.cpp > @@ -174,10 +174,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.35.1 >
diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst index 70de82af..1d4314bb 100644 --- a/Documentation/guides/pipeline-handler.rst +++ b/Documentation/guides/pipeline-handler.rst @@ -827,14 +827,12 @@ As well as a list of supported StreamFormats, the StreamConfiguration is also expected to provide an initialised default configuration. This may be arbitrary, but depending on use case you may wish 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 @@ -900,8 +898,6 @@ Add the following function implementation to your file: status = Adjusted; } - cfg.bufferCount = 4; - return status; } @@ -1145,13 +1141,20 @@ is performed by using the ``V4L2VideoDevice`` API, which provides an .. _FrameBuffer: https://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/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h index 1f471071..bcc03347 100644 --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h @@ -86,9 +86,6 @@ private: V4L2M2MConverter *converter_; unsigned int index_; std::unique_ptr<V4L2M2MDevice> m2m_; - - unsigned int inputBufferCount_; - unsigned int outputBufferCount_; }; std::unique_ptr<V4L2M2MDevice> m2m_; diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h index 29235ddf..dba9b453 100644 --- a/include/libcamera/stream.h +++ b/include/libcamera/stream.h @@ -47,8 +47,6 @@ struct StreamConfiguration { unsigned int stride; unsigned int frameSize; - unsigned int bufferCount; - std::optional<ColorSpace> colorSpace; Stream *stream() const { return stream_; } diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp index 045e6006..f51f0c3a 100644 --- a/src/android/camera_stream.cpp +++ b/src/android/camera_stream.cpp @@ -131,7 +131,7 @@ int CameraStream::configure() allocator_ = std::make_unique<PlatformFrameBufferAllocator>(cameraDevice_); mutex_ = std::make_unique<Mutex>(); - camera3Stream_->max_buffers = configuration().bufferCount; + camera3Stream_->max_buffers = bufferCount; return 0; } diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp index 9d25f25a..b9372804 100644 --- a/src/libcamera/converter/converter_v4l2_m2m.cpp +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp @@ -95,9 +95,6 @@ int V4L2M2MConverter::Stream::configure(const StreamConfiguration &inputCfg, return -EINVAL; } - inputBufferCount_ = inputCfg.bufferCount; - outputBufferCount_ = outputCfg.bufferCount; - return 0; } diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp index feb69991..02dc9406 100644 --- a/src/libcamera/pipeline/ipu3/cio2.cpp +++ b/src/libcamera/pipeline/ipu3/cio2.cpp @@ -234,7 +234,6 @@ StreamConfiguration CIO2Device::generateConfiguration(Size size) const cfg.size = sensorFormat.size; cfg.pixelFormat = mbusCodesToPixelFormat.at(sensorFormat.mbus_code); - cfg.bufferCount = kBufferCount; return cfg; } diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 4d8fcfeb..4511adde 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -98,7 +98,6 @@ private: class IPU3CameraConfiguration : public CameraConfiguration { public: - static constexpr unsigned int kBufferCount = 4; static constexpr unsigned int kMaxStreams = 3; IPU3CameraConfiguration(IPU3CameraData *data); @@ -326,7 +325,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_)); @@ -370,7 +368,6 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() ImgUDevice::kOutputAlignHeight); cfg->pixelFormat = formats::NV12; - cfg->bufferCount = kBufferCount; cfg->stride = info.stride(cfg->size.width, 0, 1); cfg->frameSize = info.frameSize(cfg->size, 1); @@ -439,7 +436,6 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &ro 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; @@ -459,7 +455,6 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &ro .alignedDownTo(ImgUDevice::kOutputMarginWidth, ImgUDevice::kOutputMarginHeight); pixelFormat = formats::NV12; - bufferCount = IPU3CameraConfiguration::kBufferCount; streamFormats[pixelFormat] = { { ImgUDevice::kOutputMinSize, size } }; break; @@ -469,7 +464,6 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &ro 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(format); @@ -488,7 +482,6 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &ro .alignedDownTo(ImgUDevice::kOutputAlignWidth, ImgUDevice::kOutputAlignHeight); pixelFormat = formats::NV12; - bufferCount = IPU3CameraConfiguration::kBufferCount; streamFormats[pixelFormat] = { { ImgUDevice::kOutputMinSize, size } }; break; @@ -504,7 +497,6 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &ro 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 72502c36..c0d96024 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -591,7 +591,6 @@ PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &rol std::unique_ptr<CameraConfiguration> config = std::make_unique<RPiCameraConfiguration>(data); V4L2SubdeviceFormat sensorFormat; - unsigned int bufferCount; PixelFormat pixelFormat; V4L2VideoDevice::Formats fmts; Size size; @@ -612,7 +611,6 @@ PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &rol BayerFormat::Packing::CSI2); ASSERT(pixelFormat.isValid()); colorSpace = ColorSpace::Raw; - bufferCount = 2; rawCount++; break; @@ -627,7 +625,6 @@ PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &rol colorSpace = ColorSpace::Sycc; /* Return the largest sensor resolution. */ size = sensorSize; - bufferCount = 1; outCount++; break; @@ -648,7 +645,6 @@ PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &rol */ colorSpace = ColorSpace::Rec709; size = { 1920, 1080 }; - bufferCount = 4; outCount++; break; @@ -657,7 +653,6 @@ PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &rol pixelFormat = formats::ARGB8888; colorSpace = ColorSpace::Sycc; size = { 800, 600 }; - bufferCount = 4; outCount++; break; @@ -704,7 +699,6 @@ PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &rol cfg.size = size; cfg.pixelFormat = pixelFormat; cfg.colorSpace = colorSpace; - 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 a168e0ad..9ef9c3e2 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp @@ -190,7 +190,6 @@ RkISP1Path::generateConfiguration(const CameraSensor *sensor, StreamRole role) StreamConfiguration cfg(formats); cfg.pixelFormat = format; cfg.size = maxResolution; - cfg.bufferCount = RKISP1_BUFFER_COUNT; return cfg; } @@ -280,7 +279,6 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, 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 5b53783c..366720de 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h @@ -63,8 +63,6 @@ public: private: void populateFormats(); - static constexpr unsigned int RKISP1_BUFFER_COUNT = 4; - const char *name_; bool running_; diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 196e5252..5d309777 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -338,7 +338,6 @@ protected: int queueRequestDevice(Camera *camera, Request *request) override; private: - static constexpr unsigned int kNumInternalBuffers = 3; static constexpr unsigned int kSimpleBufferSlotCount = 16; struct EntityData { @@ -1010,7 +1009,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) = data_->converter_->strideAndFrameSize(cfg.pixelFormat, @@ -1029,8 +1028,6 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() cfg.stride = format.planes[0].bpl; cfg.frameSize = format.planes[0].size; } - - cfg.bufferCount = 3; } return status; @@ -1174,7 +1171,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 18966d01..5d88e6f3 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -154,8 +154,6 @@ CameraConfiguration::Status UVCCameraConfiguration::validate() status = Adjusted; } - cfg.bufferCount = 4; - V4L2DeviceFormat format; format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat); format.size = cfg.size; @@ -196,7 +194,6 @@ 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 eaa6ebf7..ef5146f9 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -171,8 +171,6 @@ CameraConfiguration::Status VimcCameraConfiguration::validate() status = Adjusted; } - cfg.bufferCount = 4; - V4L2DeviceFormat format; format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat); format.size = cfg.size; @@ -230,7 +228,6 @@ 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 67f30815..aa61c855 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 - */ - /** * \var StreamConfiguration::colorSpace * \brief The ColorSpace for this stream diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp index 92884004..cb7a0cef 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" @@ -93,10 +95,13 @@ protected: return TestFail; } + unsigned int bufferCount = + camera_->properties().get(properties::MinimumRequests).value(); + Stream *stream = cfg.stream(); BufferSource source; - int ret = source.allocate(cfg); + int ret = source.allocate(cfg, bufferCount); if (ret != TestPass) return ret; @@ -140,10 +145,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 dde11f36..c1bc45db 100644 --- a/test/libtest/buffer_source.cpp +++ b/test/libtest/buffer_source.cpp @@ -26,7 +26,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"; @@ -78,7 +78,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 0cc71aa5..f430a7f3 100644 --- a/test/libtest/buffer_source.h +++ b/test/libtest/buffer_source.h @@ -18,7 +18,7 @@ public: BufferSource(); ~BufferSource(); - int allocate(const libcamera::StreamConfiguration &config); + int allocate(const libcamera::StreamConfiguration &config, unsigned int count); const std::vector<std::unique_ptr<libcamera::FrameBuffer>> &buffers(); private: diff --git a/test/v4l2_videodevice/buffer_cache.cpp b/test/v4l2_videodevice/buffer_cache.cpp index 5a9aa219..8d2cf6d1 100644 --- a/test/v4l2_videodevice/buffer_cache.cpp +++ b/test/v4l2_videodevice/buffer_cache.cpp @@ -174,10 +174,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;