[PATCH v1 5/7] pipeline: rpi: Track which ISP output is configured for a stream

[Laurent Pinchart]

Hello,

On Wed, Aug 28, 2024 at 11:03:34AM +0100, Naushir Patuck wrote:
> On Wed, 28 Aug 2024 at 10:50, Jacopo Mondi wrote:
> > On Thu, Aug 08, 2024 at 11:23:44AM GMT, Naushir Patuck wrote:
> > > Add a ispIndex field to CropParams that is used to track
> > > which ISP output (0/1) will be used for a given stream during
> > > configuration.
> > >
> > > Tracking this information is required for an upcoming change where crop
> > > rectangles can be specified for each configured stream. Currently, the
> > > value is fixed to 0.
> >
> > Maybe I'm missing something, but isn't this the same value used to
> > index the cropParams_ map ?
> 
> No it's not the same, and horribly tricky :)
> 
> We have 2 indexes to track - the key of the cropParams_ maps tracks
> the index of the configured stream, and CropParams::ispCrop tracks the
> actual ISP output channel used for that stream.  Looking at the code
> in the pisp pipeline handler [1] might make things clearer.
> 
> [1] https://github.com/raspberrypi/libcamera/blob/08eb6758129788cb7e33cc1726aea40c672201b8/src/libcamera/pipeline/rpi/pisp/pisp.cpp#L1556
> 
> > > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> > > ---
> > >  src/libcamera/pipeline/rpi/common/pipeline_base.h | 6 ++++--
> > >  src/libcamera/pipeline/rpi/vc4/vc4.cpp            | 2 +-
> > >  2 files changed, 5 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h
> > > index 2bed905178bc..0ccfa4e82e86 100644
> > > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h
> > > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h
> > > @@ -135,8 +135,8 @@ public:
> > >       IPACameraSensorInfo sensorInfo_;
> > >
> > >       struct CropParams {
> > > -             CropParams(Rectangle ispCrop_, Size ispMinCropSize_)
> > > -                     : ispCrop(ispCrop_), ispMinCropSize(ispMinCropSize_)
> > > +             CropParams(Rectangle ispCrop_, Size ispMinCropSize_, unsigned int ispIndex_)
> > > +                     : ispCrop(ispCrop_), ispMinCropSize(ispMinCropSize_), ispIndex(ispIndex_)
> > >               {
> > >               }
> > >
> > > @@ -148,6 +148,8 @@ public:
> > >               Rectangle ispCrop;
> > >               /* Minimum crop size in ISP output pixels */
> > >               Size ispMinCropSize;
> > > +             /* Index of the ISP output channel for this crop */
> > > +             unsigned int ispIndex;
> > >       };
> > >
> > >       /* Mapping of CropParams keyed by the stream index in CameraConfiguration */
> > > diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> > > index d118252f02dd..ffc45653daf1 100644
> > > --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> > > +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> > > @@ -715,7 +715,7 @@ int Vc4CameraData::platformConfigure(const RPi::RPiCameraConfiguration *rpiConfi
> > >       cropParams_.clear();
> > >       cropParams_.emplace(std::piecewise_construct,
> > >                           std::forward_as_tuple(0),
> > > -                         std::forward_as_tuple(scaleIspCrop(ispCrop), testCrop.size()));
> > > +                         std::forward_as_tuple(scaleIspCrop(ispCrop), testCrop.size(), 0));
> > >
> > >       return 0;
> > >  }

-- 
Regards,

Laurent Pinchart