diff --git a/lib/src/jpegdecoderhelper.cpp b/lib/src/jpegdecoderhelper.cpp index c23b88e2..82d8da32 100644 --- a/lib/src/jpegdecoderhelper.cpp +++ b/lib/src/jpegdecoderhelper.cpp @@ -139,6 +139,39 @@ static void jpeg_extract_marker_payload(const j_decompress_ptr cinfo, const uint } } +// Locate the selected payload in the original JPEG stream. The saved-marker list is incomplete +// by design and is not a reliable source of byte offsets when other segments precede EXIF. +static long find_marker_payload_offset(const uint8_t* image, size_t length, uint32_t marker_code, + const uint8_t* payload, size_t payload_length) { + if (image == nullptr || payload == nullptr || length < 2 || image[0] != 0xff || + image[1] != 0xd8) { + return -1; + } + + size_t pos = 2; + while (pos + 1 < length) { + if (image[pos] != 0xff) return -1; + while (pos < length && image[pos] == 0xff) ++pos; + if (pos >= length) return -1; + + const uint8_t marker = image[pos++]; + if (marker == 0xd9 || marker == 0xda) return -1; + if (marker == 0xd8 || marker == 0x01 || (marker >= 0xd0 && marker <= 0xd7)) continue; + if (pos + 2 > length) return -1; + + const size_t segment_length = (static_cast(image[pos]) << 8) | image[pos + 1]; + if (segment_length < 2 || segment_length > length - pos) return -1; + const size_t segment_payload_length = segment_length - 2; + const size_t segment_payload_offset = pos + 2; + if (marker == marker_code && segment_payload_length == payload_length && + memcmp(image + segment_payload_offset, payload, payload_length) == 0) { + return static_cast(segment_payload_offset); + } + pos += segment_length; + } + return -1; +} + static uhdr_img_fmt_t getOutputSamplingFormat(const j_decompress_ptr cinfo) { if (cinfo->num_components == 1) return UHDR_IMG_FMT_8bppYCbCr400; @@ -238,6 +271,19 @@ uhdr_error_info_t JpegDecoderHelper::decode(const void* image, size_t length, de jpeg_extract_marker_payload(&cinfo, kAPP1Marker, kExifIdCode, sizeof kExifIdCode / sizeof kExifIdCode[0], mEXIFBuffer, mExifPayLoadOffset); + if (mExifPayLoadOffset >= 0) { + mExifPayLoadOffset = find_marker_payload_offset( + static_cast(image), length, kAPP1Marker, mEXIFBuffer.data(), + mEXIFBuffer.size()); + if (mExifPayLoadOffset < 0) { + status.error_code = UHDR_CODEC_ERROR; + status.has_detail = 1; + snprintf(status.detail, sizeof status.detail, + "failed to locate extracted EXIF payload in JPEG source"); + jpeg_destroy_decompress(&cinfo); + return status; + } + } jpeg_extract_marker_payload(&cinfo, kAPP2Marker, kICCSig, sizeof kICCSig / sizeof kICCSig[0], mICCBuffer, payloadOffset); jpeg_extract_marker_payload(&cinfo, kAPP2Marker, kIsoMetadataNameSpace, diff --git a/lib/src/jpegr.cpp b/lib/src/jpegr.cpp index 4a5ba667..4ed12ff0 100644 --- a/lib/src/jpegr.cpp +++ b/lib/src/jpegr.cpp @@ -161,9 +161,14 @@ UltraHdr::UltraHdr(void* uhdrGLESCtxt, int mapDimensionScaleFactor, int mapCompr * (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(uhdr_compressed_image_t* pDest, uhdr_compressed_image_t* pSource, +static bool copyJpegWithoutExif(uhdr_compressed_image_t* pDest, uhdr_compressed_image_t* pSource, size_t exif_pos, size_t exif_size) { const size_t exif_offset = 4; // exif_pos has 4 bytes offset to the FF sign + if (pDest == nullptr || pSource == nullptr || pSource->data == nullptr || exif_pos < exif_offset || + exif_pos > pSource->data_sz || exif_size > pSource->data_sz - exif_pos || + exif_offset > pSource->data_sz - exif_size) { + return false; + } pDest->data_sz = pSource->data_sz - exif_size - exif_offset; pDest->data = new uint8_t[pDest->data_sz]; pDest->capacity = pDest->data_sz; @@ -173,6 +178,7 @@ static void copyJpegWithoutExif(uhdr_compressed_image_t* pDest, uhdr_compressed_ 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->data_sz - exif_pos - exif_size); + return true; } /* Encode API-0 */ @@ -1197,8 +1203,15 @@ uhdr_error_info_t JpegR::appendGainMap(uhdr_compressed_image_t* sdr_intent_compr "contains exif, unsure which one to use"); return status; } - copyJpegWithoutExif(&new_jpg_image, sdr_intent_compressed, decoder.getEXIFPos(), - decoder.getEXIFSize()); + if (!copyJpegWithoutExif(&new_jpg_image, sdr_intent_compressed, decoder.getEXIFPos(), + decoder.getEXIFSize())) { + uhdr_error_info_t status; + status.error_code = UHDR_CODEC_INVALID_PARAM; + status.has_detail = 1; + snprintf(status.detail, sizeof status.detail, + "invalid EXIF position or size in compressed base image"); + return status; + } dest_data.reset(reinterpret_cast(new_jpg_image.data)); exif_from_jpg.data = decoder.getEXIFPtr(); exif_from_jpg.data_sz = decoder.getEXIFSize(); @@ -1316,11 +1329,18 @@ uhdr_error_info_t JpegR::appendGainMap(uhdr_compressed_image_t* sdr_intent_compr if (base_data[base_pos] != 0xFF) break; if (base_pos + 1 >= base_size) break; // guard against truncated data uint8_t marker = base_data[base_pos+1]; + if (marker == 0xFF) { + // JPEG permits repeated 0xFF fill bytes before a marker code. Keep the final + // 0xFF as the marker prefix so the normal segment parsing below can consume it. + while (base_pos + 2 < base_size && base_data[base_pos+1] == 0xFF) ++base_pos; + if (base_pos + 1 >= base_size || base_data[base_pos+1] == 0xFF) break; + marker = base_data[base_pos+1]; + } if (marker == 0xDA) { // SOS - stop here, MPF goes before this sos_offset = base_pos; break; } - if (marker == 0x00 || marker == 0xFF) { base_pos += 2; continue; } + if (marker == 0x00) { base_pos += 2; continue; } if (marker >= 0xD0 && marker <= 0xD7) { base_pos += 2; continue; } if (marker == 0xD9) break; // EOI - shouldn't happen here but guard // OOB check: need at least 2 bytes for the segment length field diff --git a/tests/jpegdecoderhelper_test.cpp b/tests/jpegdecoderhelper_test.cpp index aee3e6f5..d51abbe5 100644 --- a/tests/jpegdecoderhelper_test.cpp +++ b/tests/jpegdecoderhelper_test.cpp @@ -10,14 +10,19 @@ #include +#include +#include #include #include +#include #include +#include #include #include "ultrahdr/ultrahdrcommon.h" #include "ultrahdr/jpegdecoderhelper.h" #include "ultrahdr/icc.h" +#include "ultrahdr/jpegr.h" namespace ultrahdr { @@ -83,6 +88,118 @@ static bool loadFile(const char filename[], JpegDecoderHelperTest::Image* result return false; } +struct JpegSegment { + bool found = false; + size_t marker_offset = 0; + size_t payload_offset = 0; + size_t end = 0; +}; + +static JpegSegment findJpegSegment(const std::vector& image, uint8_t marker_code, + const uint8_t* payload_prefix = nullptr, + size_t payload_prefix_size = 0) { + JpegSegment result; + size_t pos = 2; // Skip SOI. + while (pos + 3 < image.size()) { + if (image[pos] != 0xff) return result; + const size_t marker_offset = pos; + while (pos < image.size() && image[pos] == 0xff) ++pos; + if (pos >= image.size()) return result; + const uint8_t code = image[pos++]; + if (code == 0xda || code == 0xd9) return result; + if (pos + 2 > image.size()) return result; + const size_t length = (static_cast(image[pos]) << 8) | image[pos + 1]; + if (length < 2 || pos + length > image.size()) return result; + const size_t payload_offset = pos + 2; + const size_t payload_size = length - 2; + if (code == marker_code && + (payload_prefix == nullptr || + (payload_size >= payload_prefix_size && + memcmp(image.data() + payload_offset, payload_prefix, payload_prefix_size) == 0))) { + result.found = true; + result.marker_offset = marker_offset; + result.payload_offset = payload_offset; + result.end = pos + length; + return result; + } + pos += length; + } + return result; +} + +static std::vector makeJpegMarker(uint8_t marker_code, + const std::vector& payload) { + const size_t length = payload.size() + 2; + EXPECT_LE(length, static_cast(std::numeric_limits::max())); + std::vector marker = {0xff, marker_code, static_cast(length >> 8), + static_cast(length)}; + marker.insert(marker.end(), payload.begin(), payload.end()); + return marker; +} + +static std::vector makeApp13Marker() { + static constexpr uint8_t kPhotoshopId[] = { + 'P', 'h', 'o', 't', 'o', 's', 'h', 'o', 'p', ' ', '3', '.', '0', '\0', + }; + // A small valid Photoshop resource block keeps this control marker meaningful to the + // APP13 parser as well as to the marker-offset calculation. + static constexpr uint8_t kIptcResource[] = { + '8', 'B', 'I', 'M', 0x04, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x01, 0x02, 0x03, 0x04, + }; + std::vector payload(kPhotoshopId, kPhotoshopId + sizeof(kPhotoshopId)); + payload.insert(payload.end(), kIptcResource, kIptcResource + sizeof(kIptcResource)); + return makeJpegMarker(0xed, payload); +} + +static std::vector makeJpegWithPrefix(const std::vector& source, + const std::vector& prefix, + bool exif_first) { + static constexpr uint8_t kExifId[] = {'E', 'x', 'i', 'f', '\0', '\0'}; + const JpegSegment exif = + findJpegSegment(source, 0xe1, kExifId, sizeof(kExifId)); + EXPECT_TRUE(exif.found); + if (!exif.found) return {}; + + std::vector result; + result.reserve(source.size() + prefix.size()); + result.insert(result.end(), source.begin(), source.begin() + (exif_first ? 2 : exif.marker_offset)); + if (!exif_first) result.insert(result.end(), prefix.begin(), prefix.end()); + result.insert(result.end(), source.begin() + exif.marker_offset, source.begin() + exif.end); + result.insert(result.end(), source.begin() + exif.end, source.end()); + return result; +} + +static void expectExifPayloadAndOffset(const std::vector& image, + const std::vector& expected_payload) { + static constexpr uint8_t kExifId[] = {'E', 'x', 'i', 'f', '\0', '\0'}; + const JpegSegment exif = + findJpegSegment(image, 0xe1, kExifId, sizeof(kExifId)); + ASSERT_TRUE(exif.found); + ASSERT_EQ(exif.end - exif.payload_offset, expected_payload.size()); + + JpegDecoderHelper decoder; + ASSERT_EQ(decoder.decompressImage(image.data(), image.size()).error_code, UHDR_CODEC_OK); + EXPECT_EQ(decoder.getEXIFPos(), static_cast(exif.payload_offset)); + ASSERT_EQ(decoder.getEXIFSize(), expected_payload.size()); + ASSERT_NE(decoder.getEXIFPtr(), nullptr); + EXPECT_EQ(memcmp(decoder.getEXIFPtr(), expected_payload.data(), expected_payload.size()), 0); +} + +static std::vector>> makeProblematicPrefixes( + const std::vector& source) { + const JpegSegment source_dqt = findJpegSegment(source, 0xdb); + if (!source_dqt.found) return {}; + const std::vector valid_dqt(source.begin() + source_dqt.marker_offset, + source.begin() + source_dqt.end); + return { + {"COM", makeJpegMarker(0xfe, {'r', 'e', 'g', 'r', 'e', 's', 's', 'i', 'o', 'n'})}, + {"APP14", makeJpegMarker(0xee, {'A', 'd', 'o', 'b', 'e', 0x00, 0x64, 0x00, 0x00, 0x00, + 0x00, 0x01})}, + {"valid DQT", valid_dqt}, + {"marker fill bytes", {0xff, 0xff, 0xff}}, + }; +} + void JpegDecoderHelperTest::SetUp() { if (!loadFile(YUV_IMAGE, &mYuvImage)) { FAIL() << "Load file " << YUV_IMAGE << " failed"; @@ -174,4 +291,140 @@ TEST_F(JpegDecoderHelperTest, getCompressedImageParametersIcc) { UHDR_CG_BT_709); } +TEST_F(JpegDecoderHelperTest, exifPayloadOffsetHandlesMarkerOrderControls) { + const std::vector source(mYuvIccImage.buffer.get(), + mYuvIccImage.buffer.get() + mYuvIccImage.size); + static constexpr uint8_t kExifId[] = {'E', 'x', 'i', 'f', '\0', '\0'}; + const JpegSegment source_exif = + findJpegSegment(source, 0xe1, kExifId, sizeof(kExifId)); + ASSERT_TRUE(source_exif.found); + const std::vector expected_payload( + source.begin() + source_exif.payload_offset, source.begin() + source_exif.end); + + { + SCOPED_TRACE("EXIF immediately after SOI"); + const std::vector image = makeJpegWithPrefix(source, {}, true); + expectExifPayloadAndOffset(image, expected_payload); + } + + { + SCOPED_TRACE("APP13 before EXIF"); + const std::vector image = makeJpegWithPrefix(source, makeApp13Marker(), false); + expectExifPayloadAndOffset(image, expected_payload); + } +} + +TEST_F(JpegDecoderHelperTest, exifPayloadOffsetCountsUnretainedJpegMarkers) { + const std::vector source(mYuvIccImage.buffer.get(), + mYuvIccImage.buffer.get() + mYuvIccImage.size); + static constexpr uint8_t kExifId[] = {'E', 'x', 'i', 'f', '\0', '\0'}; + const JpegSegment source_exif = + findJpegSegment(source, 0xe1, kExifId, sizeof(kExifId)); + ASSERT_TRUE(source_exif.found); + const std::vector expected_payload( + source.begin() + source_exif.payload_offset, source.begin() + source_exif.end); + const auto prefixes = makeProblematicPrefixes(source); + ASSERT_EQ(prefixes.size(), 4u); + + for (const auto& test_case : prefixes) { + SCOPED_TRACE(test_case.first); + const std::vector image = makeJpegWithPrefix(source, test_case.second, false); + expectExifPayloadAndOffset(image, expected_payload); + } +} + +TEST_F(JpegDecoderHelperTest, exifPayloadOffsetMatchesSelectedDuplicatePayload) { + const std::vector source(mYuvIccImage.buffer.get(), + mYuvIccImage.buffer.get() + mYuvIccImage.size); + const std::vector duplicate_payload = { + 'E', 'x', 'i', 'f', '\0', '\0', 's', 'e', 'l', 'e', 'c', 't', 'e', 'd', + }; + const std::vector duplicate_com = + makeJpegMarker(0xfe, {'d', 'u', 'p', 'l', 'i', 'c', 'a', 't', 'e'}); + const std::vector duplicate_exif = makeJpegMarker(0xe1, duplicate_payload); + std::vector prefix = duplicate_com; + prefix.insert(prefix.end(), duplicate_exif.begin(), duplicate_exif.end()); + const std::vector image = makeJpegWithPrefix(source, prefix, false); + ASSERT_FALSE(image.empty()); + + const JpegSegment expected_exif = + findJpegSegment(image, 0xe1, duplicate_payload.data(), duplicate_payload.size()); + ASSERT_TRUE(expected_exif.found); + expectExifPayloadAndOffset(image, duplicate_payload); +} + +TEST_F(JpegDecoderHelperTest, jpegrRecombinationPreservesExactPrimaryJpegBytes) { + const std::vector source(mYuvIccImage.buffer.get(), + mYuvIccImage.buffer.get() + mYuvIccImage.size); + static constexpr uint8_t kExifId[] = {'E', 'x', 'i', 'f', '\0', '\0'}; + static constexpr uint8_t kMpfId[] = {'M', 'P', 'F', '\0'}; + const JpegSegment source_exif = + findJpegSegment(source, 0xe1, kExifId, sizeof(kExifId)); + ASSERT_TRUE(source_exif.found); + + const auto prefixes = makeProblematicPrefixes(source); + ASSERT_EQ(prefixes.size(), 4u); + + for (const auto& test_case : prefixes) { + SCOPED_TRACE(test_case.first); + std::vector base_image = + makeJpegWithPrefix(source, test_case.second, false); + ASSERT_FALSE(base_image.empty()); + const JpegSegment base_exif = findJpegSegment(base_image, 0xe1, kExifId, sizeof(kExifId)); + ASSERT_TRUE(base_exif.found); + + uhdr_compressed_image_t base{}; + base.data = base_image.data(); + base.data_sz = base_image.size(); + base.capacity = base_image.size(); + base.cg = UHDR_CG_BT_709; + base.ct = UHDR_CT_SRGB; + base.range = UHDR_CR_FULL_RANGE; + + uhdr_compressed_image_t gainmap{}; + gainmap.data = mYuvImage.buffer.get(); + gainmap.data_sz = mYuvImage.size; + gainmap.capacity = mYuvImage.size; + gainmap.cg = UHDR_CG_BT_709; + gainmap.ct = UHDR_CT_SRGB; + gainmap.range = UHDR_CR_FULL_RANGE; + + uhdr_gainmap_metadata_ext_t metadata(kJpegrVersion); + for (int i = 0; i < 3; ++i) { + metadata.max_content_boost[i] = 2.0f; + metadata.min_content_boost[i] = 1.0f; + metadata.gamma[i] = 1.0f; + metadata.offset_sdr[i] = 0.0f; + metadata.offset_hdr[i] = 0.0f; + } + metadata.hdr_capacity_min = 1.0f; + metadata.hdr_capacity_max = 2.0f; + metadata.use_base_cg = true; + + std::vector output(base_image.size() + gainmap.data_sz + 65536); + uhdr_compressed_image_t dest{}; + dest.data = output.data(); + dest.capacity = output.size(); + + JpegR jpegr; + ASSERT_EQ(jpegr.encodeJPEGR(&base, &gainmap, &metadata, &dest).error_code, UHDR_CODEC_OK); + + const JpegSegment mpf = findJpegSegment(output, 0xe2, kMpfId, sizeof(kMpfId)); + ASSERT_TRUE(mpf.found); + + const JpegSegment output_exif = findJpegSegment(output, 0xe1, kExifId, sizeof(kExifId)); + ASSERT_TRUE(output_exif.found); + ASSERT_EQ(output_exif.end - output_exif.payload_offset, + source_exif.end - source_exif.payload_offset); + EXPECT_EQ(memcmp(output.data() + output_exif.payload_offset, + source.data() + source_exif.payload_offset, + source_exif.end - source_exif.payload_offset), + 0); + + JpegDecoderHelper output_decoder; + ASSERT_EQ(output_decoder.parseImage(output.data(), dest.data_sz).error_code, UHDR_CODEC_OK); + EXPECT_EQ(output_decoder.getEXIFPos(), static_cast(output_exif.payload_offset)); + } +} + } // namespace ultrahdr