[libcamera-devel] [PATCH v3 11/13] pipeline: rkisp1: Fix stream size validation

[Kieran Bingham]

Quoting Laurent Pinchart (2022-11-23 11:06:09)
> Hi Kieran,
> 
> On Wed, Nov 23, 2022 at 09:56:06AM +0000, Kieran Bingham wrote:
> > Quoting Laurent Pinchart (2022-10-30 17:24:49)
> > > On Fri, Oct 28, 2022 at 11:11:06PM +0100, Kieran Bingham wrote:
> > > > Quoting Laurent Pinchart via libcamera-devel (2022-10-24 01:03:54)
> > > > > Unlike RkISP1Path::generateConfiguration(), the validate() function
> > > > > doesn't take the camera sensor resolution into account but only
> > > > > considers the absolute minimum and maximum sizes support by the ISP to
> > > > > validate the stream size. Fix it by using the same logic as when
> > > > > generating the configuration.
> > > > > 
> > > > > Instead of passing the sensor resolution to the validate() function,
> > > > > pass the CameraSensor pointer to prepare for subsequent changes that
> > > > > will require access to more camera sensor data. While at it, update the
> > > > > generateConfiguration() function similarly for the same reason.
> > > > 
> > > > I have quite the surprising curveball on this patch.
> > > > 
> > > > While the validation seems to aim to restrict sizes to the capabilities
> > > > of the ISP and the Sensor size, it seems to miss something.
> > > > 
> > > > I've hooked up an Arducam 16MP to test this - and the set up fails
> > > > because the sensor size selected is larger than the maximum input size
> > > > of the ISP.
> > > > 
> > > > Can this somehow take the input limtiations of the ISP into account when
> > > > selecting and filtering sensor sizes easily ?
> > > 
> > > Is this a regression ?
> > 
> > I wasn't able to test this camera before on this platform. So it's not a
> > regression for me, it's just never worked...
> > 
> > But ... if we're "fixing the validation of the stream size", I'd call
> > this a bug report that this patch doesn't seem to account for the ISP
> > limitations when doing so.
> 
> No disagreement there :-) What I'm wondering is if it has to be fixed in
> here, or could be addressed on top. And of course if someone with access
> to a 16MP camera module could do it, that would be even better ;-)

It can be done on top.
--
Kieran