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

[Jacopo Mondi]

Hi Niklas,

    I only have very small comments on the series, nice clean up !

For the whole series
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

On Sat, Jun 27, 2020 at 05:00:30AM +0200, Niklas Söderlund 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 stremas are
> mixed much nicer IMHO.
>
> 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
>
> I have also played a bit with qcam and it works as expected and the
> resulting normal och raw frames are generated and contains the expected
> data.
>
> 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: Align how CIO2 and ImgU are stored in CameraData
>   libcamera: ipu3: imgu: Remove ImgUOutput
>
>  src/libcamera/pipeline/ipu3/imgu.cpp    | 351 ++++++++++++++
>  src/libcamera/pipeline/ipu3/imgu.h      |  87 ++++
>  src/libcamera/pipeline/ipu3/ipu3.cpp    | 581 ++++--------------------
>  src/libcamera/pipeline/ipu3/meson.build |   1 +
>  4 files changed, 515 insertions(+), 505 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