[libcamera-devel] [PATCH 2/5] libcamera: ipu3: Return video output default configuration

[Laurent Pinchart]

Hi Jacopo,

On Thu, Feb 21, 2019 at 05:21:16PM +0100, Jacopo Mondi wrote:
> On Thu, Feb 21, 2019 at 04:58:35PM +0100, Niklas Söderlund wrote:
> > On 2019-02-20 14:17:54 +0100, Jacopo Mondi wrote:
> >> Return default configuration for the output stream produced by the imgu
> >> 'output' video node.
> >>
> >> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> >> ---
> >>  src/libcamera/pipeline/ipu3/ipu3.cpp | 16 +++-------------
> >>  1 file changed, 3 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> >> index 9694d0ce51ab..9065073913a2 100644
> >> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> >> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> >> @@ -99,21 +99,11 @@ PipelineHandlerIPU3::streamConfiguration(Camera *camera,
> >>  {
> >>  	IPU3CameraData *data = cameraData(camera);
> >>  	std::map<Stream *, StreamConfiguration> configs;
> >> -	V4L2SubdeviceFormat format = {};
> >> -
> >> -	/*
> >> -	 * FIXME: As of now, return the image format reported by the sensor.
> >> -	 * In future good defaults should be provided for each stream.
> >> -	 */
> >> -	if (data->sensor_->getFormat(0, &format)) {
> >> -		LOG(IPU3, Error) << "Failed to create stream configurations";
> >> -		return configs;
> >> -	}
> >>
> >>  	StreamConfiguration config = {};
> >> -	config.width = format.width;
> >> -	config.height = format.height;
> >> -	config.pixelFormat = V4L2_PIX_FMT_IPU3_SGRBG10;
> >> +	config.width = 2560;
> >> +	config.height = 1920;
> >> +	config.pixelFormat = V4L2_PIX_FMT_NV12;
> >
> > This is a good change and trust that the format you selected are the
> > correct one so I have not verified it by reading or testing on the IPU3.
> >
> > Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> >
> > I'm just curious why the pixelformat changes, did something change in
> > the upstream driver or is this in preparation of some later change?
> 
> We're preparing to provide frames to the application from the IMGU not
> from the CIO2 unit. The output format of the two is different.

But until we get that, this patch breaks capture with the IPU3, doesn't
it ? I think it should be squashed with the patch that will add ImgU
support, not get merged separately.

> >>  	config.bufferCount = 4;
> >>
> >>  	configs[&data->stream_] = config;

-- 
Regards,

Laurent Pinchart