[libcamera-devel,2/5] android: jpeg: thumbnailer: Use MappedFrameBuffer::planes()
diff mbox series

Message ID 20210831063438.785767-3-hiroh@chromium.org
State New
Headers show
Series
  • Improve/Clean up post processors code in android
Related show

Commit Message

Hirokazu Honda Aug. 31, 2021, 6:34 a.m. UTC
Thumbnailer gets the address of NV12 UV Plane by computing the
first plane size. MappedFrameBuffer::planes() returns the plane
address. This replaces the address computation with
MappedFrameBuffer::planes() in Thumbnailer.

Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
---
 src/android/jpeg/thumbnailer.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jacopo Mondi Aug. 31, 2021, 10:35 a.m. UTC | #1
Hi Hiro,

On Tue, Aug 31, 2021 at 03:34:35PM +0900, Hirokazu Honda wrote:
> Thumbnailer gets the address of NV12 UV Plane by computing the
> first plane size. MappedFrameBuffer::planes() returns the plane
> address. This replaces the address computation with
> MappedFrameBuffer::planes() in Thumbnailer.
>
> Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
> ---
>  src/android/jpeg/thumbnailer.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/android/jpeg/thumbnailer.cpp b/src/android/jpeg/thumbnailer.cpp
> index 043c7b33..fe2c4969 100644
> --- a/src/android/jpeg/thumbnailer.cpp
> +++ b/src/android/jpeg/thumbnailer.cpp
> @@ -63,7 +63,7 @@ void Thumbnailer::createThumbnail(const FrameBuffer &source,
>
>  	/* Image scaling block implementing nearest-neighbour algorithm. */
>  	unsigned char *src = static_cast<unsigned char *>(frame.planes()[0].data());
> -	unsigned char *srcC = src + sh * sw;
> +	unsigned char *srcC = static_cast<unsigned char *>(frame.planes()[1].data());

I am not sure I really understand the thumbnailer code, but this seems
sane.

Assuming the new srcC value is the same as the old one
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

>  	unsigned char *srcCb, *srcCr;
>  	unsigned char *dstY, *srcY;
>
> --
> 2.33.0.259.gc128427fd7-goog
>
Laurent Pinchart Aug. 31, 2021, 9:15 p.m. UTC | #2
Hi Hiro,

Thank you for the patch.

On Tue, Aug 31, 2021 at 12:35:51PM +0200, Jacopo Mondi wrote:
> On Tue, Aug 31, 2021 at 03:34:35PM +0900, Hirokazu Honda wrote:
> > Thumbnailer gets the address of NV12 UV Plane by computing the
> > first plane size. MappedFrameBuffer::planes() returns the plane
> > address. This replaces the address computation with
> > MappedFrameBuffer::planes() in Thumbnailer.
> >
> > Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
> > ---
> >  src/android/jpeg/thumbnailer.cpp | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/android/jpeg/thumbnailer.cpp b/src/android/jpeg/thumbnailer.cpp
> > index 043c7b33..fe2c4969 100644
> > --- a/src/android/jpeg/thumbnailer.cpp
> > +++ b/src/android/jpeg/thumbnailer.cpp
> > @@ -63,7 +63,7 @@ void Thumbnailer::createThumbnail(const FrameBuffer &source,
> >
> >  	/* Image scaling block implementing nearest-neighbour algorithm. */
> >  	unsigned char *src = static_cast<unsigned char *>(frame.planes()[0].data());
> > -	unsigned char *srcC = src + sh * sw;
> > +	unsigned char *srcC = static_cast<unsigned char *>(frame.planes()[1].data());
> 
> I am not sure I really understand the thumbnailer code, but this seems
> sane.
> 
> Assuming the new srcC value is the same as the old one
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

It looks much nicer, happy to see the mapped frame buffer API making the
code cleaner.

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

> >  	unsigned char *srcCb, *srcCr;
> >  	unsigned char *dstY, *srcY;
> >

Patch
diff mbox series

diff --git a/src/android/jpeg/thumbnailer.cpp b/src/android/jpeg/thumbnailer.cpp
index 043c7b33..fe2c4969 100644
--- a/src/android/jpeg/thumbnailer.cpp
+++ b/src/android/jpeg/thumbnailer.cpp
@@ -63,7 +63,7 @@  void Thumbnailer::createThumbnail(const FrameBuffer &source,
 
 	/* Image scaling block implementing nearest-neighbour algorithm. */
 	unsigned char *src = static_cast<unsigned char *>(frame.planes()[0].data());
-	unsigned char *srcC = src + sh * sw;
+	unsigned char *srcC = static_cast<unsigned char *>(frame.planes()[1].data());
 	unsigned char *srcCb, *srcCr;
 	unsigned char *dstY, *srcY;