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
diff --git a/libs/ultrahdr/include/ultrahdr/jpegencoderhelper.h b/libs/ultrahdr/include/ultrahdr/jpegencoderhelper.h
index ac02155..2c6778e 100644
--- a/libs/ultrahdr/include/ultrahdr/jpegencoderhelper.h
+++ b/libs/ultrahdr/include/ultrahdr/jpegencoderhelper.h
@@ -61,6 +61,11 @@
*/
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 @@
// 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 fc6e4d1..10a7630 100644
--- a/libs/ultrahdr/jpegencoderhelper.cpp
+++ b/libs/ultrahdr/jpegencoderhelper.cpp
@@ -38,11 +38,6 @@
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 8e1dc8c..24d1911 100644
--- a/libs/ultrahdr/jpegr.cpp
+++ b/libs/ultrahdr/jpegr.cpp
@@ -66,9 +66,12 @@
// 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 @@
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 @@
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 b9a2d84..8f18ac0 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 @@
virtual void SetUp();
virtual void TearDown();
- Image mValidImage, mInvalidSizeImage, mSingleChannelImage;
+ Image mAlignedImage, mUnalignedImage, mSingleChannelImage;
};
JpegEncoderHelperTest::JpegEncoderHelperTest() {}
@@ -82,16 +82,16 @@
}
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::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));