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

[Andrey Konovalov]

Hi Niklas, Kieran,

On 07.10.2020 16:04, Kieran Bingham wrote:
> Hi Niklas, Andrey,
> 
> On 07/10/2020 14:01, Niklas Söderlund wrote:
>> Hello Andrey,
>>
>> Thanks for your patch.
>>
>> On 2020-10-07 10:54:57 +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.
>>
>> I wonder if the check should not be moved to SimpleCameraData::init()
>> where the formats_ array is built. The array contains all supported
>> formats of the camera and excluding mplaner formats from it will make it
>> not show up at all for applications. Also validate() would Adjust if any
>> format is asked for that is not in the formats_ array.

Yes, this is a better option, thanks! I'll send the v2 shortly.

> That sounds pretty good too. If we go that route, I think it will need a
> highlighting '\todo: support mplane formats' so that it's clear/easy to
> find the code which is mysteriously removing supported formats from a
> device which is capable of using them ;-) (after we really support
> Multiplanar).

OK, will add the \todo.
BTW, the "mplane format" in this context means the one using non-contiguous
memory for frame buffers. E.g. 'NV16' I've tested is allowed, while 'NM16'
(aka V4L2_PIX_FMT_NV16M) would be excluded.

Thanks,
Andrey

> --
> Kieran
> 
> 
> 
>>
>>>
>>> Signed-off-by: Andrey Konovalov <andrey.konovalov at linaro.org>
>>> ---
>>>   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";
>>> +		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));
>>> -- 
>>> 2.17.1
>>>
>>> _______________________________________________
>>> libcamera-devel mailing list
>>> libcamera-devel at lists.libcamera.org
>>> https://lists.libcamera.org/listinfo/libcamera-devel
>>
>