JPEG/R: lift the checking criteria for width 8-alignment

JPEG/R library uses jpeg-turbo for JPEG encoding, which runs DCT transform on block size of 16x16 for luma, and 8x8 for chroma. The resolution in the bug report is not 16-aligned and it results in null pointer dereference for the last line in jpeg-turbo.

The original checking for 8-alignment width was wrong (should check
16-alignment). jpeg-turbo has some edge case handling for this case, and
it requires some extra room at the end of input. This change removed the checking criteria by adding a
padding zero method. A reason size of the padding zeros is a CB block,
which is 8x8, 64 bytes.

Bug: 277982036
Test: CTS: ImageReaderTest#testJpegR, uint test: jpegrencoderhelper_test.cpp

Change-Id: I1313a002db6d4bc63b32dc3dd3d6ccdf06779149

4 files changed, 52 insertions, 44 deletions

diff --git a/libs/ultrahdr/include/ultrahdr/jpegencoderhelper.h b/libs/ultrahdr/include/ultrahdr/jpegencoderhelper.h
index ac0215559d..2c6778e299 100644
--- a/libs/ultrahdr/include/ultrahdr/jpegencoderhelper.h
+++ b/libs/ultrahdr/include/ultrahdr/jpegencoderhelper.h
@@ -61,6 +61,11 @@ public:
      */
     size_t getCompressedImageSize();
 
+    /*
+     * Process 16 lines of Y and 16 lines of U/V each time.
+     * We must pass at least 16 scanlines according to libjpeg documentation.
+     */
+    static const int kCompressBatchSize = 16;
 private:
     // initDestination(), emptyOutputBuffer() and emptyOutputBuffer() are callback functions to be
     // passed into jpeg library.
@@ -82,9 +87,6 @@ private:
 
     // The block size for encoded jpeg image buffer.
     static const int kBlockSize = 16384;
-    // Process 16 lines of Y and 16 lines of U/V each time.
-    // We must pass at least 16 scanlines according to libjpeg documentation.
-    static const int kCompressBatchSize = 16;
 
     // The buffer that holds the compressed result.
     std::vector<JOCTET> mResultBuffer;
