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

[Kieran Bingham]

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.


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).

--
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
> 

-- 
Regards
--
Kieran