[libcamera-devel] [PATCH 09/13] libcamera: ipu3: Do not duplicate data in IPU3Stream
Niklas Söderlund
niklas.soderlund at ragnatech.se
Sun Jun 28 00:46:30 CEST 2020
Hi Jacopo,
Thanks for your feedback.
On 2020-06-27 12:06:16 +0200, Jacopo Mondi wrote:
>
> On Sat, Jun 27, 2020 at 05:00:39AM +0200, Niklas Söderlund wrote:
> > Do not keep the duplicated ImgUDevice::ImgUOutput information in both
> > the stream and camera data classes. Remove it from the stream and only
> > access it from the camera data class.
> >
> > Which stream is which can instead be checked by comparing it to the
> > known streams in camera data. This match how streams are checked in
> > other parts of the code making the driver more coherent.
>
>
> not a driver :)
wops :-)
>
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > ---
> > src/libcamera/pipeline/ipu3/ipu3.cpp | 31 +++++++++++++++-------------
> > 1 file changed, 17 insertions(+), 14 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index ae7a01b81dacf498..1f320dc166767bab 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -35,13 +35,11 @@ class IPU3Stream : public Stream
> > {
> > public:
> > IPU3Stream()
> > - : active_(false), raw_(false), device_(nullptr)
> > + : active_(false)
> > {
> > }
> >
> > bool active_;
> > - bool raw_;
> > - ImgUDevice::ImgUOutput *device_;
> > };
> >
> > class IPU3CameraData : public CameraData
> > @@ -276,7 +274,7 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
> > const StreamConfiguration oldCfg = cfg;
> > const IPU3Stream *stream = streams_[i];
> >
> > - if (stream->raw_) {
> > + if (stream == &data_->rawStream_) {
> > cfg = cio2Configuration_;
> > } else {
> > bool scale = stream == &data_->vfStream_;
> > @@ -572,13 +570,16 @@ int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,
> > std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > {
> > IPU3CameraData *data = cameraData(camera);
> > - IPU3Stream *ipu3stream = static_cast<IPU3Stream *>(stream);
> > unsigned int count = stream->configuration().bufferCount;
> >
> > - if (ipu3stream->raw_)
> > + if (stream == &data->outStream_)
> > + return data->imgu_->output_.dev->exportBuffers(count, buffers);
> > + else if (stream == &data->vfStream_)
> > + return data->imgu_->viewfinder_.dev->exportBuffers(count, buffers);
> > + else if (stream == &data->rawStream_)
> > return data->cio2_.exportBuffers(count, buffers);
> >
> > - return ipu3stream->device_->dev->exportBuffers(count, buffers);
> > + return -EINVAL;
> > }
> >
> > /**
> > @@ -685,11 +686,16 @@ int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)
> > /* Queue all buffers from the request aimed for the ImgU. */
> > for (auto it : request->buffers()) {
> > IPU3Stream *stream = static_cast<IPU3Stream *>(it.first);
> > - if (stream->raw_)
> > - continue;
> > -
> > FrameBuffer *buffer = it.second;
> > - int ret = stream->device_->dev->queueBuffer(buffer);
> > + int ret;
> > +
> > + if (stream == &data->outStream_)
> > + ret = data->imgu_->output_.dev->queueBuffer(buffer);
> > + else if (stream == &data->vfStream_)
> > + ret = data->imgu_->viewfinder_.dev->queueBuffer(buffer);
> > + else
> > + continue;
> > +
>
> I would keep the previous code flow. Up to you
>
> if (stream == &data->rawStream_)
> continue;
>
> int ret = 0;
> if (stream == &data->outStream_)
> ret = data->imgu_->output_.dev->queueBuffer(buffer);
> else if (stream == &data->vfStream_)
> ret = data->imgu_->viewfinder_.dev->queueBuffer(buffer);
I have no strong preference so I will go with your suggestion in v2.
>
> > if (ret < 0)
> > error = ret;
> > }
> > @@ -801,9 +807,6 @@ int PipelineHandlerIPU3::registerCameras()
> > * second.
> > */
> > data->imgu_ = numCameras ? &imgu1_ : &imgu0_;
> > - data->outStream_.device_ = &data->imgu_->output_;
> > - data->vfStream_.device_ = &data->imgu_->viewfinder_;
> > - data->rawStream_.raw_ = true;
> >
> > /*
> > * Connect video devices' 'bufferReady' signals to their
> > --
> > 2.27.0
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list