[libcamera-devel] [PATCH 15/15] libcamera: ipu3: Pass cio2 fourcc to ImgUDevice::configure()

[Jacopo Mondi]

Hi Niklas,

On Wed, Jul 01, 2020 at 06:56:41PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2020-07-01 14:30:36 +0200, Jacopo Mondi wrote:
> > Only the CIO2 4cc code is required for ImgUDevice::configure(), as the
> > input frame sizes are now reported through the ImgUDevice::Pipe
> > structure.
> >
> > Add the CIO2 pixel format to ImguDevice::Pipe instead of passing the
> > whole CIO2 V4L2DeviceFormat to ImgUDevice::configure().
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  src/libcamera/pipeline/ipu3/imgu.cpp | 19 ++++++++++++++-----
> >  src/libcamera/pipeline/ipu3/imgu.h   |  3 ++-
> >  src/libcamera/pipeline/ipu3/ipu3.cpp |  3 ++-
> >  3 files changed, 18 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp
> > index f580c1d2ff6c..59e63644d22f 100644
> > --- a/src/libcamera/pipeline/ipu3/imgu.cpp
> > +++ b/src/libcamera/pipeline/ipu3/imgu.cpp
> > @@ -26,11 +26,16 @@ LOG_DECLARE_CATEGORY(IPU3)
> >   * The ImgU unit processes images through several components, which have
> >   * to be properly configured inspecting the input image size and the desired
> >   * output sizes. This structure collects the ImgU input configuration and the
> > - * requested main output and viewfinder configurations.
> > + * requested main output and viewfinder configurations, along with the
> > + * pixel format to be applied to the ImgU input, which is the same as the
> > + * one used by the CIO2 unit.
> >   *
> >   * \var Pipe::input
> >   * \brief The input image size
> >   *
> > + * \var Pipe::cio2Fourcc
> > + * \brief The CIO2 pixel format to be applied to ImgU input
> > + *
> >   * \var Pipe::output
> >   * \brief The requested main output size
> >   *
> > @@ -96,17 +101,21 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index)
> >  /**
> >   * \brief Configure the ImgU unit input
> >   * \param[in] pipe The ImgU pipe configuration
> > - * \param[in] inputFormat The format to be applied to ImgU input
> >   * \return 0 on success or a negative error code otherwise
> >   */
> > -int ImgUDevice::configure(struct Pipe *pipe, V4L2DeviceFormat *inputFormat)
> > +int ImgUDevice::configure(struct Pipe *pipe)
> >  {
> >  	/* Configure the ImgU input video device with the requested sizes. */
> > -	int ret = input_->setFormat(inputFormat);
> > +	V4L2DeviceFormat inputFormat = {
> > +		.fourcc = pipe->cio2Fourcc,
> > +		.size = pipe->input,
> > +		.planesCount = 1,
> > +	};
> > +	int ret = input_->setFormat(&inputFormat);
> >  	if (ret)
> >  		return ret;
> >
> > -	LOG(IPU3, Debug) << "ImgU input format = " << inputFormat->toString();
> > +	LOG(IPU3, Debug) << "ImgU input format = " << inputFormat.toString();
> >
> >  	/*
> >  	 * \todo The IPU3 driver implementation shall be changed to use the
> > diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h
> > index 45c5cc1ce8e6..b0ed228756c7 100644
> > --- a/src/libcamera/pipeline/ipu3/imgu.h
> > +++ b/src/libcamera/pipeline/ipu3/imgu.h
> > @@ -25,13 +25,14 @@ class ImgUDevice
> >  public:
> >  	struct Pipe {
> >  		Size input;
> > +		V4L2PixelFormat cio2Fourcc;
>
> Would it make sens to name this inputFourcc ?
>

To pair it with "Size input" ? I really think so, yes!

> >  		Size output;
> >  		Size viewfinder;
> >  	};
> >
> >  	int init(MediaDevice *media, unsigned int index);
> >
> > -	int configure(struct Pipe *pipe, V4L2DeviceFormat *inputFormat);
> > +	int configure(struct Pipe *pipe);
> >
> >  	int configureOutput(const StreamConfiguration &cfg,
> >  			    V4L2DeviceFormat *outputFormat)
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index 317e78155ebb..79ec492341b2 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -410,6 +410,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> >  	ImgUDevice::Pipe imguPipe{};
> >  	imguPipe.input.width = cio2Format.size.width;
> >  	imguPipe.input.height = cio2Format.size.height;
> > +	imguPipe.cio2Fourcc = cio2Format.fourcc;
> >
> >  	for (unsigned int i = 0; i < config->size(); ++i) {
> >  		StreamConfiguration &cfg = (*config)[i];
> > @@ -459,7 +460,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> >  	 * Configure the ImgU with the collected pipe configuration and the
> >  	 * CIO2 unit format.
> >  	 */
> > -	ret = imgu->configure(&imguPipe, &cio2Format);
> > +	ret = imgu->configure(&imguPipe);
> >  	if (ret)
> >  		return ret;
> >
> > --
> > 2.27.0
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards,
> Niklas Söderlund