[libcamera-devel] [RFC] libcamera: ipu3: Always use sensor full frame size

Fri Sep 18 16:05:42 CEST 2020

Hi Jacopo,

On Fri, Sep 18, 2020 at 12:05:51PM +0200, Jacopo Mondi wrote:
> On Fri, Sep 18, 2020 at 05:49:13AM +0300, Laurent Pinchart wrote:
> > On Fri, Sep 11, 2020 at 08:48:52PM +0200, Tomasz Figa wrote:
> > > On Thu, Sep 10, 2020 at 12:25 PM Niklas Söderlund wrote:
> > > > On 2020-09-09 15:10:14 +0200, Jacopo Mondi wrote:
> > > > > When calculating the pipeline configuration for the IPU3 platform,
> > > > > libcamera tries to be smart and select the smaller sensor frame
> > > > > resolution larger enough to accommodate the stream sizes
> > > > > requested by the application.
> > > > >
> > > > > While this seems to make a lot of sense, in practice optimizing the
> > > > > selected sensor resolution makes the pipeline configuration calculation
> > > > > process fail in multiple occasions, or results in stalls in the capture
> > > > > process.
> > > > >
> > > > > As a trivial example, capturing with cam with the following command
> > > > > line result in a stall:
> > > > > $ cam -swidth=1280,height=720 -swidth=640,height=480 -c1 -C
> > > > >
> > > > > Likewise, the Android HAL supported format enumeration fails in
> > > > > reporting smaller resolutions as supported, when used with the OV5670
> > > > > sensor.
> > > > >
> > > > > 320x240:
> > > > > DEBUG IPU3 ipu3.cpp:192 CIO2 configuration: 648x486-SGRBG10_IPU3
> > > > > ERROR IPU3 imgu.cpp:408 Failed to calculate pipe configuration
> > > > > ERROR IPU3 ipu3.cpp:299 Failed to calculate pipe configuration: unsupported resolutions.
> > > > >
> > > > > 640x480:
> > > > > DEBUG IPU3 ipu3.cpp:192 CIO2 configuration: 648x486-SGRBG10_IPU3
> > > > > ERROR IPU3 imgu.cpp:408 Failed to calculate pipe configuration
> > > > > ERROR IPU3 ipu3.cpp:299 Failed to calculate pipe configuration: unsupported resolutions.
> > > > >
> > > > > Furthermore the reference .xml files used for the IPU3 camera
> > > > > configuration on the ChromeOS platform restrict the number of sensor
> > > > > resolution to be used for the OV5670 sensor to 2 from the 6 supported by
> > > > > the driver [1].
> > > > >
> > > > > The selection criteria of the correct CIO2 mode are not specified, and
> > > > > for the time being, always use the sensor maximum resolution at the
> > > > > expense of frame rate and bus bandwidth allows the pipeline to successfully
> > > > > support smaller modes for the OV5670 sensor and solves pipeline stalls
> > > > > when capturing with both sensors.
> > > > >
> > > > > [1] See the <sensor_modes> enumeration in:
> > > > > https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/master/baseboard-poppy/media-libs/cros-camera-hal-configs-poppy/files/gcss/graph_settings_ov5670.xml
> > > > >
> > > > > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > > >
> > > > I love how the IPU3 configuration depends on a xml file ;-)
> > >
> > > However it sounds, I believe some static configuration is unavoidable,
> > > especially for software which is expected to be used in production and
> > > maintain stable operating conditions. Linux has a lot of configuration
> > > as well, either compile-time or runtime (sysfs, proc) for the same
> > > reason.
> > >
> > > As we noticed in another thread, about Android stream mapping, there
> > > are multiple configurations existing to achieve the same final
> > > parameters requested, even though the end result might not be exactly
> > > the same. For example, scaling in one part of the pipeline or another
> > > could have different effect on quality, with the choice between one or
> > > another depending on business decisions of a particular integrator.
> > >
> > > So as much as we want to avoid configuration files, I think that we
> > > actually have to start thinking of how to make it possible to use such
> > > configuration if one wants.
> >
> > Regardless of how we will allow overriding pipeline configuration as
> > part of the integration process, we need a heuristic that works
> > reasonably well. This patch, which I think is fine as a short term
> > workaround, outlines shortcomings in the configuration of the ImgU. As
> > Jacopo figured out, either we're not using the python code provided by
> > Intel correctly, or the code fails in some cases. This is something that
> > has to be sorted out before the ImgU driver can get out of staging.
> 
> For the records:

