From 1790bb5d5bd420047ed14ec0bfd2c73478c747d0 Mon Sep 17 00:00:00 2001 From: Ram Mohan Date: Sun, 11 Jun 2023 07:07:27 +0530 Subject: ultrahdr: do not select less accurate integer method during dct At high quality setting, less accurate integer method can have larger psnr drops. Switch to default setting Bug: 286617381 Test: ./libultrahdr_app -p inp_p010.yuv -y inp_420p.yuv -w 1920 -h 1080 -o 0 Change-Id: Ide126b87262e1c9a20edca7873b6ca27fc52b2cb --- libs/ultrahdr/jpegdecoderhelper.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'libs/ultrahdr/jpegdecoderhelper.cpp') diff --git a/libs/ultrahdr/jpegdecoderhelper.cpp b/libs/ultrahdr/jpegdecoderhelper.cpp index fac90c503d..6c3a8c10b7 100644 --- a/libs/ultrahdr/jpegdecoderhelper.cpp +++ b/libs/ultrahdr/jpegdecoderhelper.cpp @@ -242,7 +242,7 @@ bool JpegDecoderHelper::decode(const void* image, int length, bool decodeToRGBA) cinfo.raw_data_out = TRUE; } - cinfo.dct_method = JDCT_IFAST; + cinfo.dct_method = JDCT_ISLOW; jpeg_start_decompress(&cinfo); -- cgit v1.2.3-59-g8ed1b From f63125c42183c26fe871348d213d5628c8dfe32d Mon Sep 17 00:00:00 2001 From: Dichen Zhang Date: Mon, 31 Jul 2023 22:27:27 +0000 Subject: ultrahdr: handle unsupported sampling formats If primary/gain-map image sampling format is not as expected mark the api call for failure Bug: 290504502 Test: ./ultrahdr_dec_fuzzer Change-Id: I039bd2d198c13d236cc8687461519194451e63d4 --- libs/ultrahdr/jpegdecoderhelper.cpp | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) (limited to 'libs/ultrahdr/jpegdecoderhelper.cpp') diff --git a/libs/ultrahdr/jpegdecoderhelper.cpp b/libs/ultrahdr/jpegdecoderhelper.cpp index fef544452a..d22f4eca9a 100644 --- a/libs/ultrahdr/jpegdecoderhelper.cpp +++ b/libs/ultrahdr/jpegdecoderhelper.cpp @@ -227,10 +227,20 @@ bool JpegDecoderHelper::decode(const void* image, int length, bool decodeToRGBA) mHeight = cinfo.image_height; if (decodeToRGBA) { - if (cinfo.jpeg_color_space == JCS_GRAYSCALE) { - // We don't intend to support decoding grayscale to RGBA - status = false; - ALOGE("%s: decoding grayscale to RGBA is unsupported", __func__); + // The primary image is expected to be yuv420 sampling + if (cinfo.jpeg_color_space != JCS_YCbCr) { + status = false; + ALOGE("%s: decodeToRGBA unexpected jpeg color space ", __func__); + goto CleanUp; + } + if (cinfo.comp_info[0].h_samp_factor != 2 || + cinfo.comp_info[1].h_samp_factor != 1 || + cinfo.comp_info[2].h_samp_factor != 1 || + cinfo.comp_info[0].v_samp_factor != 2 || + cinfo.comp_info[1].v_samp_factor != 1 || + cinfo.comp_info[2].v_samp_factor != 1 ) { + status = false; + ALOGE("%s: decodeToRGBA unexpected primary image sub-sampling", __func__); goto CleanUp; } // 4 bytes per pixel @@ -251,6 +261,10 @@ bool JpegDecoderHelper::decode(const void* image, int length, bool decodeToRGBA) mResultBuffer.resize(cinfo.image_width * cinfo.image_height * 3 / 2, 0); } else if (cinfo.jpeg_color_space == JCS_GRAYSCALE) { mResultBuffer.resize(cinfo.image_width * cinfo.image_height, 0); + } else { + status = false; + ALOGE("%s: decodeToYUV unexpected jpeg color space", __func__); + goto CleanUp; } cinfo.out_color_space = cinfo.jpeg_color_space; cinfo.raw_data_out = TRUE; -- cgit v1.2.3-59-g8ed1b From 0202c12515ed55a4450377776d1601bbc135f4b5 Mon Sep 17 00:00:00 2001 From: Ram Mohan Date: Sun, 13 Aug 2023 17:22:35 +0530 Subject: ultrahdr: updates to jpegr impl - 2 - clang-format the code to a common guideline - test resources are automatically pushed by pusher - minor cleanup Bug: 294218453 Test: atest ultrahdr_unit_test Change-Id: I917e9385f2a4b92add8667248a88789b5332da76 --- libs/ultrahdr/include/ultrahdr/jpegdecoderhelper.h | 7 +- libs/ultrahdr/jpegdecoderhelper.cpp | 158 ++++++++------------- libs/ultrahdr/tests/Android.bp | 37 +---- libs/ultrahdr/tests/AndroidTest.xml | 26 ++++ libs/ultrahdr/tests/jpegdecoderhelper_test.cpp | 20 +-- libs/ultrahdr/tests/jpegencoderhelper_test.cpp | 6 +- libs/ultrahdr/tests/jpegr_test.cpp | 6 +- 7 files changed, 113 insertions(+), 147 deletions(-) create mode 100644 libs/ultrahdr/tests/AndroidTest.xml (limited to 'libs/ultrahdr/jpegdecoderhelper.cpp') diff --git a/libs/ultrahdr/include/ultrahdr/jpegdecoderhelper.h b/libs/ultrahdr/include/ultrahdr/jpegdecoderhelper.h index 8b5499a2c0..1f0c38ff7f 100644 --- a/libs/ultrahdr/include/ultrahdr/jpegdecoderhelper.h +++ b/libs/ultrahdr/include/ultrahdr/jpegdecoderhelper.h @@ -26,6 +26,8 @@ extern "C" { #include #include +// constraint on max width and max height is only due to device alloc constraints +// Can tune these values basing on the target device static const int kMaxWidth = 8192; static const int kMaxHeight = 8192; @@ -94,9 +96,8 @@ public: /* * Decompresses metadata of the image. All vectors are owned by the caller. */ - bool getCompressedImageParameters(const void* image, int length, - size_t* pWidth, size_t* pHeight, - std::vector* iccData, + bool getCompressedImageParameters(const void* image, int length, size_t* pWidth, + size_t* pHeight, std::vector* iccData, std::vector* exifData); private: diff --git a/libs/ultrahdr/jpegdecoderhelper.cpp b/libs/ultrahdr/jpegdecoderhelper.cpp index 33bf9ef3d4..7ccf1f3697 100644 --- a/libs/ultrahdr/jpegdecoderhelper.cpp +++ b/libs/ultrahdr/jpegdecoderhelper.cpp @@ -26,11 +26,11 @@ using namespace std; namespace android::ultrahdr { -#define ALIGNM(x, m) ((((x) + ((m) - 1)) / (m)) * (m)) +#define ALIGNM(x, m) ((((x) + ((m)-1)) / (m)) * (m)) -const uint32_t kAPP0Marker = JPEG_APP0; // JFIF -const uint32_t kAPP1Marker = JPEG_APP0 + 1; // EXIF, XMP -const uint32_t kAPP2Marker = JPEG_APP0 + 2; // ICC +const uint32_t kAPP0Marker = JPEG_APP0; // JFIF +const uint32_t kAPP1Marker = JPEG_APP0 + 1; // EXIF, XMP +const uint32_t kAPP2Marker = JPEG_APP0 + 2; // ICC const std::string kXmpNameSpace = "http://ns.adobe.com/xap/1.0/"; const std::string kExifIdCode = "Exif"; @@ -76,8 +76,8 @@ static void jpegr_skip_input_data(j_decompress_ptr cinfo, long num_bytes) { static void jpegr_term_source(j_decompress_ptr /*cinfo*/) {} -jpegr_source_mgr::jpegr_source_mgr(const uint8_t* ptr, int len) : - mBufferPtr(ptr), mBufferLength(len) { +jpegr_source_mgr::jpegr_source_mgr(const uint8_t* ptr, int len) + : mBufferPtr(ptr), mBufferLength(len) { init_source = jpegr_init_source; fill_input_buffer = jpegr_fill_input_buffer; skip_input_data = jpegr_skip_input_data; @@ -92,25 +92,18 @@ static void jpegrerror_exit(j_common_ptr cinfo) { longjmp(err->setjmp_buffer, 1); } -JpegDecoderHelper::JpegDecoderHelper() { -} +JpegDecoderHelper::JpegDecoderHelper() {} -JpegDecoderHelper::~JpegDecoderHelper() { -} +JpegDecoderHelper::~JpegDecoderHelper() {} bool JpegDecoderHelper::decompressImage(const void* image, int length, bool decodeToRGBA) { if (image == nullptr || length <= 0) { ALOGE("Image size can not be handled: %d", length); return false; } - mResultBuffer.clear(); mXMPBuffer.clear(); - if (!decode(image, length, decodeToRGBA)) { - return false; - } - - return true; + return decode(image, length, decodeToRGBA); } void* JpegDecoderHelper::getDecompressedImagePtr() { @@ -154,26 +147,28 @@ size_t JpegDecoderHelper::getDecompressedImageHeight() { } bool JpegDecoderHelper::decode(const void* image, int length, bool decodeToRGBA) { + bool status = true; jpeg_decompress_struct cinfo; - jpegr_source_mgr mgr(static_cast(image), length); jpegrerror_mgr myerr; - bool status = true; - cinfo.err = jpeg_std_error(&myerr.pub); myerr.pub.error_exit = jpegrerror_exit; - if (setjmp(myerr.setjmp_buffer)) { jpeg_destroy_decompress(&cinfo); return false; } + jpeg_create_decompress(&cinfo); jpeg_save_markers(&cinfo, kAPP0Marker, 0xFFFF); jpeg_save_markers(&cinfo, kAPP1Marker, 0xFFFF); jpeg_save_markers(&cinfo, kAPP2Marker, 0xFFFF); + jpegr_source_mgr mgr(static_cast(image), length); cinfo.src = &mgr; - jpeg_read_header(&cinfo, TRUE); + if (jpeg_read_header(&cinfo, TRUE) != JPEG_HEADER_OK) { + jpeg_destroy_decompress(&cinfo); + return false; + } // Save XMP data, EXIF data, and ICC data. // Here we only handle the first XMP / EXIF / ICC package. @@ -184,31 +179,24 @@ bool JpegDecoderHelper::decode(const void* image, int length, bool decodeToRGBA) bool xmpAppears = false; bool iccAppears = false; for (jpeg_marker_struct* marker = cinfo.marker_list; - marker && !(exifAppears && xmpAppears && iccAppears); - marker = marker->next) { - + marker && !(exifAppears && xmpAppears && iccAppears); marker = marker->next) { if (marker->marker != kAPP1Marker && marker->marker != kAPP2Marker) { continue; } const unsigned int len = marker->data_length; - if (!xmpAppears && - len > kXmpNameSpace.size() && - !strncmp(reinterpret_cast(marker->data), - kXmpNameSpace.c_str(), + if (!xmpAppears && len > kXmpNameSpace.size() && + !strncmp(reinterpret_cast(marker->data), kXmpNameSpace.c_str(), kXmpNameSpace.size())) { - mXMPBuffer.resize(len+1, 0); + mXMPBuffer.resize(len + 1, 0); memcpy(static_cast(mXMPBuffer.data()), marker->data, len); xmpAppears = true; - } else if (!exifAppears && - len > kExifIdCode.size() && - !strncmp(reinterpret_cast(marker->data), - kExifIdCode.c_str(), + } else if (!exifAppears && len > kExifIdCode.size() && + !strncmp(reinterpret_cast(marker->data), kExifIdCode.c_str(), kExifIdCode.size())) { mEXIFBuffer.resize(len, 0); memcpy(static_cast(mEXIFBuffer.data()), marker->data, len); exifAppears = true; - } else if (!iccAppears && - len > sizeof(kICCSig) && + } else if (!iccAppears && len > sizeof(kICCSig) && !memcmp(marker->data, kICCSig, sizeof(kICCSig))) { mICCBuffer.resize(len, 0); memcpy(static_cast(mICCBuffer.data()), marker->data, len); @@ -216,31 +204,25 @@ bool JpegDecoderHelper::decode(const void* image, int length, bool decodeToRGBA) } } - if (cinfo.image_width > kMaxWidth || cinfo.image_height > kMaxHeight) { - // constraint on max width and max height is only due to alloc constraints - // tune these values basing on the target device + mWidth = cinfo.image_width; + mHeight = cinfo.image_height; + if (mWidth > kMaxWidth || mHeight > kMaxHeight) { status = false; goto CleanUp; } - mWidth = cinfo.image_width; - mHeight = cinfo.image_height; - if (decodeToRGBA) { // The primary image is expected to be yuv420 sampling - if (cinfo.jpeg_color_space != JCS_YCbCr) { - status = false; - ALOGE("%s: decodeToRGBA unexpected jpeg color space ", __func__); - goto CleanUp; - } - if (cinfo.comp_info[0].h_samp_factor != 2 || - cinfo.comp_info[1].h_samp_factor != 1 || - cinfo.comp_info[2].h_samp_factor != 1 || - cinfo.comp_info[0].v_samp_factor != 2 || - cinfo.comp_info[1].v_samp_factor != 1 || - cinfo.comp_info[2].v_samp_factor != 1 ) { - status = false; - ALOGE("%s: decodeToRGBA unexpected primary image sub-sampling", __func__); + if (cinfo.jpeg_color_space != JCS_YCbCr) { + status = false; + ALOGE("%s: decodeToRGBA unexpected jpeg color space ", __func__); + goto CleanUp; + } + if (cinfo.comp_info[0].h_samp_factor != 2 || cinfo.comp_info[0].v_samp_factor != 2 || + cinfo.comp_info[1].h_samp_factor != 1 || cinfo.comp_info[1].v_samp_factor != 1 || + cinfo.comp_info[2].h_samp_factor != 1 || cinfo.comp_info[2].v_samp_factor != 1) { + status = false; + ALOGE("%s: decodeToRGBA unexpected primary image sub-sampling", __func__); goto CleanUp; } // 4 bytes per pixel @@ -248,12 +230,9 @@ bool JpegDecoderHelper::decode(const void* image, int length, bool decodeToRGBA) cinfo.out_color_space = JCS_EXT_RGBA; } else { if (cinfo.jpeg_color_space == JCS_YCbCr) { - if (cinfo.comp_info[0].h_samp_factor != 2 || - cinfo.comp_info[1].h_samp_factor != 1 || - cinfo.comp_info[2].h_samp_factor != 1 || - cinfo.comp_info[0].v_samp_factor != 2 || - cinfo.comp_info[1].v_samp_factor != 1 || - cinfo.comp_info[2].v_samp_factor != 1) { + if (cinfo.comp_info[0].h_samp_factor != 2 || cinfo.comp_info[0].v_samp_factor != 2 || + cinfo.comp_info[1].h_samp_factor != 1 || cinfo.comp_info[1].v_samp_factor != 1 || + cinfo.comp_info[2].h_samp_factor != 1 || cinfo.comp_info[2].v_samp_factor != 1) { status = false; ALOGE("%s: decoding to YUV only supports 4:2:0 subsampling", __func__); goto CleanUp; @@ -271,11 +250,9 @@ bool JpegDecoderHelper::decode(const void* image, int length, bool decodeToRGBA) } cinfo.dct_method = JDCT_ISLOW; - jpeg_start_decompress(&cinfo); - if (!decompress(&cinfo, static_cast(mResultBuffer.data()), - cinfo.jpeg_color_space == JCS_GRAYSCALE)) { + cinfo.jpeg_color_space == JCS_GRAYSCALE)) { status = false; goto CleanUp; } @@ -288,25 +265,20 @@ CleanUp: } bool JpegDecoderHelper::decompress(jpeg_decompress_struct* cinfo, const uint8_t* dest, - bool isSingleChannel) { - if (isSingleChannel) { - return decompressSingleChannel(cinfo, dest); - } - if (cinfo->out_color_space == JCS_EXT_RGBA) - return decompressRGBA(cinfo, dest); - else - return decompressYUV(cinfo, dest); + bool isSingleChannel) { + return isSingleChannel + ? decompressSingleChannel(cinfo, dest) + : ((cinfo->out_color_space == JCS_EXT_RGBA) ? decompressRGBA(cinfo, dest) + : decompressYUV(cinfo, dest)); } -bool JpegDecoderHelper::getCompressedImageParameters(const void* image, int length, - size_t *pWidth, size_t *pHeight, - std::vector *iccData , std::vector *exifData) { +bool JpegDecoderHelper::getCompressedImageParameters(const void* image, int length, size_t* pWidth, + size_t* pHeight, std::vector* iccData, + std::vector* exifData) { jpeg_decompress_struct cinfo; - jpegr_source_mgr mgr(static_cast(image), length); jpegrerror_mgr myerr; cinfo.err = jpeg_std_error(&myerr.pub); myerr.pub.error_exit = jpegrerror_exit; - if (setjmp(myerr.setjmp_buffer)) { jpeg_destroy_decompress(&cinfo); return false; @@ -316,6 +288,7 @@ bool JpegDecoderHelper::getCompressedImageParameters(const void* image, int leng jpeg_save_markers(&cinfo, kAPP1Marker, 0xFFFF); jpeg_save_markers(&cinfo, kAPP2Marker, 0xFFFF); + jpegr_source_mgr mgr(static_cast(image), length); cinfo.src = &mgr; if (jpeg_read_header(&cinfo, TRUE) != JPEG_HEADER_OK) { jpeg_destroy_decompress(&cinfo); @@ -330,8 +303,7 @@ bool JpegDecoderHelper::getCompressedImageParameters(const void* image, int leng } if (iccData != nullptr) { - for (jpeg_marker_struct* marker = cinfo.marker_list; marker; - marker = marker->next) { + for (jpeg_marker_struct* marker = cinfo.marker_list; marker; marker = marker->next) { if (marker->marker != kAPP2Marker) { continue; } @@ -368,25 +340,20 @@ bool JpegDecoderHelper::getCompressedImageParameters(const void* image, int leng } bool JpegDecoderHelper::decompressRGBA(jpeg_decompress_struct* cinfo, const uint8_t* dest) { - JSAMPLE* decodeDst = (JSAMPLE*) dest; - uint32_t lines = 0; - // TODO: use batches for more effectiveness - while (lines < cinfo->image_height) { - uint32_t ret = jpeg_read_scanlines(cinfo, &decodeDst, 1); - if (ret == 0) { - break; - } - decodeDst += cinfo->image_width * 4; - lines++; + JSAMPLE* out = (JSAMPLE*)dest; + + while (cinfo->output_scanline < cinfo->image_height) { + if (1 != jpeg_read_scanlines(cinfo, &out, 1)) return false; + out += cinfo->image_width * 4; } - return lines == cinfo->image_height; + return true; } bool JpegDecoderHelper::decompressYUV(jpeg_decompress_struct* cinfo, const uint8_t* dest) { JSAMPROW y[kCompressBatchSize]; JSAMPROW cb[kCompressBatchSize / 2]; JSAMPROW cr[kCompressBatchSize / 2]; - JSAMPARRAY planes[3] {y, cb, cr}; + JSAMPARRAY planes[3]{y, cb, cr}; size_t y_plane_size = cinfo->image_width * cinfo->image_height; size_t uv_plane_size = y_plane_size / 4; @@ -405,7 +372,7 @@ bool JpegDecoderHelper::decompressYUV(jpeg_decompress_struct* cinfo, const uint8 JSAMPROW y_intrm[kCompressBatchSize]; JSAMPROW cb_intrm[kCompressBatchSize / 2]; JSAMPROW cr_intrm[kCompressBatchSize / 2]; - JSAMPARRAY planes_intrm[3] {y_intrm, cb_intrm, cr_intrm}; + JSAMPARRAY planes_intrm[3]{y_intrm, cb_intrm, cr_intrm}; if (!is_width_aligned) { size_t mcu_row_size = aligned_width * kCompressBatchSize * 3 / 2; buffer_intrm = std::make_unique(mcu_row_size); @@ -462,9 +429,10 @@ bool JpegDecoderHelper::decompressYUV(jpeg_decompress_struct* cinfo, const uint8 return true; } -bool JpegDecoderHelper::decompressSingleChannel(jpeg_decompress_struct* cinfo, const uint8_t* dest) { +bool JpegDecoderHelper::decompressSingleChannel(jpeg_decompress_struct* cinfo, + const uint8_t* dest) { JSAMPROW y[kCompressBatchSize]; - JSAMPARRAY planes[1] {y}; + JSAMPARRAY planes[1]{y}; uint8_t* y_plane = const_cast(dest); std::unique_ptr empty = std::make_unique(cinfo->image_width); @@ -475,7 +443,7 @@ bool JpegDecoderHelper::decompressSingleChannel(jpeg_decompress_struct* cinfo, c std::unique_ptr buffer_intrm = nullptr; uint8_t* y_plane_intrm = nullptr; JSAMPROW y_intrm[kCompressBatchSize]; - JSAMPARRAY planes_intrm[1] {y_intrm}; + JSAMPARRAY planes_intrm[1]{y_intrm}; if (!is_width_aligned) { size_t mcu_row_size = aligned_width * kCompressBatchSize; buffer_intrm = std::make_unique(mcu_row_size); @@ -510,4 +478,4 @@ bool JpegDecoderHelper::decompressSingleChannel(jpeg_decompress_struct* cinfo, c return true; } -} // namespace ultrahdr +} // namespace android::ultrahdr diff --git a/libs/ultrahdr/tests/Android.bp b/libs/ultrahdr/tests/Android.bp index 004a582fd6..bda804ae31 100644 --- a/libs/ultrahdr/tests/Android.bp +++ b/libs/ultrahdr/tests/Android.bp @@ -28,6 +28,8 @@ cc_test { "gainmapmath_test.cpp", "icchelper_test.cpp", "jpegr_test.cpp", + "jpegencoderhelper_test.cpp", + "jpegdecoderhelper_test.cpp", ], shared_libs: [ "libimage_io", @@ -42,38 +44,7 @@ cc_test { "libultrahdr", "libutils", ], -} - -cc_test { - name: "jpegencoderhelper_test", - test_suites: ["device-tests"], - srcs: [ - "jpegencoderhelper_test.cpp", - ], - shared_libs: [ - "libjpeg", - "liblog", - ], - static_libs: [ - "libgtest", - "libjpegencoder", - ], -} - -cc_test { - name: "jpegdecoderhelper_test", - test_suites: ["device-tests"], - srcs: [ - "jpegdecoderhelper_test.cpp", - ], - shared_libs: [ - "libjpeg", - "liblog", - ], - static_libs: [ - "libgtest", - "libjpegdecoder", - "libultrahdr", - "libutils", + data: [ + "./data/*.*", ], } diff --git a/libs/ultrahdr/tests/AndroidTest.xml b/libs/ultrahdr/tests/AndroidTest.xml new file mode 100644 index 0000000000..1754a5ccb9 --- /dev/null +++ b/libs/ultrahdr/tests/AndroidTest.xml @@ -0,0 +1,26 @@ + + + + + + + + diff --git a/libs/ultrahdr/tests/jpegdecoderhelper_test.cpp b/libs/ultrahdr/tests/jpegdecoderhelper_test.cpp index e2da01c373..af0d59edc0 100644 --- a/libs/ultrahdr/tests/jpegdecoderhelper_test.cpp +++ b/libs/ultrahdr/tests/jpegdecoderhelper_test.cpp @@ -14,9 +14,9 @@ * limitations under the License. */ -#include -#include #include +#include +#include #include #include @@ -24,13 +24,13 @@ namespace android::ultrahdr { // No ICC or EXIF -#define YUV_IMAGE "/sdcard/Documents/minnie-320x240-yuv.jpg" +#define YUV_IMAGE "/data/local/tmp/minnie-320x240-yuv.jpg" #define YUV_IMAGE_SIZE 20193 // Has ICC and EXIF -#define YUV_ICC_IMAGE "/sdcard/Documents/minnie-320x240-yuv-icc.jpg" +#define YUV_ICC_IMAGE "/data/local/tmp/minnie-320x240-yuv-icc.jpg" #define YUV_ICC_IMAGE_SIZE 34266 // No ICC or EXIF -#define GREY_IMAGE "/sdcard/Documents/minnie-320x240-y.jpg" +#define GREY_IMAGE "/data/local/tmp/minnie-320x240-y.jpg" #define GREY_IMAGE_SIZE 20193 #define IMAGE_WIDTH 320 @@ -44,6 +44,7 @@ public: }; JpegDecoderHelperTest(); ~JpegDecoderHelperTest(); + protected: virtual void SetUp(); virtual void TearDown(); @@ -127,8 +128,8 @@ TEST_F(JpegDecoderHelperTest, getCompressedImageParameters) { std::vector icc, exif; JpegDecoderHelper decoder; - EXPECT_TRUE(decoder.getCompressedImageParameters(mYuvImage.buffer.get(), mYuvImage.size, - &width, &height, &icc, &exif)); + EXPECT_TRUE(decoder.getCompressedImageParameters(mYuvImage.buffer.get(), mYuvImage.size, &width, + &height, &icc, &exif)); EXPECT_EQ(width, IMAGE_WIDTH); EXPECT_EQ(height, IMAGE_HEIGHT); @@ -149,8 +150,7 @@ TEST_F(JpegDecoderHelperTest, getCompressedImageParametersIcc) { EXPECT_GT(icc.size(), 0); EXPECT_GT(exif.size(), 0); - EXPECT_EQ(IccHelper::readIccColorGamut(icc.data(), icc.size()), - ULTRAHDR_COLORGAMUT_BT709); + EXPECT_EQ(IccHelper::readIccColorGamut(icc.data(), icc.size()), ULTRAHDR_COLORGAMUT_BT709); } -} // namespace android::ultrahdr +} // namespace android::ultrahdr diff --git a/libs/ultrahdr/tests/jpegencoderhelper_test.cpp b/libs/ultrahdr/tests/jpegencoderhelper_test.cpp index 33cb9f658f..af54eb2a8a 100644 --- a/libs/ultrahdr/tests/jpegencoderhelper_test.cpp +++ b/libs/ultrahdr/tests/jpegencoderhelper_test.cpp @@ -22,13 +22,13 @@ namespace android::ultrahdr { -#define ALIGNED_IMAGE "/sdcard/Documents/minnie-320x240.yu12" +#define ALIGNED_IMAGE "/data/local/tmp/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 "/data/local/tmp/minnie-320x240.y" #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 "/data/local/tmp/minnie-318x240.yu12" #define UNALIGNED_IMAGE_WIDTH 318 #define UNALIGNED_IMAGE_HEIGHT 240 #define JPEG_QUALITY 90 diff --git a/libs/ultrahdr/tests/jpegr_test.cpp b/libs/ultrahdr/tests/jpegr_test.cpp index a75086755a..5fa758e88d 100644 --- a/libs/ultrahdr/tests/jpegr_test.cpp +++ b/libs/ultrahdr/tests/jpegr_test.cpp @@ -30,9 +30,9 @@ namespace android::ultrahdr { // resources used by unit tests -const char* kYCbCrP010FileName = "raw_p010_image.p010"; -const char* kYCbCr420FileName = "raw_yuv420_image.yuv420"; -const char* kSdrJpgFileName = "jpeg_image.jpg"; +const char* kYCbCrP010FileName = "/data/local/tmp/raw_p010_image.p010"; +const char* kYCbCr420FileName = "/data/local/tmp/raw_yuv420_image.yuv420"; +const char* kSdrJpgFileName = "/data/local/tmp/jpeg_image.jpg"; const int kImageWidth = 1280; const int kImageHeight = 720; const int kQuality = 90; -- cgit v1.2.3-59-g8ed1b From 6b3cd9ac6ba7568c29570d492c1fedc980fba07f Mon Sep 17 00:00:00 2001 From: Dichen Zhang Date: Fri, 18 Aug 2023 01:48:10 +0000 Subject: UltraHDR: reorder MPF and EXIF For encode API-2, 3 and 4 where input has a JPEG, we always want the EXIF package appears in front of MPF. This change also fixed "double null terminator" string problem in the decoder helper. Bug: 292561235 Test: jpegr_test.cpp Change-Id: If6890cc499e179ba01c4e7775796111edb7a5d2f --- libs/ultrahdr/include/ultrahdr/jpegdecoderhelper.h | 17 +++- libs/ultrahdr/include/ultrahdr/jpegr.h | 16 ++-- libs/ultrahdr/jpegdecoderhelper.cpp | 90 ++++++++++++++++++---- libs/ultrahdr/jpegr.cpp | 80 ++++++++++++++++--- 4 files changed, 169 insertions(+), 34 deletions(-) (limited to 'libs/ultrahdr/jpegdecoderhelper.cpp') diff --git a/libs/ultrahdr/include/ultrahdr/jpegdecoderhelper.h b/libs/ultrahdr/include/ultrahdr/jpegdecoderhelper.h index 1f0c38ff7f..30149c11f0 100644 --- a/libs/ultrahdr/include/ultrahdr/jpegdecoderhelper.h +++ b/libs/ultrahdr/include/ultrahdr/jpegdecoderhelper.h @@ -75,15 +75,27 @@ public: * calling decompressImage() or getCompressedImageParameters(). */ size_t getXMPSize(); + /* + * Extracts EXIF package and updates the EXIF position / length without decoding the image. + */ + bool extractEXIF(const void* image, int length); /* * Returns the EXIF data from the image. + * This method must be called after extractEXIF() or decompressImage(). */ void* getEXIFPtr(); /* * Returns the decompressed EXIF buffer size. This method must be called only after - * calling decompressImage() or getCompressedImageParameters(). + * calling decompressImage(), extractEXIF() or getCompressedImageParameters(). */ size_t getEXIFSize(); + /* + * Returns the position offset of EXIF package + * (4 bypes offset to FF sign, the byte after FF E1 XX XX ), + * or -1 if no EXIF exists. + * This method must be called after extractEXIF() or decompressImage(). + */ + int getEXIFPos() { return mExifPos; } /* * Returns the ICC data from the image. */ @@ -122,6 +134,9 @@ private: // Resolution of the decompressed image. size_t mWidth; size_t mHeight; + + // Position of EXIF package, default value is -1 which means no EXIF package appears. + size_t mExifPos; }; } /* namespace android::ultrahdr */ diff --git a/libs/ultrahdr/include/ultrahdr/jpegr.h b/libs/ultrahdr/include/ultrahdr/jpegr.h index 850cb32e75..114c81d818 100644 --- a/libs/ultrahdr/include/ultrahdr/jpegr.h +++ b/libs/ultrahdr/include/ultrahdr/jpegr.h @@ -366,22 +366,24 @@ private: * the compressed gain map and optionally the exif package as inputs, and generate the XMP * metadata, and finally append everything in the order of: * SOI, APP2(EXIF) (if EXIF is from outside), APP2(XMP), primary image, gain map - * Note that EXIF package is only available for encoding API-0 and API-1. For encoding API-2 and - * API-3 this parameter is null, but the primary image in JPEG/R may still have EXIF as long as - * the input JPEG has EXIF. * - + * Note that in the final JPEG/R output, EXIF package will appear if ONLY ONE of the following + * conditions is fulfilled: + * (1) EXIF package is available from outside input. I.e. pExif != nullptr. + * (2) Input JPEG has EXIF. + * If both conditions are fulfilled, this method will return ERROR_JPEGR_INVALID_INPUT_TYPE + * * @param primary_jpg_image_ptr destination of primary image * @param gainmap_jpg_image_ptr destination of compressed gain map image - * @param (nullable) exif EXIF package - * @param (nullable) icc ICC package + * @param (nullable) pExif EXIF package + * @param (nullable) pIcc ICC package * @param icc_size length in bytes of ICC package * @param metadata JPEG/R metadata to encode in XMP of the jpeg * @param dest compressed JPEGR image * @return NO_ERROR if calculation succeeds, error code if error occurs. */ status_t appendGainMap(jr_compressed_ptr primary_jpg_image_ptr, - jr_compressed_ptr gainmap_jpg_image_ptr, jr_exif_ptr exif, void* icc, + jr_compressed_ptr gainmap_jpg_image_ptr, jr_exif_ptr pExif, void* pIcc, size_t icc_size, ultrahdr_metadata_ptr metadata, jr_compressed_ptr dest); /* diff --git a/libs/ultrahdr/jpegdecoderhelper.cpp b/libs/ultrahdr/jpegdecoderhelper.cpp index 7ccf1f3697..2e7940cc2c 100644 --- a/libs/ultrahdr/jpegdecoderhelper.cpp +++ b/libs/ultrahdr/jpegdecoderhelper.cpp @@ -32,12 +32,17 @@ const uint32_t kAPP0Marker = JPEG_APP0; // JFIF const uint32_t kAPP1Marker = JPEG_APP0 + 1; // EXIF, XMP const uint32_t kAPP2Marker = JPEG_APP0 + 2; // ICC -const std::string kXmpNameSpace = "http://ns.adobe.com/xap/1.0/"; -const std::string kExifIdCode = "Exif"; constexpr uint32_t kICCMarkerHeaderSize = 14; constexpr uint8_t kICCSig[] = { 'I', 'C', 'C', '_', 'P', 'R', 'O', 'F', 'I', 'L', 'E', '\0', }; +constexpr uint8_t kXmpNameSpace[] = { + 'h', 't', 't', 'p', ':', '/', '/', 'n', 's', '.', 'a', 'd', 'o', 'b', 'e', + '.', 'c', 'o', 'm', '/', 'x', 'a', 'p', '/', '1', '.', '0', '/', '\0', +}; +constexpr uint8_t kExifIdCode[] = { + 'E', 'x', 'i', 'f', '\0', '\0', +}; struct jpegr_source_mgr : jpeg_source_mgr { jpegr_source_mgr(const uint8_t* ptr, int len); @@ -146,6 +151,58 @@ size_t JpegDecoderHelper::getDecompressedImageHeight() { return mHeight; } +// Here we only handle the first EXIF package, and in theary EXIF (or JFIF) must be the first +// in the image file. +// We assume that all packages are starting with two bytes marker (eg FF E1 for EXIF package), +// two bytes of package length which is stored in marker->original_length, and the real data +// which is stored in marker->data. +bool JpegDecoderHelper::extractEXIF(const void* image, int length) { + jpeg_decompress_struct cinfo; + jpegr_source_mgr mgr(static_cast(image), length); + jpegrerror_mgr myerr; + + cinfo.err = jpeg_std_error(&myerr.pub); + myerr.pub.error_exit = jpegrerror_exit; + + if (setjmp(myerr.setjmp_buffer)) { + jpeg_destroy_decompress(&cinfo); + return false; + } + jpeg_create_decompress(&cinfo); + + jpeg_save_markers(&cinfo, kAPP0Marker, 0xFFFF); + jpeg_save_markers(&cinfo, kAPP1Marker, 0xFFFF); + + cinfo.src = &mgr; + jpeg_read_header(&cinfo, TRUE); + + size_t pos = 2; // position after SOI + for (jpeg_marker_struct* marker = cinfo.marker_list; + marker; + marker = marker->next) { + + pos += 4; + pos += marker->original_length; + + if (marker->marker != kAPP1Marker) { + continue; + } + + const unsigned int len = marker->data_length; + + if (len > sizeof(kExifIdCode) && + !memcmp(marker->data, kExifIdCode, sizeof(kExifIdCode))) { + mEXIFBuffer.resize(len, 0); + memcpy(static_cast(mEXIFBuffer.data()), marker->data, len); + mExifPos = pos - marker->original_length; + break; + } + } + + jpeg_destroy_decompress(&cinfo); + return true; +} + bool JpegDecoderHelper::decode(const void* image, int length, bool decodeToRGBA) { bool status = true; jpeg_decompress_struct cinfo; @@ -178,25 +235,31 @@ bool JpegDecoderHelper::decode(const void* image, int length, bool decodeToRGBA) bool exifAppears = false; bool xmpAppears = false; bool iccAppears = false; + size_t pos = 2; // position after SOI for (jpeg_marker_struct* marker = cinfo.marker_list; - marker && !(exifAppears && xmpAppears && iccAppears); marker = marker->next) { + marker && !(exifAppears && xmpAppears && iccAppears); + marker = marker->next) { + pos += 4; + pos += marker->original_length; if (marker->marker != kAPP1Marker && marker->marker != kAPP2Marker) { continue; } const unsigned int len = marker->data_length; - if (!xmpAppears && len > kXmpNameSpace.size() && - !strncmp(reinterpret_cast(marker->data), kXmpNameSpace.c_str(), - kXmpNameSpace.size())) { - mXMPBuffer.resize(len + 1, 0); + if (!xmpAppears && + len > sizeof(kXmpNameSpace) && + !memcmp(marker->data, kXmpNameSpace, sizeof(kXmpNameSpace))) { + mXMPBuffer.resize(len+1, 0); memcpy(static_cast(mXMPBuffer.data()), marker->data, len); xmpAppears = true; - } else if (!exifAppears && len > kExifIdCode.size() && - !strncmp(reinterpret_cast(marker->data), kExifIdCode.c_str(), - kExifIdCode.size())) { + } else if (!exifAppears && + len > sizeof(kExifIdCode) && + !memcmp(marker->data, kExifIdCode, sizeof(kExifIdCode))) { mEXIFBuffer.resize(len, 0); memcpy(static_cast(mEXIFBuffer.data()), marker->data, len); exifAppears = true; - } else if (!iccAppears && len > sizeof(kICCSig) && + mExifPos = pos - marker->original_length; + } else if (!iccAppears && + len > sizeof(kICCSig) && !memcmp(marker->data, kICCSig, sizeof(kICCSig))) { mICCBuffer.resize(len, 0); memcpy(static_cast(mICCBuffer.data()), marker->data, len); @@ -325,9 +388,8 @@ bool JpegDecoderHelper::getCompressedImageParameters(const void* image, int leng } const unsigned int len = marker->data_length; - if (len >= kExifIdCode.size() && - !strncmp(reinterpret_cast(marker->data), kExifIdCode.c_str(), - kExifIdCode.size())) { + if (len >= sizeof(kExifIdCode) && + !memcmp(marker->data, kExifIdCode, sizeof(kExifIdCode))) { exifData->resize(len, 0); memcpy(static_cast(exifData->data()), marker->data, len); exifAppears = true; diff --git a/libs/ultrahdr/jpegr.cpp b/libs/ultrahdr/jpegr.cpp index f27bd2d774..74760d9b32 100644 --- a/libs/ultrahdr/jpegr.cpp +++ b/libs/ultrahdr/jpegr.cpp @@ -72,6 +72,32 @@ int GetCPUCoreCount() { return cpuCoreCount; } +/* + * Helper function copies the JPEG image from without EXIF. + * + * @param pDest destination of the data to be written. + * @param pSource source of data being written. + * @param exif_pos position of the EXIF package, which is aligned with jpegdecoder.getEXIFPos(). + * (4 bytes offset to FF sign, the byte after FF E1 XX XX ). + * @param exif_size exif size without the initial 4 bytes, aligned with jpegdecoder.getEXIFSize(). + */ +static void copyJpegWithoutExif(jr_compressed_ptr pDest, + jr_compressed_ptr pSource, + size_t exif_pos, + size_t exif_size) { + memcpy(pDest, pSource, sizeof(jpegr_compressed_struct)); + + const size_t exif_offset = 4; //exif_pos has 4 bytes offset to the FF sign + pDest->length = pSource->length - exif_size - exif_offset; + pDest->data = new uint8_t[pDest->length]; + std::unique_ptr dest_data; + dest_data.reset(reinterpret_cast(pDest->data)); + memcpy(pDest->data, pSource->data, exif_pos - exif_offset); + memcpy((uint8_t*)pDest->data + exif_pos - exif_offset, + (uint8_t*)pSource->data + exif_pos + exif_size, + pSource->length - exif_pos - exif_size); +} + status_t JpegR::areInputArgumentsValid(jr_uncompressed_ptr p010_image_ptr, jr_uncompressed_ptr yuv420_image_ptr, ultrahdr_transfer_function hdr_tf, @@ -1152,7 +1178,8 @@ status_t JpegR::extractPrimaryImageAndGainMap(jr_compressed_ptr jpegr_image_ptr, // JPEG/R structure: // SOI (ff d8) // -// (Optional, only if EXIF package is from outside) +// (Optional, if EXIF package is from outside (Encode API-0 API-1), or if EXIF package presents +// in the JPEG input (Encode API-2, API-3, API-4)) // APP1 (ff e1) // 2 bytes of length (2 + length of exif package) // EXIF package (this includes the first two bytes representing the package length) @@ -1166,7 +1193,7 @@ status_t JpegR::extractPrimaryImageAndGainMap(jr_compressed_ptr jpegr_image_ptr, // 2 bytes of length // MPF // -// (Required) primary image (without the first two bytes (SOI), may have other packages) +// (Required) primary image (without the first two bytes (SOI) and EXIF, may have other packages) // // SOI (ff d8) // @@ -1183,8 +1210,8 @@ status_t JpegR::extractPrimaryImageAndGainMap(jr_compressed_ptr jpegr_image_ptr, // Adobe XMP spec part 3 for XMP marker // ICC v4.3 spec for ICC status_t JpegR::appendGainMap(jr_compressed_ptr primary_jpg_image_ptr, - jr_compressed_ptr gainmap_jpg_image_ptr, jr_exif_ptr exif, void* icc, - size_t icc_size, ultrahdr_metadata_ptr metadata, + jr_compressed_ptr gainmap_jpg_image_ptr, jr_exif_ptr pExif, + void* pIcc, size_t icc_size, ultrahdr_metadata_ptr metadata, jr_compressed_ptr dest) { if (primary_jpg_image_ptr == nullptr || gainmap_jpg_image_ptr == nullptr || metadata == nullptr || dest == nullptr) { @@ -1229,6 +1256,35 @@ status_t JpegR::appendGainMap(jr_compressed_ptr primary_jpg_image_ptr, // same as primary const int xmp_primary_length = 2 + nameSpaceLength + xmp_primary.size(); + // Check if EXIF package presents in the JPEG input. + // If so, extract and remove the EXIF package. + JpegDecoderHelper decoder; + if (!decoder.extractEXIF(primary_jpg_image_ptr->data, primary_jpg_image_ptr->length)) { + return ERROR_JPEGR_DECODE_ERROR; + } + jpegr_exif_struct exif_from_jpg; + exif_from_jpg.data = nullptr; + exif_from_jpg.length = 0; + jpegr_compressed_struct new_jpg_image; + new_jpg_image.data = nullptr; + new_jpg_image.length = 0; + if (decoder.getEXIFPos() != 0) { + if (pExif != nullptr) { + ALOGE("received EXIF from outside while the primary image already contains EXIF"); + return ERROR_JPEGR_INVALID_INPUT_TYPE; + } + copyJpegWithoutExif(&new_jpg_image, + primary_jpg_image_ptr, + decoder.getEXIFPos(), + decoder.getEXIFSize()); + exif_from_jpg.data = decoder.getEXIFPtr(); + exif_from_jpg.length = decoder.getEXIFSize(); + pExif = &exif_from_jpg; + } + + jr_compressed_ptr final_primary_jpg_image_ptr = + new_jpg_image.length == 0 ? primary_jpg_image_ptr : &new_jpg_image; + int pos = 0; // Begin primary image // Write SOI @@ -1236,15 +1292,15 @@ status_t JpegR::appendGainMap(jr_compressed_ptr primary_jpg_image_ptr, JPEGR_CHECK(Write(dest, &photos_editing_formats::image_io::JpegMarker::kSOI, 1, pos)); // Write EXIF - if (exif != nullptr) { - const int length = 2 + exif->length; + if (pExif != nullptr) { + const int length = 2 + pExif->length; const uint8_t lengthH = ((length >> 8) & 0xff); const uint8_t lengthL = (length & 0xff); JPEGR_CHECK(Write(dest, &photos_editing_formats::image_io::JpegMarker::kStart, 1, pos)); JPEGR_CHECK(Write(dest, &photos_editing_formats::image_io::JpegMarker::kAPP1, 1, pos)); JPEGR_CHECK(Write(dest, &lengthH, 1, pos)); JPEGR_CHECK(Write(dest, &lengthL, 1, pos)); - JPEGR_CHECK(Write(dest, exif->data, exif->length, pos)); + JPEGR_CHECK(Write(dest, pExif->data, pExif->length, pos)); } // Prepare and write XMP @@ -1261,7 +1317,7 @@ status_t JpegR::appendGainMap(jr_compressed_ptr primary_jpg_image_ptr, } // Write ICC - if (icc != nullptr && icc_size > 0) { + if (pIcc != nullptr && icc_size > 0) { const int length = icc_size + 2; const uint8_t lengthH = ((length >> 8) & 0xff); const uint8_t lengthL = (length & 0xff); @@ -1269,7 +1325,7 @@ status_t JpegR::appendGainMap(jr_compressed_ptr primary_jpg_image_ptr, JPEGR_CHECK(Write(dest, &photos_editing_formats::image_io::JpegMarker::kAPP2, 1, pos)); JPEGR_CHECK(Write(dest, &lengthH, 1, pos)); JPEGR_CHECK(Write(dest, &lengthL, 1, pos)); - JPEGR_CHECK(Write(dest, icc, icc_size, pos)); + JPEGR_CHECK(Write(dest, pIcc, icc_size, pos)); } // Prepare and write MPF @@ -1277,7 +1333,7 @@ status_t JpegR::appendGainMap(jr_compressed_ptr primary_jpg_image_ptr, const int length = 2 + calculateMpfSize(); const uint8_t lengthH = ((length >> 8) & 0xff); const uint8_t lengthL = (length & 0xff); - int primary_image_size = pos + length + primary_jpg_image_ptr->length; + int primary_image_size = pos + length + final_primary_jpg_image_ptr->length; // between APP2 + package size + signature // ff e2 00 58 4d 50 46 00 // 2 + 2 + 4 = 8 (bytes) @@ -1293,8 +1349,8 @@ status_t JpegR::appendGainMap(jr_compressed_ptr primary_jpg_image_ptr, } // Write primary image - JPEGR_CHECK(Write(dest, (uint8_t*)primary_jpg_image_ptr->data + 2, - primary_jpg_image_ptr->length - 2, pos)); + JPEGR_CHECK(Write(dest, (uint8_t*)final_primary_jpg_image_ptr->data + 2, + final_primary_jpg_image_ptr->length - 2, pos)); // Finish primary image // Begin secondary image (gain map) -- cgit v1.2.3-59-g8ed1b