[libcamera-devel] [PATCH v2 00/13] libcamera: ipu3: Refactoring of ImgU

[Niklas Söderlund]

Hi Tomasz,

On 2020-06-29 11:55:47 +0200, Tomasz Figa wrote:
> Hi Niklas and everyone,
> 
> On Sun, Jun 28, 2020 at 2:15 AM Niklas Söderlund
> <niklas.soderlund at ragnatech.se> wrote:
> >
> > Hi,
> >
> > This series do some cleanup of the IPU3 pipeline. It removes dead or
> > almost dead code, replacing it with something more coherent to what is
> > done elsewhere in the pipeline. It breaks out the ImgU to a separate cpp
> > and h file.
> >
> > It reworks a bit how the streams are identified and handled inside the
> > pipeline. When the pipeline was first developed only the output and
> > viewfinder streams where available, now we have a RAW stream in addition
> > to this. Some of the abstraction around streams and which hardware
> > resource they are backed by made sens in the earlier context but not so
> > much once RAW is added to the mix. The biggest difference in this rework
> > is that the ImgUDevice gets explicit functions to configure each of its
> > sink instead of having a generic one which depends which pointer is
> > passed to it. This makes reading code where RAW and ImgU streams are
> > mixed much nicer IMHO.
> 
> CIO2 is quite a generic capture interface, which could be handled by
> some platform-independent code. Have we considered splitting the
> pipeline into a generic capture part and platform-specific processing
> part? I think it could be useful on its own, as the capture part of
> all the SoCs could be handled by generic code, but I also suspect that
> the ability to split the pipeline into multiple smaller pipelines and
> then join them together could be very useful when dealing with many
> SoC generations, since vendors quite often replace some parts of the
> pipeline with new hardware, while keeping the others unchanged.
> 
> What do you think?

I think it is a good idea and something that have been lurking in the 
back of my head for some time. I'm even entertaining the thought if we 
could levy the simple-pipeline handler here somehow. I still have to dig 
in the code to see what is possible and what is not, and what is a good 
idea and what is not :-)

Right now tho I think the most important area to try and help out 
pipeline handlers is around the Request object and how internal buffers 
(RAW, Statistics and Parameters) are attached to it when for example the 
RAW buffer could come from the application or from an internal pool.  
This will be a common problem for all pipelines and it would be nice if 
the core could help out here.

> 
> Best regards,
> Tomasz
> 
> >
> > Lastly some assumptions that buffers must be allocated at video nodes
> > that are not involved in the capture session are being challenged. This
> > was true a year ago but not any more it seems. Chancing this simplifies
> > the driver enormously and saves on memory that otherwise would be
> > wasted. I have really tried to force the end result to failed by
> > resetting the hardware between each test so that no video node
> > configuration from a previous sessions saves the day.
> >
> > I have for both sensors reset the hardware and then tested the following
> > capture combinations successfully. After the first capture session for
> > each one in the list that was done after a power cycle all the other
> > captures where tried in a semi random order and always succeeded.
> >
> >   cam -c 1 -s role=viewfinder -C
> >   cam -c 1 -s role=still  -C
> >   cam -c 1 -s role=still -s role=viewfinder -C
> >   cam -c 1 -s role=still -s role=viewfinder -s role=stillraw -C
> >   cam -c 1 -s role=stillraw -C
> >
> > Niklas Söderlund (13):
> >   libcamera: ipu3: Remove unused name_ filed from IPU3Stream
> >   libcamera: ipu3: Import instead of allocate statistic buffers
> >   libcamera: ipu3: Always import buffers for ImgU sinks
> >   libcamera: ipu3: Remove usage of IPU3CameraData from ImgUDevice
> >   libcamera: ipu3: imgu: Move the ImgUDevice class to separate files
> >   libcamera: ipu3: imgu: Do not cache index
> >   libcamera: ipu3: imgu: Mark things that are internal as private
> >   libcamera: ipu3: imgu: Use specific functions to configure each sink
> >   libcamera: ipu3: Do not duplicate data in IPU3Stream
> >   libcamera: ipu3: Remove the active flag from IPU3Stream
> >   libcamera: ipu3: Remove IPU3Stream
> >   libcamera: ipu3: imgu: Remove ImgUOutput
> >   libcamera: ipu3: imgu: Use unique_ptr for video and subdevices
> >
> >  src/libcamera/pipeline/ipu3/imgu.cpp    | 353 +++++++++++++++
> >  src/libcamera/pipeline/ipu3/imgu.h      |  87 ++++
> >  src/libcamera/pipeline/ipu3/ipu3.cpp    | 548 +++---------------------
> >  src/libcamera/pipeline/ipu3/meson.build |   1 +
> >  4 files changed, 505 insertions(+), 484 deletions(-)
> >  create mode 100644 src/libcamera/pipeline/ipu3/imgu.cpp
> >  create mode 100644 src/libcamera/pipeline/ipu3/imgu.h
> >
> > --
> > 2.27.0
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund