[libcamera-devel] [PATCH RFC 2/7] android: yuv: loop over each plane for size check

[Mattijs Korpershoek]

Hi Jacopo,

Thank you for your review.

On ven., sept. 22, 2023 at 12:56, Jacopo Mondi <jacopo.mondi at ideasonboard.com> wrote:

> Hi Mattijs
>
> On Fri, Sep 15, 2023 at 09:57:26AM +0200, Mattijs Korpershoek via libcamera-devel wrote:
>> PostProcessorYuv assumption is that only formats::NV12 is supported
>> for source and destination pixelFormat.
>>
>> Because of this, we don't loop on each plane when size checking, but we
>> always assume a 2 plane buffer.
>>
>> To prepare for adding more YUV formats such as YUYV, loop over the
>> planes instead of using an if.
>>
>> No functional change, besides the logs only printing the first faulty
>> plane length.
>>
>> Signed-off-by: Mattijs Korpershoek <mkorpershoek at baylibre.com>
>> ---
>>  src/android/yuv/post_processor_yuv.cpp | 40 +++++++++++++++++-----------------
>>  1 file changed, 20 insertions(+), 20 deletions(-)
>>
>> diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp
>> index 9631c9617154..d58090db14ee 100644
>> --- a/src/android/yuv/post_processor_yuv.cpp
>> +++ b/src/android/yuv/post_processor_yuv.cpp
>> @@ -101,27 +101,27 @@ bool PostProcessorYuv::isValidBuffers(const FrameBuffer &source,
>>  		return false;
>>  	}
>>
>> -	if (source.planes()[0].length < sourceLength_[0] ||
>> -	    source.planes()[1].length < sourceLength_[1]) {
>> -		LOG(YUV, Error)
>> -			<< "The source planes lengths are too small, actual size: {"
>> -			<< source.planes()[0].length << ", "
>> -			<< source.planes()[1].length
>> -			<< "}, expected size: {"
>> -			<< sourceLength_[0] << ", "
>> -			<< sourceLength_[1] << "}";
>> -		return false;
>> +	for (unsigned int i = 0; i < 2; i++) {
>> +		if (source.planes()[i].length < sourceLength_[i]) {
>> +			LOG(YUV, Error)
>> +				<< "The source planes lengths are too small, "
>> +				<< "actual size[" << i << "]="
>> +				<< source.planes()[i].length
>> +				<< ", expected size[" << i << "]="
>> +				<< sourceLength_[i];
>> +			return false;
>> +		}
>>  	}
>> -	if (destination.plane(0).size() < destinationLength_[0] ||
>> -	    destination.plane(1).size() < destinationLength_[1]) {
>> -		LOG(YUV, Error)
>> -			<< "The destination planes lengths are too small, actual size: {"
>> -			<< destination.plane(0).size() << ", "
>> -			<< destination.plane(1).size()
>> -			<< "}, expected size: {"
>> -			<< sourceLength_[0] << ", "
>> -			<< sourceLength_[1] << "}";
>> -		return false;
>> +	for (unsigned int i = 0; i < 2; i++) {
>> +		if (destination.plane(i).size() < destinationLength_[i]) {
>> +			LOG(YUV, Error)
>> +				<< "The destination planes lengths are too small, "
>> +				<< "actual size[" << i << "]="
>> +				<< destination.plane(i).size()
>> +				<< ", expected size[" << i << "]="
>> +				<< sourceLength_[i];
>
> This should be destinationLength_[i]
>
> With this fixed
> Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

Will fix for v2 and apply your reviewed tag.

>
>> +			return false;
>> +		}
>>  	}
>>
>>  	return true;
>>
>> --
>> 2.41.0
>>