Thanks for the summary, that will be helpful when we'll get back to this
in the future, and try to recall what the problem was.

> 1) This issue is not much releated to the python tool configuration,
> (which expects input sizes from CIO2 to be pre-calculated) but more
> about the sensor size selection criteria which are not specified
> 
> 2) The python script gives different results than the ones recorded in
> the xml file and I've opened an un-resolved issue on github in June
> for this:
> https://github.com/intel/intel-ipu3-pipecfg/issues/1
> 
> 3) Of course there can be glitches in our pipeline configuration
> routine implementation, but the issue of stalling the pipeline when
> using certain resolution seems not related to the ImgU configuration
> parameters calculation, or at least, our implementation gives the same
> results as the python script, but using those parameters result in a
> pipeline stall.
> 
> In example, as reported by the commit message, this command results in
> a stall during capture operations (no frames are produced at the ImgU
> outputs):
> $ cam -swidth=1280,height=720 -swidth=640,height=480 -c1 -C
> 
> If I inspect our pipeline configuration routine results I get:
> 
> DEBUG IPU3 imgu.cpp:386 Calculating pipe configuration for:
> DEBUG IPU3 imgu.cpp:387 input: 2112x1188
> DEBUG IPU3 imgu.cpp:388 main: 1280x720
> DEBUG IPU3 imgu.cpp:389 vf: 640x480
> DEBUG IPU3 imgu.cpp:425 Computed pipe configuration:
> DEBUG IPU3 imgu.cpp:426 IF: 2072x1192
> DEBUG IPU3 imgu.cpp:427 BDS: 1792x1027
> DEBUG IPU3 imgu.cpp:428 GDC: 1280x960
> 
> The same parameters fed to the python script give the same results for
> the IF, BDS and GDC rectangles:
> 
> $ python3 pipe_config.py input=2112x1188 main=1280x720 vf=640x480
> -------- The selected pipe configuration: --------------
> output_res_if:2072x1192
> output_res_bds:1792x1027
> output_res_gdc:1280x960
> 
> But, as reported, this does not work. If I use the sensor full frame size as
> CIO2 input (4224x3136 for OV13858) I get frames and the pipeline works
> as expected
> 
> DEBUG IPU3 imgu.cpp:386 Calculating pipe configuration for:
> DEBUG IPU3 imgu.cpp:387 input: 4224x3136
> DEBUG IPU3 imgu.cpp:388 main: 1280x720
> DEBUG IPU3 imgu.cpp:389 vf: 640x480
> DEBUG IPU3 imgu.cpp:425 Computed pipe configuration:
> DEBUG IPU3 imgu.cpp:426 IF: 4220x3120
> DEBUG IPU3 imgu.cpp:427 BDS: 1688x1248
> DEBUG IPU3 imgu.cpp:428 GDC: 1280x960
> 
> Which again matches the python script results
> 
>  python3 pipe_config.py input=4224x3136 main=1280x720 vf=640x480
> -------- The selected pipe configuration: --------------
> output_res_if:4220x3120
> output_res_bds:1688x1248
> output_res_gdc:1280x960

I'd consider this as a driver bug, as the driver should reject invalid
configurations, not stall. Of course, when the driver will be fixed, we
still won't have a functional system, because the configuration we
calculate for the ImgU is invalid :-)

-- 
Regards,

Laurent Pinchart