| Message ID | 20221024000356.29521-13-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
On Mon, Oct 24, 2022 at 03:03:55AM +0300, Laurent Pinchart wrote: > From: Florian Sylvestre <fsylvestre@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@baylibre.com> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Paul Elder <paul.elder@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_; > > 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 >
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@jmondi.org> On Mon, Oct 24, 2022 at 03:03:55AM +0300, Laurent Pinchart via libcamera-devel wrote: > From: Florian Sylvestre <fsylvestre@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@baylibre.com> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > Signed-off-by: Laurent Pinchart <laurent.pinchart@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 ? 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 Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > > 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 >
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@jmondi.org> > > On Mon, Oct 24, 2022 at 03:03:55AM +0300, Laurent Pinchart via libcamera-devel wrote: > > From: Florian Sylvestre <fsylvestre@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@baylibre.com> > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@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@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);
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_; 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);