[libcamera-devel] [PATCH v3 12/13] pipeline: rkisp1: Support raw Bayer capture at runtime
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sun Oct 30 18:26:48 CET 2022
Hi Jacopo,
On Wed, Oct 26, 2022 at 06:58:04PM +0200, Jacopo Mondi wrote:
> Hi Laurent,
>
> first of all I've been testing this as well and it works, I can
> capture raw images and they look correct, without any noticeable regression
> in the other capture modes
>
> Tested-by: Jacopo Mondi <jacopo at jmondi.org>
>
> On Mon, Oct 24, 2022 at 03:03:55AM +0300, Laurent Pinchart via libcamera-devel wrote:
> > From: Florian Sylvestre <fsylvestre at baylibre.com>
> >
> > Implement support for raw Bayer capture at runtime, from start() to
> > stop(). Support of raw formats in the camera configuration is split to a
> > subsequent change to ease review.
> >
> > In raw mode, the ISP is bypassed. There is no need to provide parameter
> > buffers, and the ISP will not generate statistics. This requires
> > multiple changes in the buffer handling:
> >
> > - The params and stats buffers don't need to be allocated, and the
> > corresponding video nodes don't need to be started or stopped.
> >
> > - The IPA module fillParamsBuffer() operation must not be called in
> > queueRequestDevice(). As a result, the IPA module thus doesn't emit
> > the paramsBufferReady signal. The main and self path video buffers
> > must thus be queued directly in queueRequestDevice().
> >
> > - The tryCompleteRequest() function must not to wait until the params
> > buffer has been dequeued.
> >
> > - When the frame buffer has been captured, the IPA module
> > processStatsBuffer() operation must be called directly to fill request
> > metadata.
> >
> > Signed-off-by: Florian Sylvestre <fsylvestre at baylibre.com>
> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > Changes since v3:
> >
> > - Split from "pipeline: rkisp1: Support raw Bayer capture"
> > - Drop new completeRaw() IPA operation
> > - Don't queue params buffers in raw capture mode
> > - Fix assertion failure when stopping capture
> > ---
> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 148 ++++++++++++++---------
> > 1 file changed, 93 insertions(+), 55 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index dcab5286aa56..e57411544f7a 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -67,7 +67,8 @@ class RkISP1Frames
> > public:
> > RkISP1Frames(PipelineHandler *pipe);
> >
> > - RkISP1FrameInfo *create(const RkISP1CameraData *data, Request *request);
> > + RkISP1FrameInfo *create(const RkISP1CameraData *data, Request *request,
> > + bool isRaw);
> > int destroy(unsigned int frame);
> > void clear();
> >
> > @@ -184,6 +185,7 @@ private:
> > std::unique_ptr<V4L2Subdevice> csi_;
> >
> > bool hasSelfPath_;
> > + bool isRaw_;
>
> Will a single variable global to the whole pipeline handler prevents
> any use case in future ?
I don't think so, as we can't use multiple cameras concurrently;
> Right now I don't think it's an issue, but it might be nicer to have
> this per-CameraData.
>
> However, in all the platforms we have, the ISP has a single input
> hence it's not possible to have different applications using multiple
> cameras at the same time, so this is not a real issue for now
Even with multiple inputs, there's a single ISP, it can only use one
sensor at a time.
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
>
> > RkISP1MainPath mainPath_;
> > RkISP1SelfPath selfPath_;
> > @@ -203,28 +205,35 @@ RkISP1Frames::RkISP1Frames(PipelineHandler *pipe)
> > {
> > }
> >
> > -RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *request)
> > +RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *request,
> > + bool isRaw)
> > {
> > unsigned int frame = data->frame_;
> >
> > - if (pipe_->availableParamBuffers_.empty()) {
> > - LOG(RkISP1, Error) << "Parameters buffer underrun";
> > - return nullptr;
> > - }
> > - FrameBuffer *paramBuffer = pipe_->availableParamBuffers_.front();
> > + FrameBuffer *paramBuffer = nullptr;
> > + FrameBuffer *statBuffer = nullptr;
> > +
> > + if (!isRaw) {
> > + if (pipe_->availableParamBuffers_.empty()) {
> > + LOG(RkISP1, Error) << "Parameters buffer underrun";
> > + return nullptr;
> > + }
> > +
> > + if (pipe_->availableStatBuffers_.empty()) {
> > + LOG(RkISP1, Error) << "Statisitc buffer underrun";
> > + return nullptr;
> > + }
> > +
> > + paramBuffer = pipe_->availableParamBuffers_.front();
> > + pipe_->availableParamBuffers_.pop();
> >
> > - if (pipe_->availableStatBuffers_.empty()) {
> > - LOG(RkISP1, Error) << "Statisitc buffer underrun";
> > - return nullptr;
> > + statBuffer = pipe_->availableStatBuffers_.front();
> > + pipe_->availableStatBuffers_.pop();
> > }
> > - FrameBuffer *statBuffer = pipe_->availableStatBuffers_.front();
> >
> > FrameBuffer *mainPathBuffer = request->findBuffer(&data->mainPathStream_);
> > FrameBuffer *selfPathBuffer = request->findBuffer(&data->selfPathStream_);
> >
> > - pipe_->availableParamBuffers_.pop();
> > - pipe_->availableStatBuffers_.pop();
> > -
> > RkISP1FrameInfo *info = new RkISP1FrameInfo;
> >
> > info->frame = frame;
> > @@ -665,6 +674,8 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> > << "ISP input pad configured with " << format
> > << " crop " << rect;
> >
> > + isRaw_ = false;
> > +
> > /* YUYV8_2X8 is required on the ISP source path pad for YUV output. */
> > format.mbus_code = MEDIA_BUS_FMT_YUYV8_2X8;
> > LOG(RkISP1, Debug)
> > @@ -760,13 +771,15 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
> > data->selfPathStream_.configuration().bufferCount,
> > });
> >
> > - ret = param_->allocateBuffers(maxCount, ¶mBuffers_);
> > - if (ret < 0)
> > - goto error;
> > + if (!isRaw_) {
> > + ret = param_->allocateBuffers(maxCount, ¶mBuffers_);
> > + if (ret < 0)
> > + goto error;
> >
> > - ret = stat_->allocateBuffers(maxCount, &statBuffers_);
> > - if (ret < 0)
> > - goto error;
> > + ret = stat_->allocateBuffers(maxCount, &statBuffers_);
> > + if (ret < 0)
> > + goto error;
> > + }
> >
> > for (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) {
> > buffer->setCookie(ipaBufferId++);
> > @@ -842,23 +855,25 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL
> >
> > data->frame_ = 0;
> >
> > - ret = param_->streamOn();
> > - if (ret) {
> > - data->ipa_->stop();
> > - freeBuffers(camera);
> > - LOG(RkISP1, Error)
> > - << "Failed to start parameters " << camera->id();
> > - return ret;
> > - }
> > + if (!isRaw_) {
> > + ret = param_->streamOn();
> > + if (ret) {
> > + data->ipa_->stop();
> > + freeBuffers(camera);
> > + LOG(RkISP1, Error)
> > + << "Failed to start parameters " << camera->id();
> > + return ret;
> > + }
> >
> > - ret = stat_->streamOn();
> > - if (ret) {
> > - param_->streamOff();
> > - data->ipa_->stop();
> > - freeBuffers(camera);
> > - LOG(RkISP1, Error)
> > - << "Failed to start statistics " << camera->id();
> > - return ret;
> > + ret = stat_->streamOn();
> > + if (ret) {
> > + param_->streamOff();
> > + data->ipa_->stop();
> > + freeBuffers(camera);
> > + LOG(RkISP1, Error)
> > + << "Failed to start statistics " << camera->id();
> > + return ret;
> > + }
> > }
> >
> > if (data->mainPath_->isEnabled()) {
> > @@ -903,15 +918,17 @@ void PipelineHandlerRkISP1::stopDevice(Camera *camera)
> > selfPath_.stop();
> > mainPath_.stop();
> >
> > - ret = stat_->streamOff();
> > - if (ret)
> > - LOG(RkISP1, Warning)
> > - << "Failed to stop statistics for " << camera->id();
> > + if (!isRaw_) {
> > + ret = stat_->streamOff();
> > + if (ret)
> > + LOG(RkISP1, Warning)
> > + << "Failed to stop statistics for " << camera->id();
> >
> > - ret = param_->streamOff();
> > - if (ret)
> > - LOG(RkISP1, Warning)
> > - << "Failed to stop parameters for " << camera->id();
> > + ret = param_->streamOff();
> > + if (ret)
> > + LOG(RkISP1, Warning)
> > + << "Failed to stop parameters for " << camera->id();
> > + }
> >
> > ASSERT(data->queuedRequests_.empty());
> > data->frameInfo_.clear();
> > @@ -925,12 +942,21 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)
> > {
> > RkISP1CameraData *data = cameraData(camera);
> >
> > - RkISP1FrameInfo *info = data->frameInfo_.create(data, request);
> > + RkISP1FrameInfo *info = data->frameInfo_.create(data, request, isRaw_);
> > if (!info)
> > return -ENOENT;
> >
> > data->ipa_->queueRequest(data->frame_, request->controls());
> > - data->ipa_->fillParamsBuffer(data->frame_, info->paramBuffer->cookie());
> > + if (isRaw_) {
> > + if (info->mainPathBuffer)
> > + data->mainPath_->queueBuffer(info->mainPathBuffer);
> > +
> > + if (data->selfPath_ && info->selfPathBuffer)
> > + data->selfPath_->queueBuffer(info->selfPathBuffer);
> > + } else {
> > + data->ipa_->fillParamsBuffer(data->frame_,
> > + info->paramBuffer->cookie());
> > + }
> >
> > data->frame_++;
> >
> > @@ -1135,7 +1161,7 @@ void PipelineHandlerRkISP1::tryCompleteRequest(RkISP1FrameInfo *info)
> > if (!info->metadataProcessed)
> > return;
> >
> > - if (!info->paramDequeued)
> > + if (!isRaw_ && !info->paramDequeued)
> > return;
> >
> > data->frameInfo_.destroy(info->frame);
> > @@ -1152,16 +1178,28 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
> > if (!info)
> > return;
> >
> > + const FrameMetadata &metadata = buffer->metadata();
> > Request *request = buffer->request();
> >
> > - /*
> > - * Record the sensor's timestamp in the request metadata.
> > - *
> > - * \todo The sensor timestamp should be better estimated by connecting
> > - * to the V4L2Device::frameStart signal.
> > - */
> > - request->metadata().set(controls::SensorTimestamp,
> > - buffer->metadata().timestamp);
> > + if (metadata.status != FrameMetadata::FrameCancelled) {
> > + /*
> > + * Record the sensor's timestamp in the request metadata.
> > + *
> > + * \todo The sensor timestamp should be better estimated by connecting
> > + * to the V4L2Device::frameStart signal.
> > + */
> > + request->metadata().set(controls::SensorTimestamp,
> > + metadata.timestamp);
> > +
> > + if (isRaw_) {
> > + const ControlList &ctrls =
> > + data->delayedCtrls_->get(metadata.sequence);
> > + data->ipa_->processStatsBuffer(info->frame, 0, ctrls);
> > + }
> > + } else {
> > + if (isRaw_)
> > + info->metadataProcessed = true;
> > + }
> >
> > completeBuffer(request, buffer);
> > tryCompleteRequest(info);
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list