[libcamera-devel,v3,2/3] android: camera_stream: Support PostProcessorYuv in CameraStream

13581 diff mbox series

CameraStream creates PostProcessorYuv if the destination format
is NV12.

Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/android/camera_stream.cpp | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

Hi Hiro,

Thank you for the patch.

On Tue, Aug 31, 2021 at 06:34:38PM +0900, Hirokazu Honda wrote:
> CameraStream creates PostProcessorYuv if the destination format
> is NV12.
> 
> Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/android/camera_stream.cpp | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> index fb10bf06..49a2e336 100644
> --- a/src/android/camera_stream.cpp
> +++ b/src/android/camera_stream.cpp
> @@ -9,13 +9,15 @@
>  
>  #include <sys/mman.h>
>  
> +#include <libcamera/formats.h>
> +
> +#include "jpeg/post_processor_jpeg.h"
> +#include "yuv/post_processor_yuv.h"
> +
>  #include "camera_buffer.h"
>  #include "camera_capabilities.h"
>  #include "camera_device.h"
>  #include "camera_metadata.h"
> -#include "jpeg/post_processor_jpeg.h"
> -
> -#include <libcamera/formats.h>
>  
>  using namespace libcamera;
>  
> @@ -68,6 +70,12 @@ int CameraStream::configure()
>  		StreamConfiguration output = configuration();
>  		output.pixelFormat = outFormat;
>  		switch (outFormat) {
> +		case formats::NV12:
> +			postProcessor_ = std::make_unique<PostProcessorYuv>();
> +			output.size.width = camera3Stream_->width;
> +			output.size.height = camera3Stream_->height;

Should we set this unconditionally before the switch() ? It applies to
MJPEG too, and even if it's not used today, I think it's useful to keep
the output.size always valid for consistency.

As for 1/3, if you're fine with this change, I can make it when pushing.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +			break;
> +
>  		case formats::MJPEG:
>  			postProcessor_ = std::make_unique<PostProcessorJpeg>(cameraDevice_);
>  			break;

Hi Laurent,

On Wed, Sep 1, 2021 at 5:37 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Hiro,
>
> Thank you for the patch.
>
> On Tue, Aug 31, 2021 at 06:34:38PM +0900, Hirokazu Honda wrote:
> > CameraStream creates PostProcessorYuv if the destination format
> > is NV12.
> >
> > Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  src/android/camera_stream.cpp | 14 +++++++++++---
> >  1 file changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> > index fb10bf06..49a2e336 100644
> > --- a/src/android/camera_stream.cpp
> > +++ b/src/android/camera_stream.cpp
> > @@ -9,13 +9,15 @@
> >
> >  #include <sys/mman.h>
> >
> > +#include <libcamera/formats.h>
> > +
> > +#include "jpeg/post_processor_jpeg.h"
> > +#include "yuv/post_processor_yuv.h"
> > +
> >  #include "camera_buffer.h"
> >  #include "camera_capabilities.h"
> >  #include "camera_device.h"
> >  #include "camera_metadata.h"
> > -#include "jpeg/post_processor_jpeg.h"
> > -
> > -#include <libcamera/formats.h>
> >
> >  using namespace libcamera;
> >
> > @@ -68,6 +70,12 @@ int CameraStream::configure()
> >               StreamConfiguration output = configuration();
> >               output.pixelFormat = outFormat;
> >               switch (outFormat) {
> > +             case formats::NV12:
> > +                     postProcessor_ = std::make_unique<PostProcessorYuv>();
> > +                     output.size.width = camera3Stream_->width;
> > +                     output.size.height = camera3Stream_->height;
>
> Should we set this unconditionally before the switch() ? It applies to
> MJPEG too, and even if it's not used today, I think it's useful to keep
> the output.size always valid for consistency.
>
> As for 1/3, if you're fine with this change, I can make it when pushing.
>

That makes sense. It should work although I haven't tested it.

Thanks,
-Hiro
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> > +                     break;
> > +
> >               case formats::MJPEG:
> >                       postProcessor_ = std::make_unique<PostProcessorJpeg>(cameraDevice_);
> >                       break;
>
> --
> Regards,
>
> Laurent Pinchart

Hi,

On 9/1/21 2:06 AM, Laurent Pinchart wrote:
> Hi Hiro,
>
> Thank you for the patch.
>
> On Tue, Aug 31, 2021 at 06:34:38PM +0900, Hirokazu Honda wrote:
>> CameraStream creates PostProcessorYuv if the destination format
>> is NV12.
>>
>> Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
>> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
>> ---
>>   src/android/camera_stream.cpp | 14 +++++++++++---
>>   1 file changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
>> index fb10bf06..49a2e336 100644
>> --- a/src/android/camera_stream.cpp
>> +++ b/src/android/camera_stream.cpp
>> @@ -9,13 +9,15 @@
>>   
>>   #include <sys/mman.h>
>>   
>> +#include <libcamera/formats.h>
>> +
>> +#include "jpeg/post_processor_jpeg.h"
>> +#include "yuv/post_processor_yuv.h"
>> +
>>   #include "camera_buffer.h"
>>   #include "camera_capabilities.h"
>>   #include "camera_device.h"
>>   #include "camera_metadata.h"
>> -#include "jpeg/post_processor_jpeg.h"
>> -
>> -#include <libcamera/formats.h>
>>   
>>   using namespace libcamera;
>>   
>> @@ -68,6 +70,12 @@ int CameraStream::configure()
>>   		StreamConfiguration output = configuration();
>>   		output.pixelFormat = outFormat;
>>   		switch (outFormat) {
>> +		case formats::NV12:
>> +			postProcessor_ = std::make_unique<PostProcessorYuv>();
>> +			output.size.width = camera3Stream_->width;
>> +			output.size.height = camera3Stream_->height;
> Should we set this unconditionally before the switch() ? It applies to
> MJPEG too, and even if it's not used today, I think it's useful to keep
> the output.size always valid for consistency.


this is my thought process as well. so +1

>
> As for 1/3, if you're fine with this change, I can make it when pushing.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>


Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>

>
>> +			break;
>> +
>>   		case formats::MJPEG:
>>   			postProcessor_ = std::make_unique<PostProcessorJpeg>(cameraDevice_);
>>   			break;