[libcamera-devel] [PATCH 08/13] libcamera: pipeline: rkisp1: Prefix main path video and resizer
Niklas Söderlund
niklas.soderlund at ragnatech.se
Mon Sep 14 12:27:44 CEST 2020
Hi Laurent,
On 2020-08-20 18:20:07 +0300, Laurent Pinchart wrote:
> Hi Niklas,
>
> Thank you for the patch.
>
> On Thu, Aug 13, 2020 at 02:52:41AM +0200, Niklas Söderlund wrote:
> > In preparation of supporting both the main and self path prefix the main
> > path specific variables with mainPath.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > ---
> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 82 ++++++++++++------------
> > 1 file changed, 41 insertions(+), 41 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index 59614a9f470b7802..60179a1151f18491 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -66,7 +66,7 @@ struct RkISP1FrameInfo {
> >
> > FrameBuffer *paramBuffer;
> > FrameBuffer *statBuffer;
> > - FrameBuffer *videoBuffer;
> > + FrameBuffer *mainPathBuffer;
> >
> > bool paramFilled;
> > bool paramDequeued;
> > @@ -131,7 +131,7 @@ class RkISP1CameraData : public CameraData
> > public:
> > RkISP1CameraData(PipelineHandler *pipe, V4L2VideoDevice *video)
> > : CameraData(pipe), sensor_(nullptr), frame_(0),
> > - frameInfo_(pipe), video_(video)
> > + frameInfo_(pipe), mainPathVideo_(video)
> > {
> > }
> >
> > @@ -142,14 +142,14 @@ public:
> >
> > int loadIPA();
> >
> > - Stream stream_;
> > + Stream mainPathStream_;
> > CameraSensor *sensor_;
> > unsigned int frame_;
> > std::vector<IPABuffer> ipaBuffers_;
> > RkISP1Frames frameInfo_;
> > RkISP1Timeline timeline_;
> >
> > - V4L2VideoDevice *video_;
> > + V4L2VideoDevice *mainPathVideo_;
> >
> > private:
> > void queueFrameAction(unsigned int frame,
> > @@ -225,8 +225,8 @@ private:
> >
> > MediaDevice *media_;
> > V4L2Subdevice *isp_;
> > - V4L2Subdevice *resizer_;
> > - V4L2VideoDevice *video_;
> > + V4L2Subdevice *mainPathResizer_;
> > + V4L2VideoDevice *mainPathVideo_;
> > V4L2VideoDevice *param_;
> > V4L2VideoDevice *stat_;
>
> Would it make sense to create a RkISP1Path class and move the resizer
> anv video node there ? The RkISP1Path instances would be stored in
> PipelineHandlerRkISP1. I think we can drop the video_ member of
> RkISP1CameraData and access it from the pipe in
> RkISP1CameraData::validate().
I think this is a good idea. I do however think we should do this in a
separate series, either before this series or on-top. I really hate
having to post series as large as this as they take forever to get
merged and extending it with more stuff will only make it harder ;-)
Would you be OK with this work being done on-top?
>
> >
> > @@ -259,8 +259,8 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req
> > }
> > FrameBuffer *statBuffer = pipe_->availableStatBuffers_.front();
> >
> > - FrameBuffer *videoBuffer = request->findBuffer(&data->stream_);
> > - if (!videoBuffer) {
> > + FrameBuffer *mainPathBuffer = request->findBuffer(&data->mainPathStream_);
> > + if (!mainPathBuffer) {
> > LOG(RkISP1, Error)
> > << "Attempt to queue request with invalid stream";
> > return nullptr;
> > @@ -274,7 +274,7 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req
> > info->frame = frame;
> > info->request = request;
> > info->paramBuffer = paramBuffer;
> > - info->videoBuffer = videoBuffer;
> > + info->mainPathBuffer = mainPathBuffer;
> > info->statBuffer = statBuffer;
> > info->paramFilled = false;
> > info->paramDequeued = false;
> > @@ -333,7 +333,7 @@ RkISP1FrameInfo *RkISP1Frames::find(FrameBuffer *buffer)
> >
> > if (info->paramBuffer == buffer ||
> > info->statBuffer == buffer ||
> > - info->videoBuffer == buffer)
> > + info->mainPathBuffer == buffer)
> > return info;
> > }
> >
> > @@ -405,7 +405,7 @@ protected:
> >
> > pipe_->param_->queueBuffer(info->paramBuffer);
> > pipe_->stat_->queueBuffer(info->statBuffer);
> > - pipe_->video_->queueBuffer(info->videoBuffer);
> > + pipe_->mainPathVideo_->queueBuffer(info->mainPathBuffer);
> > }
> >
> > private:
> > @@ -544,10 +544,10 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
> > cfg.bufferCount = RKISP1_BUFFER_COUNT;
> >
> > V4L2DeviceFormat format = {};
> > - format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat);
> > + format.fourcc = data_->mainPathVideo_->toV4L2PixelFormat(cfg.pixelFormat);
> > format.size = cfg.size;
> >
> > - int ret = data_->video_->tryFormat(&format);
> > + int ret = data_->mainPathVideo_->tryFormat(&format);
> > if (ret)
> > return Invalid;
> >
> > @@ -558,8 +558,8 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
> > }
> >
> > PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)
> > - : PipelineHandler(manager), isp_(nullptr), resizer_(nullptr),
> > - video_(nullptr), param_(nullptr), stat_(nullptr)
> > + : PipelineHandler(manager), isp_(nullptr), mainPathResizer_(nullptr),
> > + mainPathVideo_(nullptr), param_(nullptr), stat_(nullptr)
> > {
> > }
> >
> > @@ -567,8 +567,8 @@ PipelineHandlerRkISP1::~PipelineHandlerRkISP1()
> > {
> > delete param_;
> > delete stat_;
> > - delete video_;
> > - delete resizer_;
> > + delete mainPathVideo_;
> > + delete mainPathResizer_;
> > delete isp_;
> > }
> >
> > @@ -649,7 +649,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> >
> > LOG(RkISP1, Debug) << "ISP output pad configured with " << format.toString();
> >
> > - ret = resizer_->setFormat(0, &format);
> > + ret = mainPathResizer_->setFormat(0, &format);
> > if (ret < 0)
> > return ret;
> >
> > @@ -659,23 +659,23 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> >
> > LOG(RkISP1, Debug) << "Configuring resizer output pad with " << format.toString();
> >
> > - ret = resizer_->setFormat(1, &format);
> > + ret = mainPathResizer_->setFormat(1, &format);
> > if (ret < 0)
> > return ret;
> >
> > LOG(RkISP1, Debug) << "Resizer output pad configured with " << format.toString();
> >
> > V4L2DeviceFormat outputFormat = {};
> > - outputFormat.fourcc = video_->toV4L2PixelFormat(cfg.pixelFormat);
> > + outputFormat.fourcc = mainPathVideo_->toV4L2PixelFormat(cfg.pixelFormat);
> > outputFormat.size = cfg.size;
> > outputFormat.planesCount = 2;
> >
> > - ret = video_->setFormat(&outputFormat);
> > + ret = mainPathVideo_->setFormat(&outputFormat);
> > if (ret)
> > return ret;
> >
> > if (outputFormat.size != cfg.size ||
> > - outputFormat.fourcc != video_->toV4L2PixelFormat(cfg.pixelFormat)) {
> > + outputFormat.fourcc != mainPathVideo_->toV4L2PixelFormat(cfg.pixelFormat)) {
> > LOG(RkISP1, Error)
> > << "Unable to configure capture in " << cfg.toString();
> > return -EINVAL;
> > @@ -693,7 +693,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> > if (ret)
> > return ret;
> >
> > - cfg.setStream(&data->stream_);
> > + cfg.setStream(&data->mainPathStream_);
> >
> > return 0;
> > }
> > @@ -702,17 +702,17 @@ int PipelineHandlerRkISP1::exportFrameBuffers(Camera *camera, Stream *stream,
> > std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > {
> > unsigned int count = stream->configuration().bufferCount;
> > - return video_->exportBuffers(count, buffers);
> > + return mainPathVideo_->exportBuffers(count, buffers);
> > }
> >
> > int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
> > {
> > RkISP1CameraData *data = cameraData(camera);
> > - unsigned int count = data->stream_.configuration().bufferCount;
> > + unsigned int count = data->mainPathStream_.configuration().bufferCount;
> > unsigned int ipaBufferId = 1;
> > int ret;
> >
> > - ret = video_->importBuffers(count);
> > + ret = mainPathVideo_->importBuffers(count);
> > if (ret < 0)
> > goto error;
> >
> > @@ -745,7 +745,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
> > error:
> > paramBuffers_.clear();
> > statBuffers_.clear();
> > - video_->releaseBuffers();
> > + mainPathVideo_->releaseBuffers();
> >
> > return ret;
> > }
> > @@ -776,8 +776,8 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)
> > if (stat_->releaseBuffers())
> > LOG(RkISP1, Error) << "Failed to release stat buffers";
> >
> > - if (video_->releaseBuffers())
> > - LOG(RkISP1, Error) << "Failed to release video buffers";
> > + if (mainPathVideo_->releaseBuffers())
> > + LOG(RkISP1, Error) << "Failed to release main path buffers";
> >
> > return 0;
> > }
> > @@ -821,7 +821,7 @@ int PipelineHandlerRkISP1::start(Camera *camera)
> > return ret;
> > }
> >
> > - ret = video_->streamOn();
> > + ret = mainPathVideo_->streamOn();
> > if (ret) {
> > param_->streamOff();
> > stat_->streamOff();
> > @@ -846,8 +846,8 @@ int PipelineHandlerRkISP1::start(Camera *camera)
> >
> > std::map<unsigned int, IPAStream> streamConfig;
> > streamConfig[0] = {
> > - .pixelFormat = data->stream_.configuration().pixelFormat,
> > - .size = data->stream_.configuration().size,
> > + .pixelFormat = data->mainPathStream_.configuration().pixelFormat,
> > + .size = data->mainPathStream_.configuration().size,
> > };
> >
> > std::map<unsigned int, const ControlInfoMap &> entityControls;
> > @@ -865,7 +865,7 @@ void PipelineHandlerRkISP1::stop(Camera *camera)
> > RkISP1CameraData *data = cameraData(camera);
> > int ret;
> >
> > - ret = video_->streamOff();
> > + ret = mainPathVideo_->streamOff();
> > if (ret)
> > LOG(RkISP1, Warning)
> > << "Failed to stop camera " << camera->id();
> > @@ -950,7 +950,7 @@ int PipelineHandlerRkISP1::initLinks(const Camera *camera,
> >
> > for (const StreamConfiguration &cfg : config) {
> > std::string resizer;
> > - if (cfg.stream() == &data->stream_)
> > + if (cfg.stream() == &data->mainPathStream_)
> > resizer = "rkisp1_resizer_mainpath";
> > else
> > return -EINVAL;
> > @@ -972,7 +972,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
> > int ret;
> >
> > std::unique_ptr<RkISP1CameraData> data =
> > - std::make_unique<RkISP1CameraData>(this, video_);
> > + std::make_unique<RkISP1CameraData>(this, mainPathVideo_);
> >
> > ControlInfoMap::Map ctrls;
> > ctrls.emplace(std::piecewise_construct,
> > @@ -993,7 +993,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
> > if (ret)
> > return ret;
> >
> > - std::set<Stream *> streams{ &data->stream_ };
> > + std::set<Stream *> streams{ &data->mainPathStream_ };
> > std::shared_ptr<Camera> camera =
> > Camera::create(this, data->sensor_->id(), streams);
> > registerCamera(std::move(camera), std::move(data));
> > @@ -1023,13 +1023,13 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
> > if (isp_->open() < 0)
> > return false;
> >
> > - resizer_ = V4L2Subdevice::fromEntityName(media_, "rkisp1_resizer_mainpath");
> > - if (resizer_->open() < 0)
> > + mainPathResizer_ = V4L2Subdevice::fromEntityName(media_, "rkisp1_resizer_mainpath");
> > + if (mainPathResizer_->open() < 0)
> > return false;
> >
> > /* Locate and open the capture video node. */
> > - video_ = V4L2VideoDevice::fromEntityName(media_, "rkisp1_mainpath");
> > - if (video_->open() < 0)
> > + mainPathVideo_ = V4L2VideoDevice::fromEntityName(media_, "rkisp1_mainpath");
> > + if (mainPathVideo_->open() < 0)
> > return false;
> >
> > stat_ = V4L2VideoDevice::fromEntityName(media_, "rkisp1_stats");
> > @@ -1040,7 +1040,7 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
> > if (param_->open() < 0)
> > return false;
> >
> > - video_->bufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady);
> > + mainPathVideo_->bufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady);
> > stat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statReady);
> > param_->bufferReady.connect(this, &PipelineHandlerRkISP1::paramReady);
> >
>
> --
> Regards,
>
> Laurent Pinchart
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list