diff --git a/libs/ultrahdr/jpegencoderhelper.cpp b/libs/ultrahdr/jpegencoderhelper.cpp
index fc6e4d1e64..10a763035f 100644
--- a/libs/ultrahdr/jpegencoderhelper.cpp
+++ b/libs/ultrahdr/jpegencoderhelper.cpp
@@ -38,11 +38,6 @@ JpegEncoderHelper::~JpegEncoderHelper() {
 bool JpegEncoderHelper::compressImage(const void* image, int width, int height, int quality,
                                    const void* iccBuffer, unsigned int iccSize,
                                    bool isSingleChannel) {
-    if (width % 8 != 0 || height % 2 != 0) {
-        ALOGE("Image size can not be handled: %dx%d", width, height);
-        return false;
-    }
-
     mResultBuffer.clear();
     if (!encode(image, width, height, quality, iccBuffer, iccSize, isSingleChannel)) {
         return false;
diff --git a/libs/ultrahdr/jpegr.cpp b/libs/ultrahdr/jpegr.cpp
index 8e1dc8c529..24d1911853 100644
--- a/libs/ultrahdr/jpegr.cpp
+++ b/libs/ultrahdr/jpegr.cpp
@@ -66,9 +66,12 @@ static const uint32_t kJpegrVersion = 1;
 // Map is quarter res / sixteenth size
 static const size_t kMapDimensionScaleFactor = 4;
 // JPEG block size.
-// JPEG encoding / decoding will require 8 x 8 DCT transform.
-// Width must be 8 dividable, and height must be 2 dividable.
-static const size_t kJpegBlock = 8;
+// JPEG encoding / decoding will require block based DCT transform 16 x 16 for luma,
+// and 8 x 8 for chroma.
+// Width must be 16 dividable for luma, and 8 dividable for chroma.
+// If this criteria is not ficilitated, we will pad zeros based on the required block size.
+static const size_t kJpegBlock = JpegEncoderHelper::kCompressBatchSize;
+static const size_t kJpegBlockSquare = kJpegBlock * kJpegBlock;
 // JPEG compress quality (0 ~ 100) for gain map
 static const int kMapCompressQuality = 85;
 
@@ -92,13 +95,6 @@ status_t JpegR::areInputImagesValid(jr_uncompressed_ptr uncompressed_p010_image,
     return ERROR_JPEGR_INVALID_NULL_PTR;
   }
 
-  if (uncompressed_p010_image->width % kJpegBlock != 0
-          || uncompressed_p010_image->height % 2 != 0) {
-    ALOGE("Image size can not be handled: %dx%d.",
-            uncompressed_p010_image->width, uncompressed_p010_image->height);
-    return ERROR_JPEGR_INVALID_INPUT_TYPE;
-  }
-
   if (uncompressed_p010_image->luma_stride != 0
           && uncompressed_p010_image->luma_stride < uncompressed_p010_image->width) {
     ALOGE("Image stride can not be smaller than width, stride=%d, width=%d",
@@ -157,8 +153,13 @@ status_t JpegR::encodeJPEGR(jr_uncompressed_ptr uncompressed_p010_image,
   metadata.version = kJpegrVersion;
 
   jpegr_uncompressed_struct uncompressed_yuv_420_image;
-  unique_ptr<uint8_t[]> uncompressed_yuv_420_image_data = make_unique<uint8_t[]>(
-      uncompressed_p010_image->width * uncompressed_p010_image->height * 3 / 2);
+  size_t gain_map_length = uncompressed_p010_image->width * uncompressed_p010_image->height * 3 / 2;
+  // Pad a pseudo chroma block (kJpegBlock / 2) x (kJpegBlock / 2)
+  // if width is not kJpegBlock aligned.
+  if (uncompressed_p010_image->width % kJpegBlock != 0) {
+    gain_map_length += kJpegBlockSquare / 4;
+  }
+  unique_ptr<uint8_t[]> uncompressed_yuv_420_image_data = make_unique<uint8_t[]>(gain_map_length);
   uncompressed_yuv_420_image.data = uncompressed_yuv_420_image_data.get();
   JPEGR_CHECK(toneMap(uncompressed_p010_image, &uncompressed_yuv_420_image));
 
diff --git a/libs/ultrahdr/tests/jpegencoderhelper_test.cpp b/libs/ultrahdr/tests/jpegencoderhelper_test.cpp
index b9a2d84807..8f18ac0004 100644
--- a/libs/ultrahdr/tests/jpegencoderhelper_test.cpp
+++ b/libs/ultrahdr/tests/jpegencoderhelper_test.cpp
@@ -22,15 +22,15 @@
 
 namespace android::ultrahdr {
 
-#define VALID_IMAGE "/sdcard/Documents/minnie-320x240.yu12"
-#define VALID_IMAGE_WIDTH 320
-#define VALID_IMAGE_HEIGHT 240
+#define ALIGNED_IMAGE "/sdcard/Documents/minnie-320x240.yu12"
+#define ALIGNED_IMAGE_WIDTH 320
+#define ALIGNED_IMAGE_HEIGHT 240
 #define SINGLE_CHANNEL_IMAGE "/sdcard/Documents/minnie-320x240.y"
-#define SINGLE_CHANNEL_IMAGE_WIDTH VALID_IMAGE_WIDTH
-#define SINGLE_CHANNEL_IMAGE_HEIGHT VALID_IMAGE_HEIGHT
-#define INVALID_SIZE_IMAGE "/sdcard/Documents/minnie-318x240.yu12"
-#define INVALID_SIZE_IMAGE_WIDTH 318
-#define INVALID_SIZE_IMAGE_HEIGHT 240
+#define SINGLE_CHANNEL_IMAGE_WIDTH ALIGNED_IMAGE_WIDTH
+#define SINGLE_CHANNEL_IMAGE_HEIGHT ALIGNED_IMAGE_HEIGHT
+#define UNALIGNED_IMAGE "/sdcard/Documents/minnie-318x240.yu12"
+#define UNALIGNED_IMAGE_WIDTH 318
+#define UNALIGNED_IMAGE_HEIGHT 240
 #define JPEG_QUALITY 90
 
 class JpegEncoderHelperTest : public testing::Test {
@@ -46,7 +46,7 @@ protected:
     virtual void SetUp();
     virtual void TearDown();
 
-    Image mValidImage, mInvalidSizeImage, mSingleChannelImage;
+    Image mAlignedImage, mUnalignedImage, mSingleChannelImage;
 };
 
 JpegEncoderHelperTest::JpegEncoderHelperTest() {}
@@ -82,16 +82,16 @@ static bool loadFile(const char filename[], JpegEncoderHelperTest::Image* result
 }
 
 void JpegEncoderHelperTest::SetUp() {
-    if (!loadFile(VALID_IMAGE, &mValidImage)) {
-        FAIL() << "Load file " << VALID_IMAGE << " failed";
+    if (!loadFile(ALIGNED_IMAGE, &mAlignedImage)) {
+        FAIL() << "Load file " << ALIGNED_IMAGE << " failed";
     }
-    mValidImage.width = VALID_IMAGE_WIDTH;
-    mValidImage.height = VALID_IMAGE_HEIGHT;
-    if (!loadFile(INVALID_SIZE_IMAGE, &mInvalidSizeImage)) {
-        FAIL() << "Load file " << INVALID_SIZE_IMAGE << " failed";
+    mAlignedImage.width = ALIGNED_IMAGE_WIDTH;
+    mAlignedImage.height = ALIGNED_IMAGE_HEIGHT;
+    if (!loadFile(UNALIGNED_IMAGE, &mUnalignedImage)) {
+        FAIL() << "Load file " << UNALIGNED_IMAGE << " failed";
     }
-    mInvalidSizeImage.width = INVALID_SIZE_IMAGE_WIDTH;
-    mInvalidSizeImage.height = INVALID_SIZE_IMAGE_HEIGHT;
+    mUnalignedImage.width = UNALIGNED_IMAGE_WIDTH;
+    mUnalignedImage.height = UNALIGNED_IMAGE_HEIGHT;
     if (!loadFile(SINGLE_CHANNEL_IMAGE, &mSingleChannelImage)) {
         FAIL() << "Load file " << SINGLE_CHANNEL_IMAGE << " failed";
     }
@@ -101,20 +101,30 @@ void JpegEncoderHelperTest::SetUp() {
 
 void JpegEncoderHelperTest::TearDown() {}
 
-TEST_F(JpegEncoderHelperTest, validImage) {
+TEST_F(JpegEncoderHelperTest, encodeAlignedImage) {
     JpegEncoderHelper encoder;
-    EXPECT_TRUE(encoder.compressImage(mValidImage.buffer.get(), mValidImage.width,
-                                         mValidImage.height, JPEG_QUALITY, NULL, 0));
+    EXPECT_TRUE(encoder.compressImage(mAlignedImage.buffer.get(), mAlignedImage.width,
+                                      mAlignedImage.height, JPEG_QUALITY, NULL, 0));
     ASSERT_GT(encoder.getCompressedImageSize(), static_cast<uint32_t>(0));
 }
 
-TEST_F(JpegEncoderHelperTest, invalidSizeImage) {
+// The width of the "unaligned" image is not 16-aligned, and will fail if encoded directly.
+// Should pass with the padding zero method.
+TEST_F(JpegEncoderHelperTest, encodeUnalignedImage) {
     JpegEncoderHelper encoder;
-    EXPECT_FALSE(encoder.compressImage(mInvalidSizeImage.buffer.get(), mInvalidSizeImage.width,
-                                          mInvalidSizeImage.height, JPEG_QUALITY, NULL, 0));
+    const size_t paddingZeroLength = JpegEncoderHelper::kCompressBatchSize
+            * JpegEncoderHelper::kCompressBatchSize / 4;
+    std::unique_ptr<uint8_t[]> imageWithPaddingZeros(
+            new uint8_t[UNALIGNED_IMAGE_WIDTH * UNALIGNED_IMAGE_HEIGHT * 3 / 2
+            + paddingZeroLength]);
+    memcpy(imageWithPaddingZeros.get(), mUnalignedImage.buffer.get(),
+            UNALIGNED_IMAGE_WIDTH * UNALIGNED_IMAGE_HEIGHT * 3 / 2);
+    EXPECT_TRUE(encoder.compressImage(imageWithPaddingZeros.get(), mUnalignedImage.width,
+                                      mUnalignedImage.height, JPEG_QUALITY, NULL, 0));
+    ASSERT_GT(encoder.getCompressedImageSize(), static_cast<uint32_t>(0));
 }
 
-TEST_F(JpegEncoderHelperTest, singleChannelImage) {
+TEST_F(JpegEncoderHelperTest, encodeSingleChannelImage) {
     JpegEncoderHelper encoder;
     EXPECT_TRUE(encoder.compressImage(mSingleChannelImage.buffer.get(), mSingleChannelImage.width,
                                          mSingleChannelImage.height, JPEG_QUALITY, NULL, 0, true));