[libcamera-devel] [PATCH 13/15] libcamera: ipu3: Collect ImgU pipe configuration
Jacopo Mondi
jacopo at jmondi.org
Thu Jul 2 09:36:04 CEST 2020
Hi Niklas,
On Wed, Jul 01, 2020 at 06:54:24PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2020-07-01 14:30:34 +0200, Jacopo Mondi wrote:
> > Break the stream assignement and ImgU configuration performed at
> > IPU3PipelineHandler::configure() time in two parts, to collect the
> > ImgU pipe configuration.
> >
> > The first pass collects the desired ImgU pipe configuration and assigns
> > streams to configurations. The second pass, which is performed after the
> > ImgU pipe has been configured, programs the computed sizes to the ImgU
> > output video devices.
> >
> > Use the collected pipe configuration in ImgUDevice::configureInput() as
> > it will be used to compute the pipe configuration parameters.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> > src/libcamera/pipeline/ipu3/imgu.cpp | 11 ++--
> > src/libcamera/pipeline/ipu3/imgu.h | 2 +-
> > src/libcamera/pipeline/ipu3/ipu3.cpp | 80 +++++++++++++++++++---------
> > 3 files changed, 60 insertions(+), 33 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp
> > index 7e9047cc8dc1..49201157eb07 100644
> > --- a/src/libcamera/pipeline/ipu3/imgu.cpp
> > +++ b/src/libcamera/pipeline/ipu3/imgu.cpp
> > @@ -95,12 +95,11 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index)
> >
> > /**
> > * \brief Configure the ImgU unit input
> > - * \param[in] size The ImgU input frame size
> > + * \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::configureInput(const Size &size,
> > - V4L2DeviceFormat *inputFormat)
> > +int ImgUDevice::configureInput(struct Pipe *pipe, V4L2DeviceFormat *inputFormat)
> > {
> > /* Configure the ImgU input video device with the requested sizes. */
> > int ret = input_->setFormat(inputFormat);
> > @@ -122,8 +121,8 @@ int ImgUDevice::configureInput(const Size &size,
> > Rectangle rect = {
> > .x = 0,
> > .y = 0,
> > - .width = inputFormat->size.width,
> > - .height = inputFormat->size.height,
> > + .width = pipe->input.width,
> > + .height = pipe->input.height,
> > };
> > ret = imgu_->setSelection(PAD_INPUT, V4L2_SEL_TGT_CROP, &rect);
> > if (ret)
> > @@ -138,7 +137,7 @@ int ImgUDevice::configureInput(const Size &size,
> >
> > V4L2SubdeviceFormat imguFormat = {};
> > imguFormat.mbus_code = MEDIA_BUS_FMT_FIXED;
> > - imguFormat.size = size;
> > + imguFormat.size = pipe->input;
> >
> > ret = imgu_->setFormat(PAD_INPUT, &imguFormat);
> > if (ret)
> > diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h
> > index 8fb271fb8350..c0353955ea43 100644
> > --- a/src/libcamera/pipeline/ipu3/imgu.h
> > +++ b/src/libcamera/pipeline/ipu3/imgu.h
> > @@ -31,7 +31,7 @@ public:
> >
> > int init(MediaDevice *media, unsigned int index);
> >
> > - int configureInput(const Size &size, V4L2DeviceFormat *inputFormat);
> > + int configureInput(struct Pipe *pipe, V4L2DeviceFormat *inputFormat);
> >
> > int configureOutput(const StreamConfiguration &cfg,
> > V4L2DeviceFormat *outputFormat)
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index 16bda97a3cc3..9499c127ef79 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -396,32 +396,26 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> > outCount++;
> > }
> >
> > - /*
> > - * Configure the CIO2 unit with the format computed during validation
> > - * and apply the same format to the ImgU input.
> > - */
> > + /* Configure the CIO2 unit with the format computed during validation. */
> > const Size &sensorSize = config->cio2Format().size;
> > V4L2DeviceFormat cio2Format = {};
> > ret = data->cio2_.configure(sensorSize, &cio2Format);
> > if (ret)
> > return ret;
> >
> > - ret = imgu->configureInput(sensorSize, &cio2Format);
> > - if (ret)
> > - return ret;
> > + /*
> > + * Collect the input, output and viewfinder sizes to configure the
> > + * ImgU pipe and assign streams to configurations while iterating them.
> > + */
> > + ImgUDevice::Pipe imguPipe{};
> > + imguPipe.input.width = cio2Format.size.width;
> > + imguPipe.input.height = cio2Format.size.height;
>
> Would not 'imguPipe.input = cio2Format.size' work here and below? With
> this fixed,
Ah yes!
I started wit ImgUPipe having Rectangles, then I moved to use Size but
forgot to update this.
Thanks for spotting
>
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
>
> >
> > - /* Apply the format to the configured streams output devices. */
> > - bool outActive = false;
> > - bool vfActive = false;
> > for (unsigned int i = 0; i < config->size(); ++i) {
> > StreamConfiguration &cfg = (*config)[i];
> > const PixelFormatInfo &info =
> > PixelFormatInfo::info(cfg.pixelFormat);
> > if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {
> > - /*
> > - * The RAW stream is configured as part of the CIO2 and
> > - * no configuration is needed for the ImgU.
> > - */
> > cfg.setStream(&data->rawStream_);
> > cfg.stride = cio2Format.planes[0].bpl;
> >
> > @@ -437,13 +431,9 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> > * requested.
> > */
> > if (cfg.size == maxOut && outCount > 1) {
> > - ret = imgu->configureOutput(cfg, &outputFormat);
> > - if (ret)
> > - return ret;
> > -
> > - cfg.stride = outputFormat.planes[0].bpl;
> > cfg.setStream(&data->outStream_);
> > - outActive = true;
> > + imguPipe.output.width = cfg.size.width;
> > + imguPipe.output.height = cfg.size.height;
> >
> > LOG(IPU3, Debug) << "Assigned " << cfg.toString()
> > << " to the main output";
> > @@ -457,18 +447,56 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> > * the smaller one to reduce the cropping required (if any) on
> > * the main output.
> > */
> > - ret = imgu->configureViewfinder(cfg, &outputFormat);
> > - if (ret)
> > - return ret;
> > -
> > - cfg.stride = outputFormat.planes[0].bpl;
> > cfg.setStream(&data->vfStream_);
> > - vfActive = true;
> > + imguPipe.viewfinder.width = cfg.size.width;
> > + imguPipe.viewfinder.height = cfg.size.height;
> >
> > LOG(IPU3, Debug) << "Assigned " << cfg.toString()
> > << " to the viewfinder output";
> > }
> >
> > + /*
> > + * Configure the ImgU with the collected pipe configuration and the
> > + * CIO2 unit format.
> > + */
> > + ret = imgu->configureInput(&imguPipe, &cio2Format);
> > + if (ret)
> > + return ret;
> > +
> > + /* Apply the format to the ImgU output devices. */
> > + bool outActive = false;
> > + bool vfActive = false;
> > + for (unsigned int i = 0; i < config->size(); ++i) {
> > + StreamConfiguration &cfg = (*config)[i];
> > + Stream *stream = cfg.stream();
> > +
> > + if (stream == &data->rawStream_) {
> > + /*
> > + * The RAW stream is configured as part of the CIO2 and
> > + * no configuration is needed for the ImgU.
> > + */
> > + continue;
> > + }
> > +
> > + if (stream == &data->outStream_) {
> > + ret = imgu->configureOutput(cfg, &outputFormat);
> > + if (ret)
> > + return ret;
> > +
> > + outActive = true;
> > + cfg.stride = outputFormat.planes[0].bpl;
> > + }
> > +
> > + if (stream == &data->vfStream_) {
> > + ret = imgu->configureViewfinder(cfg, &outputFormat);
> > + if (ret)
> > + return ret;
> > +
> > + vfActive = true;
> > + cfg.stride = outputFormat.planes[0].bpl;
> > + }
> > + }
> > +
> > /*
> > * As we need to set format also on the non-active streams, use
> > * the configuration of the active one for that purpose (there should
> > --
> > 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