[libcamera-devel,RFC,5/7] android: camera_stream: add add explicit input configuration
diff mbox series

Message ID 20230915-libyuv-convert-v1-5-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 
PostProcessors are configured using and input StreamConfigurations.
Right now, we use configuration() for the input StreamConfiguration.

Use an intermediate variable instead, to prepare for software conversion using libyuv.

Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
---
 src/android/camera_stream.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Jacopo Mondi Sept. 22, 2023, 1:45 p.m. UTC | #1 
Hi Mattijs

On Fri, Sep 15, 2023 at 09:57:29AM +0200, Mattijs Korpershoek via libcamera-devel wrote:
> PostProcessors are configured using and input StreamConfigurations.
> Right now, we use configuration() for the input StreamConfiguration.
>
> Use an intermediate variable instead, to prepare for software conversion using libyuv.

Try to stay < 70 cols in commit messages ;)

>
> Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j
> ---
>  src/android/camera_stream.cpp | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> index 045e60061a20..4fd05dda5ed3 100644
> --- a/src/android/camera_stream.cpp
> +++ b/src/android/camera_stream.cpp
> @@ -87,6 +87,7 @@ int CameraStream::configure()
>  	if (type_ == Type::Internal || type_ == Type::Mapped) {
>  		const PixelFormat outFormat =
>  			cameraDevice_->capabilities()->toPixelFormat(camera3Stream_->format);
> +		StreamConfiguration input = configuration();
>  		StreamConfiguration output = configuration();
>  		output.pixelFormat = outFormat;
>  		output.size.width = camera3Stream_->width;
> @@ -106,7 +107,7 @@ int CameraStream::configure()
>  			return -EINVAL;
>  		}
>
> -		int ret = postProcessor_->configure(configuration(), output);
> +		int ret = postProcessor_->configure(input, output);
>  		if (ret)
>  			return ret;
>
>
> --
> 2.41.0
>
Mattijs Korpershoek Sept. 24, 2023, 12:18 p.m. UTC | #2 
Hi Jacopo,

Thank you for your review.

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

> Hi Mattijs
>
> On Fri, Sep 15, 2023 at 09:57:29AM +0200, Mattijs Korpershoek via libcamera-devel wrote:
>> PostProcessors are configured using and input StreamConfigurations.
>> Right now, we use configuration() for the input StreamConfiguration.
>>
>> Use an intermediate variable instead, to prepare for software conversion using libyuv.
>
> Try to stay < 70 cols in commit messages ;)

Since I have to spin a v2, will fix this.

>
>>
>> Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>
> Thanks
>   j
>> ---
>>  src/android/camera_stream.cpp | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
>> index 045e60061a20..4fd05dda5ed3 100644
>> --- a/src/android/camera_stream.cpp
>> +++ b/src/android/camera_stream.cpp
>> @@ -87,6 +87,7 @@ int CameraStream::configure()
>>  	if (type_ == Type::Internal || type_ == Type::Mapped) {
>>  		const PixelFormat outFormat =
>>  			cameraDevice_->capabilities()->toPixelFormat(camera3Stream_->format);
>> +		StreamConfiguration input = configuration();
>>  		StreamConfiguration output = configuration();
>>  		output.pixelFormat = outFormat;
>>  		output.size.width = camera3Stream_->width;
>> @@ -106,7 +107,7 @@ int CameraStream::configure()
>>  			return -EINVAL;
>>  		}
>>
>> -		int ret = postProcessor_->configure(configuration(), output);
>> +		int ret = postProcessor_->configure(input, output);
>>  		if (ret)
>>  			return ret;
>>
>>
>> --
>> 2.41.0
>>
Laurent Pinchart Sept. 24, 2023, 6:54 p.m. UTC | #3 
On Sun, Sep 24, 2023 at 02:18:54PM +0200, Mattijs Korpershoek via libcamera-devel wrote:
> On ven., sept. 22, 2023 at 15:45, Jacopo Mondi wrote:
> > On Fri, Sep 15, 2023 at 09:57:29AM +0200, Mattijs Korpershoek via libcamera-devel wrote:
> >> PostProcessors are configured using and input StreamConfigurations.
> >> Right now, we use configuration() for the input StreamConfiguration.
> >>
> >> Use an intermediate variable instead, to prepare for software conversion using libyuv.
> >
> > Try to stay < 70 cols in commit messages ;)
> 
> Since I have to spin a v2, will fix this.

It's 72 columns actually :-) I don't know if it's specific to my
distribution or my editor, but vim automatically wraps at 72 columns in
commit messages for me.

> >> Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> >
> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> >
> >> ---
> >>  src/android/camera_stream.cpp | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> >> index 045e60061a20..4fd05dda5ed3 100644
> >> --- a/src/android/camera_stream.cpp
> >> +++ b/src/android/camera_stream.cpp
> >> @@ -87,6 +87,7 @@ int CameraStream::configure()
> >>  	if (type_ == Type::Internal || type_ == Type::Mapped) {
> >>  		const PixelFormat outFormat =
> >>  			cameraDevice_->capabilities()->toPixelFormat(camera3Stream_->format);
> >> +		StreamConfiguration input = configuration();
> >>  		StreamConfiguration output = configuration();
> >>  		output.pixelFormat = outFormat;
> >>  		output.size.width = camera3Stream_->width;
> >> @@ -106,7 +107,7 @@ int CameraStream::configure()
> >>  			return -EINVAL;
> >>  		}
> >>
> >> -		int ret = postProcessor_->configure(configuration(), output);
> >> +		int ret = postProcessor_->configure(input, output);

The change looks fine, but it's hard to review it on its own. As the
change is very small, I would squash this with the patch that needs it.

> >>  		if (ret)
> >>  			return ret;
> >>

Patch
diff mbox series 

diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
index 045e60061a20..4fd05dda5ed3 100644
--- a/src/android/camera_stream.cpp
+++ b/src/android/camera_stream.cpp
@@ -87,6 +87,7 @@  int CameraStream::configure()
 	if (type_ == Type::Internal || type_ == Type::Mapped) {
 		const PixelFormat outFormat =
 			cameraDevice_->capabilities()->toPixelFormat(camera3Stream_->format);
+		StreamConfiguration input = configuration();
 		StreamConfiguration output = configuration();
 		output.pixelFormat = outFormat;
 		output.size.width = camera3Stream_->width;
@@ -106,7 +107,7 @@  int CameraStream::configure()
 			return -EINVAL;
 		}
 
-		int ret = postProcessor_->configure(configuration(), output);
+		int ret = postProcessor_->configure(input, output);
 		if (ret)
 			return ret;