[libcamera-devel,RFC,1/7] android: yuv: separate source destination in length check
diff mbox series

Message ID 20230915-libyuv-convert-v1-1-1e5bcf68adac@baylibre.com
State New
Headers show
Series
  • android: add YUYV->NV12 conversion via libyuv
Related show

Commit Message

Mattijs Korpershoek Sept. 15, 2023, 7:57 a.m. UTC 
Right now, the assumption of PostProcessorYuv is that both the source
and the destination are in formats::NV12.

This might change in the future, if we add other formats or pixel format
conversion as supported in libyuv.

Split out source and destination check to prepare for that.

No functional change.

Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
---
 src/android/yuv/post_processor_yuv.cpp | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

Comments

Jacopo Mondi Sept. 22, 2023, 10:34 a.m. UTC | #1 
Hi Mattijs

On Fri, Sep 15, 2023 at 09:57:25AM +0200, Mattijs Korpershoek via libcamera-devel wrote:
> Right now, the assumption of PostProcessorYuv is that both the source
> and the destination are in formats::NV12.
>
> This might change in the future, if we add other formats or pixel format
> conversion as supported in libyuv.
>
> Split out source and destination check to prepare for that.
>
> No functional change.
>
> Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> ---
>  src/android/yuv/post_processor_yuv.cpp | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp
> index ed44e6fe02da..9631c9617154 100644
> --- a/src/android/yuv/post_processor_yuv.cpp
> +++ b/src/android/yuv/post_processor_yuv.cpp
> @@ -133,14 +133,17 @@ void PostProcessorYuv::calculateLengths(const StreamConfiguration &inCfg,
>  	sourceSize_ = inCfg.size;
>  	destinationSize_ = outCfg.size;
>
> -	const PixelFormatInfo &nv12Info = PixelFormatInfo::info(formats::NV12);
> +	const PixelFormatInfo &sourceInfo = PixelFormatInfo::info(formats::NV12);
>  	for (unsigned int i = 0; i < 2; i++) {
>  		sourceStride_[i] = inCfg.stride;
> -		destinationStride_[i] = nv12Info.stride(destinationSize_.width, i, 1);
> +		sourceLength_[i] = sourceInfo.planeSize(sourceSize_.height, i,
> +							sourceStride_[i]);
> +	}
>
> -		sourceLength_[i] = nv12Info.planeSize(sourceSize_.height, i,
> -						      sourceStride_[i]);
> -		destinationLength_[i] = nv12Info.planeSize(destinationSize_.height, i,
> -							   destinationStride_[i]);
> +	const PixelFormatInfo &destinationInfo = PixelFormatInfo::info(formats::NV12);
> +	for (unsigned int i = 0; i < 2; i++) {
> +		destinationStride_[i] = sourceInfo.stride(destinationSize_.width, i, 1);
> +		destinationLength_[i] = sourceInfo.planeSize(destinationSize_.height, i,
> +							     destinationStride_[i]);

You probably want to use the newly introduced destinationInfo here ?

>  	}
>  }
>
> --
> 2.41.0
>
Mattijs Korpershoek Sept. 24, 2023, 11:54 a.m. UTC | #2 
Hi Jacopo,

Thank you for your review.

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

> Hi Mattijs
>
> On Fri, Sep 15, 2023 at 09:57:25AM +0200, Mattijs Korpershoek via libcamera-devel wrote:
>> Right now, the assumption of PostProcessorYuv is that both the source
>> and the destination are in formats::NV12.
>>
>> This might change in the future, if we add other formats or pixel format
>> conversion as supported in libyuv.
>>
>> Split out source and destination check to prepare for that.
>>
>> No functional change.
>>
>> Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
>> ---
>>  src/android/yuv/post_processor_yuv.cpp | 15 +++++++++------
>>  1 file changed, 9 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp
>> index ed44e6fe02da..9631c9617154 100644
>> --- a/src/android/yuv/post_processor_yuv.cpp
>> +++ b/src/android/yuv/post_processor_yuv.cpp
>> @@ -133,14 +133,17 @@ void PostProcessorYuv::calculateLengths(const StreamConfiguration &inCfg,
>>  	sourceSize_ = inCfg.size;
>>  	destinationSize_ = outCfg.size;
>>
>> -	const PixelFormatInfo &nv12Info = PixelFormatInfo::info(formats::NV12);
>> +	const PixelFormatInfo &sourceInfo = PixelFormatInfo::info(formats::NV12);
>>  	for (unsigned int i = 0; i < 2; i++) {
>>  		sourceStride_[i] = inCfg.stride;
>> -		destinationStride_[i] = nv12Info.stride(destinationSize_.width, i, 1);
>> +		sourceLength_[i] = sourceInfo.planeSize(sourceSize_.height, i,
>> +							sourceStride_[i]);
>> +	}
>>
>> -		sourceLength_[i] = nv12Info.planeSize(sourceSize_.height, i,
>> -						      sourceStride_[i]);
>> -		destinationLength_[i] = nv12Info.planeSize(destinationSize_.height, i,
>> -							   destinationStride_[i]);
>> +	const PixelFormatInfo &destinationInfo = PixelFormatInfo::info(formats::NV12);
>> +	for (unsigned int i = 0; i < 2; i++) {
>> +		destinationStride_[i] = sourceInfo.stride(destinationSize_.width, i, 1);
>> +		destinationLength_[i] = sourceInfo.planeSize(destinationSize_.height, i,
>> +							     destinationStride_[i]);
>
> You probably want to use the newly introduced destinationInfo here ?

Urgh. Indeed. Thank you for catching this, will fix in v2.

>
>>  	}
>>  }
>>
>> --
>> 2.41.0
>>
Laurent Pinchart Sept. 24, 2023, 1:19 p.m. UTC | #3 
On Sun, Sep 24, 2023 at 01:54:40PM +0200, Mattijs Korpershoek via libcamera-devel wrote:
> On ven., sept. 22, 2023 at 12:34, Jacopo Mondi wrote:
> > On Fri, Sep 15, 2023 at 09:57:25AM +0200, Mattijs Korpershoek via libcamera-devel wrote:
> >> Right now, the assumption of PostProcessorYuv is that both the source
> >> and the destination are in formats::NV12.
> >>
> >> This might change in the future, if we add other formats or pixel format
> >> conversion as supported in libyuv.
> >>
> >> Split out source and destination check to prepare for that.
> >>
> >> No functional change.
> >>
> >> Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> >> ---
> >>  src/android/yuv/post_processor_yuv.cpp | 15 +++++++++------
> >>  1 file changed, 9 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp
> >> index ed44e6fe02da..9631c9617154 100644
> >> --- a/src/android/yuv/post_processor_yuv.cpp
> >> +++ b/src/android/yuv/post_processor_yuv.cpp
> >> @@ -133,14 +133,17 @@ void PostProcessorYuv::calculateLengths(const StreamConfiguration &inCfg,
> >>  	sourceSize_ = inCfg.size;
> >>  	destinationSize_ = outCfg.size;
> >>
> >> -	const PixelFormatInfo &nv12Info = PixelFormatInfo::info(formats::NV12);
> >> +	const PixelFormatInfo &sourceInfo = PixelFormatInfo::info(formats::NV12);

A blank line here would be nice.

> >>  	for (unsigned int i = 0; i < 2; i++) {
> >>  		sourceStride_[i] = inCfg.stride;
> >> -		destinationStride_[i] = nv12Info.stride(destinationSize_.width, i, 1);
> >> +		sourceLength_[i] = sourceInfo.planeSize(sourceSize_.height, i,
> >> +							sourceStride_[i]);
> >> +	}
> >>
> >> -		sourceLength_[i] = nv12Info.planeSize(sourceSize_.height, i,
> >> -						      sourceStride_[i]);
> >> -		destinationLength_[i] = nv12Info.planeSize(destinationSize_.height, i,
> >> -							   destinationStride_[i]);
> >> +	const PixelFormatInfo &destinationInfo = PixelFormatInfo::info(formats::NV12);
> >> +	for (unsigned int i = 0; i < 2; i++) {
> >> +		destinationStride_[i] = sourceInfo.stride(destinationSize_.width, i, 1);
> >> +		destinationLength_[i] = sourceInfo.planeSize(destinationSize_.height, i,
> >> +							     destinationStride_[i]);
> >
> > You probably want to use the newly introduced destinationInfo here ?
> 
> Urgh. Indeed. Thank you for catching this, will fix in v2.
> 
> >
> >>  	}
> >>  }
> >>

Patch
diff mbox series 

diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp
index ed44e6fe02da..9631c9617154 100644
--- a/src/android/yuv/post_processor_yuv.cpp
+++ b/src/android/yuv/post_processor_yuv.cpp
@@ -133,14 +133,17 @@  void PostProcessorYuv::calculateLengths(const StreamConfiguration &inCfg,
 	sourceSize_ = inCfg.size;
 	destinationSize_ = outCfg.size;
 
-	const PixelFormatInfo &nv12Info = PixelFormatInfo::info(formats::NV12);
+	const PixelFormatInfo &sourceInfo = PixelFormatInfo::info(formats::NV12);
 	for (unsigned int i = 0; i < 2; i++) {
 		sourceStride_[i] = inCfg.stride;
-		destinationStride_[i] = nv12Info.stride(destinationSize_.width, i, 1);
+		sourceLength_[i] = sourceInfo.planeSize(sourceSize_.height, i,
+							sourceStride_[i]);
+	}
 
-		sourceLength_[i] = nv12Info.planeSize(sourceSize_.height, i,
-						      sourceStride_[i]);
-		destinationLength_[i] = nv12Info.planeSize(destinationSize_.height, i,
-							   destinationStride_[i]);
+	const PixelFormatInfo &destinationInfo = PixelFormatInfo::info(formats::NV12);
+	for (unsigned int i = 0; i < 2; i++) {
+		destinationStride_[i] = sourceInfo.stride(destinationSize_.width, i, 1);
+		destinationLength_[i] = sourceInfo.planeSize(destinationSize_.height, i,
+							     destinationStride_[i]);
 	}
 }