[PATCH] pipeline: rpi: Avoid duplicating size range for the same pixel format

[Jacopo Mondi]

Hi David

On Thu, Apr 25, 2024 at 12:37:48PM +0100, Naushir Patuck wrote:
> Hi David,
>
> Thank you for this patch.
>
> On Thu, 25 Apr 2024 at 11:52, David Plowman
> <david.plowman at raspberrypi.com> wrote:
> >
> > Some V4L2 formats translate to the same pixel format, e.g. YU12 and
> > YM12 both produce YUV420. In this case our ISP driver advertises the
> > same size range for both, but we must not record the same thing twice
> > for the same pixel format (which will cause a failure later on).

Am I wrong or this mostly (only) apply to contiguous and non-contiguous
format versions ?

> >
> > Instead, ignore the V4l2 format if the pixel format has already been
> > seen.
> >
> > Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
>
> Reviewed-by: Naushir Patuck <naush at raspberrypi.com>

>
> > ---
> >  src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > index 7e420b3f..0034e06d 100644
> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > @@ -474,7 +474,11 @@ PipelineHandlerBase::generateConfiguration(Camera *camera, Span<const StreamRole
> >                          */
> >                         for (const auto &format : fmts) {
> >                                 PixelFormat pf = format.first.toPixelFormat();
> > -                               if (pf.isValid()) {
> > +                               /*
> > +                                * Some V4L2 formats translate to the same pixel format (e.g. YU12, YM12
> > +                                * both give YUV420). We must avoid duplicating the range in this case.
> > +                                */
> > +                               if (pf.isValid() && deviceFormats.find(pf) == deviceFormats.end()) {

Or
                                    if (pf.isValid() && !deviceFormats.count(pf)) {


The change is anyway good!
Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

> >                                         const SizeRange &ispSizes = format.second[0];
> >                                         deviceFormats[pf].emplace_back(ispSizes.min, sensorSize,
> >                                                                        ispSizes.hStep, ispSizes.vStep);
> > --
> > 2.39.2
> >