[libcamera-devel] [PATCH 05/13] libcamera: pipeline: rkisp1: Create RkISP1Frames from camera data

[Niklas Söderlund]

Hi Jacopo,

On 2020-08-20 10:42:27 +0200, Jacopo Mondi wrote:
> Hi Niklas,
> 
> On Thu, Aug 13, 2020 at 02:52:38AM +0200, Niklas Söderlund wrote:
> > Create RkISP1Frames from camera data instead of picking information out
> > form it. This is done to prepare for multi stream support where more
> > information from the camera data will be needed.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > ---
> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 13 +++++++------
> >  1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index a1cda12d244f2d97..9d5a52d352aefabc 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -52,6 +52,7 @@ static const std::array<PixelFormat, 7> RKISP1_RSZ_MP_FORMATS{
> >
> >  class PipelineHandlerRkISP1;
> >  class RkISP1ActionQueueBuffers;
> > +class RkISP1CameraData;
> >
> >  enum RkISP1ActionType {
> >  	SetSensor,
> > @@ -77,7 +78,7 @@ class RkISP1Frames
> >  public:
> >  	RkISP1Frames(PipelineHandler *pipe);
> >
> > -	RkISP1FrameInfo *create(unsigned int frame, Request *request, Stream *stream);
> > +	RkISP1FrameInfo *create(const RkISP1CameraData *data, Request *request);
> 
> Should data be a const & ?

It could but camera data is a pointer in all other places and I see no 
harm in keeping it the same here.

> 
> This apart, the patch looks good
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> 
> Thanks
>   j
> 
> >  	int destroy(unsigned int frame);
> >  	void clear();
> >
> > @@ -242,8 +243,10 @@ RkISP1Frames::RkISP1Frames(PipelineHandler *pipe)
> >  {
> >  }
> >
> > -RkISP1FrameInfo *RkISP1Frames::create(unsigned int frame, Request *request, Stream *stream)
> > +RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *request)
> >  {
> > +	unsigned int frame = data->frame_;
> > +
> >  	if (pipe_->availableParamBuffers_.empty()) {
> >  		LOG(RkISP1, Error) << "Parameters buffer underrun";
> >  		return nullptr;
> > @@ -256,7 +259,7 @@ RkISP1FrameInfo *RkISP1Frames::create(unsigned int frame, Request *request, Stre
> >  	}
> >  	FrameBuffer *statBuffer = pipe_->availableStatBuffers_.front();
> >
> > -	FrameBuffer *videoBuffer = request->findBuffer(stream);
> > +	FrameBuffer *videoBuffer = request->findBuffer(&data->stream_);
> >  	if (!videoBuffer) {
> >  		LOG(RkISP1, Error)
> >  			<< "Attempt to queue request with invalid stream";
> > @@ -880,10 +883,8 @@ void PipelineHandlerRkISP1::stop(Camera *camera)
> >  int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)
> >  {
> >  	RkISP1CameraData *data = cameraData(camera);
> > -	Stream *stream = &data->stream_;
> >
> > -	RkISP1FrameInfo *info = data->frameInfo_.create(data->frame_, request,
> > -							stream);
> > +	RkISP1FrameInfo *info = data->frameInfo_.create(data, request);
> >  	if (!info)
> >  		return -ENOENT;
> >
> > --
> > 2.28.0
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund