[libcamera-devel] [PATCH v2] libcamera: pipeline: simple: enable mplane devices using contiguous memory

[Laurent Pinchart]

On Sat, Oct 10, 2020 at 01:40:28AM +0300, Laurent Pinchart wrote:
> Hi Andrey,
> 
> Thank you for the patch.
> 
> On Fri, Oct 09, 2020 at 02:17:39PM +0300, Andrey Konovalov wrote:
> > The current simple pipeline handler refuses to work with capture devices
> > which have V4L2_CAP_VIDEO_CAPTURE_MPLANE flag set in the device capabilities
> > field. This is too restrictive, as devices supporting the multi-planar API
> > can be using contiguous memory for semi-planar and planar formats, and this
> > would just work without any changes to libcamera.
> > 
> > Drop the guard against MPLANE devices, and replace it with the check of
> > the number of planes in the format the simple pipeline handler is going to
> > use for capture. This will let MPLANE devices which don't use non-contiguous
> > memory for frame buffers to work with the simple pipeline handler.
> > 
> > The following code in SimpleCameraData::init() filters out the pixel formats
> > libcamera doesn't support:
> > 
> > 	PixelFormat pixelFormat = videoFormat.first.toPixelFormat();
> > 	if (!pixelFormat)
> > 		continue;
> > 
> > So the check for the number of memory planes this patch adds would not
> > trigger until non-contiguous planar formats becomes supported in libcamera,
> > and video devices using these formats are enabled in the simple pipeline
> > handler. Then this check will remind one to review the simple pipeline
> > handler code.
> 
> Perfect :-)
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> 
> > Signed-off-by: Andrey Konovalov <andrey.konovalov at linaro.org>
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > ---
> >  Changes in v2:
> >  - The commit message extended to explain how the simple pipeline handler
> >    filters out pixel formats not supported in libcamera. No changes to
> >    the code vs v1.
> > 
> >  src/libcamera/pipeline/simple/simple.cpp | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > index 10223a9b..8dc23623 100644
> > --- a/src/libcamera/pipeline/simple/simple.cpp
> > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > @@ -592,6 +592,12 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
> >  	if (ret)
> >  		return ret;
> >  
> > +	if (captureFormat.planesCount != 1) {
> > +		LOG(SimplePipeline, Error)
> > +			<<  "Planar formats using non-contiguous memory not supported";

Actually, s/  / /

I'll fix when applying. May I recommend using the checkstyle.py script
as a post-commit or pre-commit git hook ? The hooks can be found in
utils/hook/, you can just copy the one you prefer to .git/hooks/

> > +		return -EINVAL;
> > +	}
> > +
> >  	if (captureFormat.fourcc != videoFormat ||
> >  	    captureFormat.size != pipeConfig.captureSize) {
> >  		LOG(SimplePipeline, Error)
> > @@ -845,12 +851,6 @@ V4L2VideoDevice *SimplePipelineHandler::video(const MediaEntity *entity)
> >  	if (video->open() < 0)
> >  		return nullptr;
> >  
> > -	if (video->caps().isMultiplanar()) {
> > -		LOG(SimplePipeline, Error)
> > -			<< "V4L2 multiplanar devices are not supported";
> > -		return nullptr;
> > -	}
> > -
> >  	video->bufferReady.connect(this, &SimplePipelineHandler::bufferReady);
> >  
> >  	auto element = videos_.emplace(entity, std::move(video));

-- 
Regards,

Laurent Pinchart