[libcamera-devel] [PATCH v7 10/11] libcamera: stream: Remove bufferCount
David Plowman
david.plowman at raspberrypi.com
Thu Jul 29 18:03:24 CEST 2021
Ah, OK, that sounds straightforward, then. Thanks for the explanation!
Best regards
David
On Thu, 29 Jul 2021 at 15:38, Nícolas F. R. A. Prado
<nfraprado at collabora.com> wrote:
>
> Hi David,
>
> On Thu, Jul 29, 2021 at 11:00:33AM +0100, David Plowman wrote:
> > Hi again
> >
> > I thought that perhaps I could explain in more detail why I think
> > applications need to be able to set (or influence) the buffer count
> > and not leave it to the pipeline handler. My use-case is as follows.
> >
> > I'm running some standard sort of application such as video recording
> > for which there are maybe 4 buffers allocated. Now, I also want to do
> > some image analysis that perhaps causes me to change something about
> > the recording (maybe change camera settings, or even stop the
> > recording). But this image analysis task is relatively slow.
> >
> > One option is to copy the camera frames so that I can return the
> > buffers in the request back to the camera. But copying images is
> > generally undesirable. Instead I'd prefer to hold on to the buffer
> > only now my camera pipeline is effectively working with only 3
> > buffers, which is not enough. When I eventually finish with the buffer
> > I will return it, and then grab the next one to keep again!
>
> Ohh, I see. Sorry, I got a bit confused there, but this is not a problem at all.
> If you want the pipeline to work with extra requests/buffers, you just supply
> more to it :). Going back to your code snippet, this would be the change:
>
> configuration_ = camera_->generateConfiguration({ StreamRole::StillCapture });
> - configuration_->at(0).bufferCount = <value I have determined>;
> camera_->configure(configuration_.get());
> allocator_ = new FrameBufferAllocator(camera_);
> Stream *stream = configuration_->at(0).stream();
> - allocator_->allocate(stream);
> + allocator_->allocate(stream, <value I have determined>);
> // now allocator_->buffers(stream) has the buffers I want!
>
> So, basically, you don't need to set bufferCount anymore, and instead you
> allocate as many buffers as you need to give the pipeline handler by directly
> passing the number to FrameBufferAllocator. In your case you want to allocate an
> extra buffer to compensate for the one you'll be keeping.
>
> It was already possible to supply the pipeline handler with how many
> buffers you wanted, but previously you had to allocate them elsewhere, while now
> you can allocate with the FrameBufferAllocator since it accepts the count
> argument. (Actually for the raspberry pipeline it was already possible to do
> this implicitly through bufferCount as you did, but not with the other
> pipelines).
>
> What I meant wasn't possible to change is how many internal buffers for metadata
> and parameters the pipeline uses together with the IPA. And also how many V4L2
> buffer slots are created to queue the requests into. But all of this is indeed
> only relevant to the pipeline handler, so an application shouldn't care about
> it. All of this, together with how many buffers the application would allocate,
> was previously grouped on bufferCount. With its deletion, however, they are
> getting split up.
>
> I hope I got it right this time :).
>
> Thanks,
> Nícolas
>
> >
> > So at the very least, the application needs to be able to say "please
> > allocate <n> more buffers than you thought you needed". Does that make
> > sense? Will this be possible in the new scheme?
> >
> > Thanks!
> > David
> >
> > On Fri, 23 Jul 2021 at 22:08, Nícolas F. R. A. Prado
> > <nfraprado at collabora.com> wrote:
> > >
> > > Hi David,
> > >
> > > On Fri, Jul 23, 2021 at 04:53:55PM +0100, David Plowman wrote:
> > > > Hi Nicolas
> > > >
> > > > Thanks for the explanations. I'm still not sure I entirely understand
> > > > so perhaps a quick look at how I would have to change my application
> > > > code might help? Here's what I have now (I've removed any
> > > > "unimportant" bits!):
> > > >
> > > > configuration_ = camera_->generateConfiguration({ StreamRole::StillCapture });
> > > > configuration_->at(0).bufferCount = <value I have determined>;
> > > > camera_->configure(configuration_.get());
> > > > allocator_ = new FrameBufferAllocator(camera_);
> > > > Stream *stream = configuration_->at(0).stream();
> > > > allocator_->allocate(stream);
> > > > // now allocator_->buffers(stream) has the buffers I want!
> > > >
> > > > I guess one thing to note is that the bufferCount value varies with
> > > > more than just the StreamRole, and I don't in general trust the
> > > > pipeline handler to "know" what I want. Will this still be possible?
> > >
> > > Oh I see... No, this won't be possible after we drop bufferCount. The
> > > pipeline handler will decide for itself how many buffers to allocate internally,
> > > possibly using the StreamRole as a hint.
> > >
> > > I wasn't aware this could impact an application, since all the other pipeline
> > > handlers actually ignore when bufferCount is set by the application, overriding
> > > it when the configuration is validated. But now I'm noticing the raspberrypi
> > > pipeline actually uses it.
> > >
> > > Some context: This series actually began with me making
> > > FrameBufferAllocator::allocate() receive a count argument instead of using the
> > > value of bufferCount. But since Laurent mentioned that bufferCount was going
> > > away eventually anyway I ended up removing it as well as part of this series.
> > >
> > > I do understand how being able to specify a bufferCount value that suits your
> > > specific usecase is useful, though... Laurent, what's your take on this? :)
> > >
> > > Thanks,
> > > Nícolas
> > >
> > > >
> > > > Thanks!
> > > > David
> > > >
> > > > On Fri, 23 Jul 2021 at 14:10, Nícolas F. R. A. Prado
> > > > <nfraprado at collabora.com> wrote:
> > > > >
> > > > > Hi David,
> > > > >
> > > > > On Fri, Jul 23, 2021 at 09:42:56AM +0100, David Plowman wrote:
> > > > > > Hi Nicolas
> > > > > >
> > > > > > Thanks very much for all this work! I was wondering if I could ask a
> > > > > > little more about this patch. In our applications, for example, we set
> > > > > > the bufferCount according to the use-case that the application is
> > > > > > implementing. For example:
> > > > > >
> > > > > > * For preview windows we set bufferCount to 4.
> > > > > > * For video capture we drop fewer frames with a deeper queue so we set it to 6.
> > > > > > * For stills capture we can get by with one frame, though use cases
> > > > > > that capture several stills images for fusion work better if we set
> > > > > > bufferCount to 2 or 3.
> > > > > >
> > > > > > I was wondering how our applications can achieve this behaviour if the
> > > > > > bufferCount field is no longer available.
> > > > >
> > > > > Okay, so the short answer is that the applications, knowing their particular use
> > > > > case, are responsible to provide enough requests to keep the pipeline going. On
> > > > > the pipeline handler side, it's free to use how many internal buffers it wants.
> > > > >
> > > > > So in your case, the pipeline handler could still allocate that same number of
> > > > > internal buffers as you previously did based on bufferCount, but now based
> > > > > solely on the StreamRole set. The application would then allocate and queue
> > > > > how many buffers fits the usage. (Note that before the application would call
> > > > > FrameBufferAllocator::allocate() and it would allocate 'bufferCount' buffers,
> > > > > but now the function takes a count argument so the application is free to choose
> > > > > how many)
> > > > >
> > > > > Now, I guess it would be nice for the application to be able to query what is a
> > > > > good number of buffers to use, for example 6 in the video capture of your use
> > > > > case. At the moment, the only information available is that 1 is the minimum
> > > > > number of buffers required. So the application would have to know that 6 is a
> > > > > good number for video capture. Maybe another property could be introduced later
> > > > > for this recommended amount, but previous discussions suggested this is very use
> > > > > case dependent, so might not make sense.
> > > > >
> > > > > I hope this answers the question. If not, I'm still here :).
> > > > >
> > > > > Thanks,
> > > > > Nícolas
> > > > >
> > > > > >
> > > > > > Thanks!
> > > > > > David
> > > > > >
> > > > > > On Fri, 23 Jul 2021 at 00:29, Nícolas F. R. A. Prado
> > > > > > <nfraprado at collabora.com> wrote:
> > > > > > >
> > > > > > > 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>
> > > > > > > ---
> > > > > > >
> > > > > > > No changes in v7
> > > > > > >
> > > > > > > Changes in v6:
> > > > > > > - Removed IPU3_BUFFER_COUNT as it was unused
> > > > > > >
> > > > > > > include/libcamera/stream.h | 2 --
> > > > > > > src/android/camera_stream.cpp | 2 +-
> > > > > > > src/libcamera/pipeline/ipu3/cio2.cpp | 1 -
> > > > > > > src/libcamera/pipeline/ipu3/ipu3.cpp | 8 --------
> > > > > > > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 6 ------
> > > > > > > src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 2 --
> > > > > > > src/libcamera/pipeline/simple/converter.cpp | 3 ---
> > > > > > > src/libcamera/pipeline/simple/converter.h | 3 ---
> > > > > > > src/libcamera/pipeline/simple/simple.cpp | 5 +----
> > > > > > > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 ---
> > > > > > > src/libcamera/pipeline/vimc/vimc.cpp | 3 ---
> > > > > > > src/libcamera/stream.cpp | 9 ++-------
> > > > > > > src/v4l2/v4l2_camera.cpp | 14 ++++++++++----
> > > > > > > src/v4l2/v4l2_camera.h | 5 +++--
> > > > > > > src/v4l2/v4l2_camera_proxy.cpp | 8 +++-----
> > > > > > > test/camera/buffer_import.cpp | 10 +++++++---
> > > > > > > test/libtest/buffer_source.cpp | 4 ++--
> > > > > > > test/libtest/buffer_source.h | 2 +-
> > > > > > > test/v4l2_videodevice/buffer_cache.cpp | 4 ++--
> > > > > > > 19 files changed, 32 insertions(+), 62 deletions(-)
> > > > > > >
> > > > > > > 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 b168e3c0c288..0794be409f82 100644
> > > > > > > --- a/src/android/camera_stream.cpp
> > > > > > > +++ b/src/android/camera_stream.cpp
> > > > > > > @@ -94,7 +94,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 1bcd580e251c..0e04e7214489 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/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > > > > > index 4efd201c05e5..b223b3f33cd0 100644
> > > > > > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > > > > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > > > > > @@ -38,7 +38,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 };
> > > > > > > @@ -295,7 +294,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_));
> > > > > > > @@ -339,7 +337,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);
> > > > > > >
> > > > > > > @@ -407,7 +404,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;
> > > > > > >
> > > > > > > @@ -428,7 +424,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;
> > > > > > > @@ -438,7 +433,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();
> > > > > > > @@ -457,7 +451,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;
> > > > > > > @@ -474,7 +467,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 776e0f92aed1..e6fc0425c3ef 100644
> > > > > > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > > > > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > > > > > @@ -473,7 +473,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;
> > > > > > > @@ -491,7 +490,6 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
> > > > > > > sensorFormat = findBestMode(fmts, size);
> > > > > > > pixelFormat = sensorFormat.fourcc.toPixelFormat();
> > > > > > > ASSERT(pixelFormat.isValid());
> > > > > > > - bufferCount = 2;
> > > > > > > rawCount++;
> > > > > > > break;
> > > > > > >
> > > > > > > @@ -500,7 +498,6 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
> > > > > > > pixelFormat = formats::NV12;
> > > > > > > /* Return the largest sensor resolution. */
> > > > > > > size = data->sensor_->resolution();
> > > > > > > - bufferCount = 1;
> > > > > > > outCount++;
> > > > > > > break;
> > > > > > >
> > > > > > > @@ -516,7 +513,6 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
> > > > > > > fmts = data->isp_[Isp::Output0].dev()->formats();
> > > > > > > pixelFormat = formats::YUV420;
> > > > > > > size = { 1920, 1080 };
> > > > > > > - bufferCount = 4;
> > > > > > > outCount++;
> > > > > > > break;
> > > > > > >
> > > > > > > @@ -524,7 +520,6 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
> > > > > > > fmts = data->isp_[Isp::Output0].dev()->formats();
> > > > > > > pixelFormat = formats::ARGB8888;
> > > > > > > size = { 800, 600 };
> > > > > > > - bufferCount = 4;
> > > > > > > outCount++;
> > > > > > > break;
> > > > > > >
> > > > > > > @@ -554,7 +549,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 fea330f72886..4961f3971e59 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/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp
> > > > > > > index b3bcf01483f7..fda417793e80 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 7e1d60674f62..406e63ca2a80 100644
> > > > > > > --- a/src/libcamera/pipeline/simple/converter.h
> > > > > > > +++ b/src/libcamera/pipeline/simple/converter.h
> > > > > > > @@ -87,9 +87,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 a1163eaf8be2..ded04914f610 100644
> > > > > > > --- a/src/libcamera/pipeline/simple/simple.cpp
> > > > > > > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > > > > > > @@ -628,7 +628,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);
> > > > > > > @@ -646,8 +646,6 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()
> > > > > > > cfg.stride = format.planes[0].bpl;
> > > > > > > cfg.frameSize = format.planes[0].size;
> > > > > > > }
> > > > > > > -
> > > > > > > - cfg.bufferCount = 3;
> > > > > > > }
> > > > > > >
> > > > > > > return status;
> > > > > > > @@ -770,7 +768,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;
> > > > > > >
> > > > > > > return converter_->configure(inputCfg, outputCfgs);
> > > > > > > }
> > > > > > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > > > > > index 755949e7a59a..32482300f09a 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 24ba743a946c..a698427c4361 100644
> > > > > > > --- a/src/libcamera/pipeline/vimc/vimc.cpp
> > > > > > > +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> > > > > > > @@ -167,8 +167,6 @@ CameraConfiguration::Status VimcCameraConfiguration::validate()
> > > > > > > status = Adjusted;
> > > > > > > }
> > > > > > >
> > > > > > > - cfg.bufferCount = 4;
> > > > > > > -
> > > > > > > V4L2DeviceFormat format;
> > > > > > > format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat);
> > > > > > > format.size = cfg.size;
> > > > > > > @@ -224,7 +222,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 b8626775d224..ca507b72b26a 100644
> > > > > > > --- a/src/libcamera/stream.cpp
> > > > > > > +++ b/src/libcamera/stream.cpp
> > > > > > > @@ -280,7 +280,7 @@ SizeRange StreamFormats::range(const PixelFormat &pixelformat) const
> > > > > > > * handlers provide StreamFormats.
> > > > > > > */
> > > > > > > StreamConfiguration::StreamConfiguration()
> > > > > > > - : pixelFormat(0), stride(0), frameSize(0), bufferCount(0),
> > > > > > > + : pixelFormat(0), stride(0), frameSize(0),
> > > > > > > stream_(nullptr)
> > > > > > > {
> > > > > > > }
> > > > > > > @@ -289,7 +289,7 @@ StreamConfiguration::StreamConfiguration()
> > > > > > > * \brief Construct a configuration with stream formats
> > > > > > > */
> > > > > > > StreamConfiguration::StreamConfiguration(const StreamFormats &formats)
> > > > > > > - : pixelFormat(0), stride(0), frameSize(0), bufferCount(0),
> > > > > > > + : pixelFormat(0), stride(0), frameSize(0),
> > > > > > > stream_(nullptr), formats_(formats)
> > > > > > > {
> > > > > > > }
> > > > > > > @@ -324,11 +324,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/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
> > > > > > > index d01eacfa2b84..2ff9affc6388 100644
> > > > > > > --- a/src/v4l2/v4l2_camera.cpp
> > > > > > > +++ b/src/v4l2/v4l2_camera.cpp
> > > > > > > @@ -12,6 +12,8 @@
> > > > > > >
> > > > > > > #include <libcamera/base/log.h>
> > > > > > >
> > > > > > > +#include <libcamera/property_ids.h>
> > > > > > > +
> > > > > > > using namespace libcamera;
> > > > > > >
> > > > > > > LOG_DECLARE_CATEGORY(V4L2Compat)
> > > > > > > @@ -107,14 +109,12 @@ void V4L2Camera::requestComplete(Request *request)
> > > > > > > }
> > > > > > >
> > > > > > > int V4L2Camera::configure(StreamConfiguration *streamConfigOut,
> > > > > > > - const Size &size, const PixelFormat &pixelformat,
> > > > > > > - unsigned int bufferCount)
> > > > > > > + const Size &size, const PixelFormat &pixelformat)
> > > > > > > {
> > > > > > > StreamConfiguration &streamConfig = config_->at(0);
> > > > > > > streamConfig.size.width = size.width;
> > > > > > > streamConfig.size.height = size.height;
> > > > > > > streamConfig.pixelFormat = pixelformat;
> > > > > > > - streamConfig.bufferCount = bufferCount;
> > > > > > > /* \todo memoryType (interval vs external) */
> > > > > > >
> > > > > > > CameraConfiguration::Status validation = config_->validate();
> > > > > > > @@ -146,7 +146,6 @@ int V4L2Camera::validateConfiguration(const PixelFormat &pixelFormat,
> > > > > > > StreamConfiguration &cfg = config->at(0);
> > > > > > > cfg.size = size;
> > > > > > > cfg.pixelFormat = pixelFormat;
> > > > > > > - cfg.bufferCount = 1;
> > > > > > >
> > > > > > > CameraConfiguration::Status validation = config->validate();
> > > > > > > if (validation == CameraConfiguration::Invalid)
> > > > > > > @@ -299,3 +298,10 @@ bool V4L2Camera::isRunning()
> > > > > > > {
> > > > > > > return isRunning_;
> > > > > > > }
> > > > > > > +
> > > > > > > +unsigned int V4L2Camera::minimumRequests(unsigned int count)
> > > > > > > +{
> > > > > > > + unsigned int min = camera_->properties().get(properties::MinimumRequests);
> > > > > > > +
> > > > > > > + return std::max(count, min);
> > > > > > > +}
> > > > > > > diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
> > > > > > > index a095f4e27524..4b5618ac12b5 100644
> > > > > > > --- a/src/v4l2/v4l2_camera.h
> > > > > > > +++ b/src/v4l2/v4l2_camera.h
> > > > > > > @@ -45,8 +45,7 @@ public:
> > > > > > > std::vector<Buffer> completedBuffers();
> > > > > > >
> > > > > > > int configure(StreamConfiguration *streamConfigOut,
> > > > > > > - const Size &size, const PixelFormat &pixelformat,
> > > > > > > - unsigned int bufferCount);
> > > > > > > + const Size &size, const PixelFormat &pixelformat);
> > > > > > > int validateConfiguration(const PixelFormat &pixelformat,
> > > > > > > const Size &size,
> > > > > > > StreamConfiguration *streamConfigOut);
> > > > > > > @@ -65,6 +64,8 @@ public:
> > > > > > >
> > > > > > > bool isRunning();
> > > > > > >
> > > > > > > + unsigned int minimumRequests(unsigned int count);
> > > > > > > +
> > > > > > > private:
> > > > > > > void requestComplete(Request *request);
> > > > > > >
> > > > > > > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> > > > > > > index 7682c4bddf90..634ec84e0cbb 100644
> > > > > > > --- a/src/v4l2/v4l2_camera_proxy.cpp
> > > > > > > +++ b/src/v4l2/v4l2_camera_proxy.cpp
> > > > > > > @@ -349,8 +349,7 @@ int V4L2CameraProxy::vidioc_s_fmt(V4L2CameraFile *file, struct v4l2_format *arg)
> > > > > > > Size size(arg->fmt.pix.width, arg->fmt.pix.height);
> > > > > > > V4L2PixelFormat v4l2Format = V4L2PixelFormat(arg->fmt.pix.pixelformat);
> > > > > > > ret = vcam_->configure(&streamConfig_, size,
> > > > > > > - PixelFormatInfo::info(v4l2Format).format,
> > > > > > > - bufferCount_);
> > > > > > > + PixelFormatInfo::info(v4l2Format).format);
> > > > > > > if (ret < 0)
> > > > > > > return -EINVAL;
> > > > > > >
> > > > > > > @@ -491,14 +490,13 @@ int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuf
> > > > > > > Size size(v4l2PixFormat_.width, v4l2PixFormat_.height);
> > > > > > > V4L2PixelFormat v4l2Format = V4L2PixelFormat(v4l2PixFormat_.pixelformat);
> > > > > > > int ret = vcam_->configure(&streamConfig_, size,
> > > > > > > - PixelFormatInfo::info(v4l2Format).format,
> > > > > > > - arg->count);
> > > > > > > + PixelFormatInfo::info(v4l2Format).format);
> > > > > > > if (ret < 0)
> > > > > > > return -EINVAL;
> > > > > > >
> > > > > > > setFmtFromConfig(streamConfig_);
> > > > > > >
> > > > > > > - arg->count = streamConfig_.bufferCount;
> > > > > > > + arg->count = vcam_->minimumRequests(arg->count);
> > > > > > > bufferCount_ = arg->count;
> > > > > > >
> > > > > > > ret = vcam_->allocBuffers(arg->count);
> > > > > > > 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..07fddfd2617c 100644
> > > > > > > --- a/test/v4l2_videodevice/buffer_cache.cpp
> > > > > > > +++ b/test/v4l2_videodevice/buffer_cache.cpp
> > > > > > > @@ -10,6 +10,7 @@
> > > > > > > #include <vector>
> > > > > > >
> > > > > > > #include <libcamera/formats.h>
> > > > > > > +#include <libcamera/property_ids.h>
> > > > > > > #include <libcamera/stream.h>
> > > > > > >
> > > > > > > #include "buffer_source.h"
> > > > > > > @@ -145,10 +146,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.32.0
> > > > > > >
More information about the libcamera-devel
mailing list