[libcamera-devel] [PATCH v2 28/27] android: jpeg: Support multi-planar buffers
Hirokazu Honda
hiroh at chromium.org
Mon Sep 6 16:22:52 CEST 2021
Hi Laurent,
On Mon, Sep 6, 2021 at 11:08 PM Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi Hiro,
>
> On Mon, Sep 06, 2021 at 10:56:26PM +0900, Hirokazu Honda wrote:
> > On Mon, Sep 6, 2021 at 10:40 PM Laurent Pinchart wrote:
> > >
> > > The JPEG post-processor uses MappedFrameBuffer to access pixel data, but
> > > only uses data from the first plane. Pass the vector of planes to the
> > > encode() function to correctly handle multi-planar formats (currently
> > > limited to NV12).
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >
> > I am not against this patch.
> > But is this necessary in this series?
>
> Without it the JPEG encoder crashes when capturing a still image, so I'd
> consider it as necessary :-)
Got it.
>
> > I have done this in
> > https://patchwork.libcamera.org/project/libcamera/list/?series=2423.
>
> I've given this more thoughts over the weekend, and I think we need to
> introduce an Image class. It won't be trivial, there are quite a few
> issues to handle to make it suitable for the libcamera public API. Once
> this series gets merged I'll provide feedback on that.
>
Look forward to it!
> > > ---
> > > src/android/jpeg/encoder_libjpeg.cpp | 18 +++++++++---------
> > > src/android/jpeg/encoder_libjpeg.h | 8 +++++---
> > > src/android/jpeg/post_processor_jpeg.cpp | 2 +-
> > > src/android/jpeg/thumbnailer.cpp | 4 ++--
> > > 4 files changed, 17 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp
> > > index b8b01e20a001..7f63f36e793e 100644
> > > --- a/src/android/jpeg/encoder_libjpeg.cpp
> > > +++ b/src/android/jpeg/encoder_libjpeg.cpp
> > > @@ -103,9 +103,9 @@ int EncoderLibJpeg::configure(const StreamConfiguration &cfg)
> > > return 0;
> > > }
> > >
> > > -void EncoderLibJpeg::compressRGB(Span<const uint8_t> frame)
> > > +void EncoderLibJpeg::compressRGB(const std::vector<Span<uint8_t>> &planes)
> > > {
> > > - unsigned char *src = const_cast<unsigned char *>(frame.data());
> > > + unsigned char *src = const_cast<unsigned char *>(planes[0].data());
> > > /* \todo Stride information should come from buffer configuration. */
> > > unsigned int stride = pixelFormatInfo_->stride(compress_.image_width, 0);
> > >
> > > @@ -121,7 +121,7 @@ void EncoderLibJpeg::compressRGB(Span<const uint8_t> frame)
> > > * Compress the incoming buffer from a supported NV format.
> > > * This naively unpacks the semi-planar NV12 to a YUV888 format for libjpeg.
> > > */
> > > -void EncoderLibJpeg::compressNV(Span<const uint8_t> frame)
> > > +void EncoderLibJpeg::compressNV(const std::vector<Span<uint8_t>> &planes)
> > > {
> > > uint8_t tmprowbuf[compress_.image_width * 3];
> > >
> > > @@ -133,7 +133,6 @@ void EncoderLibJpeg::compressNV(Span<const uint8_t> frame)
> > > * Possible hints at:
> > > * https://sourceforge.net/p/libjpeg/mailman/message/30815123/
> > > */
> > > - unsigned int y_stride = pixelFormatInfo_->stride(compress_.image_width, 0);
y_stride should be used in computing src_y later in while-loop.
Reviewed-by: Hirokazu Honda <hiroh at chromium.org>
-Hiro
> > > unsigned int c_stride = pixelFormatInfo_->stride(compress_.image_width, 1);
> > >
> > > unsigned int horzSubSample = 2 * compress_.image_width / c_stride;
> > > @@ -143,8 +142,8 @@ void EncoderLibJpeg::compressNV(Span<const uint8_t> frame)
> > > unsigned int cb_pos = nvSwap_ ? 1 : 0;
> > > unsigned int cr_pos = nvSwap_ ? 0 : 1;
> > >
> > > - const unsigned char *src = frame.data();
> > > - const unsigned char *src_c = src + y_stride * compress_.image_height;
> > > + const unsigned char *src = planes[0].data();
> > > + const unsigned char *src_c = planes[1].data();
> > >
> > > JSAMPROW row_pointer[1];
> > > row_pointer[0] = &tmprowbuf[0];
> > > @@ -188,11 +187,12 @@ int EncoderLibJpeg::encode(const FrameBuffer &source, Span<uint8_t> dest,
> > > return frame.error();
> > > }
> > >
> > > - return encode(frame.planes()[0], dest, exifData, quality);
> > > + return encode(frame.planes(), dest, exifData, quality);
> > > }
> > >
> > > -int EncoderLibJpeg::encode(Span<const uint8_t> src, Span<uint8_t> dest,
> > > - Span<const uint8_t> exifData, unsigned int quality)
> > > +int EncoderLibJpeg::encode(const std::vector<Span<uint8_t>> &src,
> > > + Span<uint8_t> dest, Span<const uint8_t> exifData,
> > > + unsigned int quality)
> > > {
> > > unsigned char *destination = dest.data();
> > > unsigned long size = dest.size();
> > > diff --git a/src/android/jpeg/encoder_libjpeg.h b/src/android/jpeg/encoder_libjpeg.h
> > > index 61fbd1a69278..45ffbd7fae5d 100644
> > > --- a/src/android/jpeg/encoder_libjpeg.h
> > > +++ b/src/android/jpeg/encoder_libjpeg.h
> > > @@ -9,6 +9,8 @@
> > >
> > > #include "encoder.h"
> > >
> > > +#include <vector>
> > > +
> > > #include "libcamera/internal/formats.h"
> > >
> > > #include <jpeglib.h>
> > > @@ -24,14 +26,14 @@ public:
> > > libcamera::Span<uint8_t> destination,
> > > libcamera::Span<const uint8_t> exifData,
> > > unsigned int quality) override;
> > > - int encode(libcamera::Span<const uint8_t> source,
> > > + int encode(const std::vector<libcamera::Span<uint8_t>> &planes,
> > > libcamera::Span<uint8_t> destination,
> > > libcamera::Span<const uint8_t> exifData,
> > > unsigned int quality);
> > >
> > > private:
> > > - void compressRGB(libcamera::Span<const uint8_t> frame);
> > > - void compressNV(libcamera::Span<const uint8_t> frame);
> > > + void compressRGB(const std::vector<libcamera::Span<uint8_t>> &planes);
> > > + void compressNV(const std::vector<libcamera::Span<uint8_t>> &planes);
> > >
> > > struct jpeg_compress_struct compress_;
> > > struct jpeg_error_mgr jerr_;
> > > diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp
> > > index 3160a784419c..68d74842925d 100644
> > > --- a/src/android/jpeg/post_processor_jpeg.cpp
> > > +++ b/src/android/jpeg/post_processor_jpeg.cpp
> > > @@ -72,7 +72,7 @@ void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source,
> > > */
> > > thumbnail->resize(rawThumbnail.size());
> > >
> > > - int jpeg_size = thumbnailEncoder_.encode(rawThumbnail,
> > > + int jpeg_size = thumbnailEncoder_.encode({ rawThumbnail },
> > > *thumbnail, {}, quality);
> > > thumbnail->resize(jpeg_size);
> > >
> > > diff --git a/src/android/jpeg/thumbnailer.cpp b/src/android/jpeg/thumbnailer.cpp
> > > index 043c7b33f06a..2c341535de9e 100644
> > > --- a/src/android/jpeg/thumbnailer.cpp
> > > +++ b/src/android/jpeg/thumbnailer.cpp
> > > @@ -62,8 +62,8 @@ void Thumbnailer::createThumbnail(const FrameBuffer &source,
> > > ASSERT(tw % 2 == 0 && th % 2 == 0);
> > >
> > > /* 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 *src = frame.planes()[0].data();
> > > + unsigned char *srcC = frame.planes()[1].data();
> > > unsigned char *srcCb, *srcCr;
> > > unsigned char *dstY, *srcY;
> > >
>
> --
> Regards,
>
> Laurent Pinchart
More information about the libcamera-devel
mailing list