From c07191986e27a59b594b6582b8871a3bb9c1321a Mon Sep 17 00:00:00 2001 From: george Date: Sat, 5 Apr 2025 17:54:50 +0300 Subject: [PATCH 1/3] Release GIL on image decoding methods --- .gitignore | 1 + src/wuffs-aux-image-wrapper.h | 26 +++++++-------------- src/wuffs-bindings.cpp | 44 +++++++++++++++++++++++++++++------ 3 files changed, 47 insertions(+), 24 deletions(-) diff --git a/.gitignore b/.gitignore index 2f35030..2a182ab 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ .idea/ +.vscode/ .cache/ cmake-build-debug/ cmake-build-release/ diff --git a/src/wuffs-aux-image-wrapper.h b/src/wuffs-aux-image-wrapper.h index 23e5077..85abccb 100644 --- a/src/wuffs-aux-image-wrapper.h +++ b/src/wuffs-aux-image-wrapper.h @@ -1,7 +1,5 @@ #pragma once -#include - #include #include #include @@ -138,8 +136,8 @@ struct ImageDecoderConfig { uint64_t max_incl_metadata_length = wuffs_aux::DecodeImageArgMaxInclMetadataLength::DefaultValue().repr; std::vector enabled_decoders = { - ImageDecoderType::BMP, ImageDecoderType::GIF, ImageDecoderType::NIE, - ImageDecoderType::PNG, ImageDecoderType::TGA, ImageDecoderType::WBMP, + ImageDecoderType::BMP, ImageDecoderType::GIF, ImageDecoderType::NIE, + ImageDecoderType::PNG, ImageDecoderType::TGA, ImageDecoderType::WBMP, ImageDecoderType::JPEG, ImageDecoderType::WEBP, ImageDecoderType::QOI, ImageDecoderType::ETC2, ImageDecoderType::TH}; uint32_t pixel_format = wuffs_base__make_pixel_format( @@ -151,10 +149,10 @@ struct ImageDecoderConfig { // input struct MetadataEntry { wuffs_base__more_information minfo{}; - pybind11::array_t data; + std::vector data; MetadataEntry(const wuffs_base__more_information& minfo, - pybind11::array_t&& data) + std::vector&& data) : minfo(minfo), data(std::move(data)) {} MetadataEntry() : minfo(wuffs_base__empty_more_information()) {} @@ -178,7 +176,7 @@ struct MetadataEntry { struct ImageDecodingResult { wuffs_base__pixel_config pixcfg = wuffs_base__null_pixel_config(); - pybind11::array_t pixbuf; + std::vector pixbuf; std::vector reported_metadata; std::string error_message; @@ -274,7 +272,7 @@ class ImageDecoder : public wuffs_aux::DecodeImageCallbacks { std::string HandleMetadata(const wuffs_base__more_information& minfo, wuffs_base__slice_u8 raw) override { decoding_result_.reported_metadata.emplace_back( - minfo, pybind11::array(pybind11::dtype("uint8"), {raw.len}, raw.ptr)); + minfo, std::vector{raw.ptr, raw.ptr + raw.len}); return ""; } @@ -296,15 +294,15 @@ class ImageDecoder : public wuffs_aux::DecodeImageCallbacks { if (len == 0 || SIZE_MAX < len) { return {wuffs_aux::DecodeImage_UnsupportedPixelConfiguration}; } - decoding_result_.pixbuf.resize({len}); + decoding_result_.pixbuf.resize(len); if (!allow_uninitialized_memory) { - std::memset(decoding_result_.pixbuf.mutable_data(), 0, + std::memset(decoding_result_.pixbuf.data(), 0, decoding_result_.pixbuf.size()); } wuffs_base__pixel_buffer pixbuf; wuffs_base__status status = pixbuf.set_from_slice( &image_config.pixcfg, - wuffs_base__make_slice_u8(decoding_result_.pixbuf.mutable_data(), + wuffs_base__make_slice_u8(decoding_result_.pixbuf.data(), decoding_result_.pixbuf.size())); if (!status.is_ok()) { decoding_result_.pixbuf = {}; @@ -353,12 +351,6 @@ class ImageDecoder : public wuffs_aux::DecodeImageCallbacks { decoding_result_.pixcfg = wuffs_base__null_pixel_config(); } else { decoding_result_.pixcfg = decode_image_result.pixbuf.pixcfg; - decoding_result_.pixbuf = - decoding_result_.pixbuf.reshape(std::vector{ - decoding_result_.pixcfg.height(), decoding_result_.pixcfg.width(), - decoding_result_.pixcfg.pixbuf_len() / - (decoding_result_.pixcfg.width() * - decoding_result_.pixcfg.height())}); } return std::move(decoding_result_); } diff --git a/src/wuffs-bindings.cpp b/src/wuffs-bindings.cpp index 76e3b4d..4e0ceff 100644 --- a/src/wuffs-bindings.cpp +++ b/src/wuffs-bindings.cpp @@ -82,8 +82,7 @@ PYBIND11_MODULE(pywuffs, m) { wuffs_aux_wrap::ImageDecoderQuirks::GIF_REJECT_EMPTY_FRAME) .value("GIF_REJECT_EMPTY_PALETTE", wuffs_aux_wrap::ImageDecoderQuirks::GIF_REJECT_EMPTY_PALETTE) - .value("QUALITY", - wuffs_aux_wrap::ImageDecoderQuirks::QUALITY, + .value("QUALITY", wuffs_aux_wrap::ImageDecoderQuirks::QUALITY, "Configures decoders (for a lossy format, where there is some " "leeway in \"a/the correct decoding\") to use lower than, equal " "to or higher than the default quality setting."); @@ -216,8 +215,15 @@ py::enum_( "Holds parsed metadata piece.") .def_readonly("minfo", &wuffs_aux_wrap::MetadataEntry::minfo, "wuffs_base__more_information: Info on parsed metadata.") - .def_readonly("data", &wuffs_aux_wrap::MetadataEntry::data, - "np.array: Parsed metadata (1D uint8 Numpy array)."); + .def_property_readonly( + "data", + [](wuffs_aux_wrap::MetadataEntry& self) { + return pybind11::array_t(pybind11::buffer_info( + self.data.data(), sizeof(uint8_t), + pybind11::format_descriptor::value, 1, + {self.data.size()}, {1})); + }, + "np.array: Parsed metadata (1D uint8 Numpy array)."); py::class_(aux_m, "ImageDecoderConfig", "Image decoder configuration.") @@ -319,9 +325,31 @@ py::enum_( "or reject partial success.\n" " - On failure, the error_message is non-empty and pixbuf is " "empty.") - .def_readonly("pixbuf", &wuffs_aux_wrap::ImageDecodingResult::pixbuf, - "np.array: decoded pixel buffer (uint8 Numpy array of [H, " - "W, C] shape).") + .def_property_readonly( + "pixbuf", + [](wuffs_aux_wrap::ImageDecodingResult& self) + -> pybind11::array_t { + const auto height = self.pixcfg.height(); + const auto width = self.pixcfg.width(); + + if (width == 0 || height == 0) { + return {}; + } + + constexpr size_t kNumDimensions = 3; + const auto channels = self.pixcfg.pixbuf_len() / (width * height); + const std::array shape = {height, width, + channels}; + const std::array strides = { + width * channels, channels, 1}; + + return pybind11::array_t(pybind11::buffer_info( + self.pixbuf.data(), sizeof(uint8_t), + pybind11::format_descriptor::value, kNumDimensions, + shape, strides)); + }, + "np.array: decoded pixel buffer (uint8 Numpy array of [H, " + "W, C] shape).") .def_readonly("pixcfg", &wuffs_aux_wrap::ImageDecodingResult::pixcfg, "wuffs_base__pixel_config: decoded pixel buffer config.") .def_readonly("reported_metadata", @@ -345,6 +373,7 @@ py::enum_( [](wuffs_aux_wrap::ImageDecoder& image_decoder, const py::bytes& data) -> wuffs_aux_wrap::ImageDecodingResult { py::buffer_info data_view(py::buffer(data).request()); + pybind11::gil_scoped_release release_gil; return image_decoder.Decode( reinterpret_cast(data_view.ptr), data_view.size); }, @@ -358,6 +387,7 @@ py::enum_( [](wuffs_aux_wrap::ImageDecoder& image_decoder, const std::string& path_to_file) -> wuffs_aux_wrap::ImageDecodingResult { + pybind11::gil_scoped_release release_gil; return image_decoder.Decode(path_to_file); }, "Decodes image using given file path.\n\n" From 7fe3c49d02f5906fdd3866f6faa535a654b8c504 Mon Sep 17 00:00:00 2001 From: george Date: Sat, 5 Apr 2025 17:55:38 +0300 Subject: [PATCH 2/3] Add multithreading cases to tests --- test/test_aux_image_decoder.py | 84 ++++++++++++++++++++++++++-------- 1 file changed, 66 insertions(+), 18 deletions(-) diff --git a/test/test_aux_image_decoder.py b/test/test_aux_image_decoder.py index 01cde19..d0c4cbf 100644 --- a/test/test_aux_image_decoder.py +++ b/test/test_aux_image_decoder.py @@ -6,19 +6,19 @@ from pywuffs.aux import * from pywuffs import * -IMAGES_PATH = os.path.join(os.path.dirname(os.path.realpath(__file__)), "images/") +IMAGES_PATH = os.path.join(os.path.dirname(os.path.realpath(__file__)), "images") TEST_IMAGES = [ - (ImageDecoderType.PNG, IMAGES_PATH + "/lena.png"), - (ImageDecoderType.BMP, IMAGES_PATH + "/lena.bmp"), - (ImageDecoderType.TGA, IMAGES_PATH + "/lena.tga"), - (ImageDecoderType.NIE, IMAGES_PATH + "/hippopotamus.nie"), - (ImageDecoderType.GIF, IMAGES_PATH + "/lena.gif"), - (ImageDecoderType.WBMP, IMAGES_PATH + "/lena.wbmp"), - (ImageDecoderType.JPEG, IMAGES_PATH + "/lena.jpeg"), - (ImageDecoderType.WEBP, IMAGES_PATH + "/lena.webp"), - (ImageDecoderType.QOI, IMAGES_PATH + "/lena.qoi"), - (ImageDecoderType.ETC2, IMAGES_PATH + "/bricks-color.etc2.pkm"), - (ImageDecoderType.TH, IMAGES_PATH + "/1QcSHQRnh493V4dIh4eXh1h4kJUI.th") + (ImageDecoderType.PNG, os.path.join(IMAGES_PATH, "lena.png")), + (ImageDecoderType.BMP, os.path.join(IMAGES_PATH, "lena.bmp")), + (ImageDecoderType.TGA, os.path.join(IMAGES_PATH, "lena.tga")), + (ImageDecoderType.NIE, os.path.join(IMAGES_PATH, "hippopotamus.nie")), + (ImageDecoderType.GIF, os.path.join(IMAGES_PATH, "lena.gif")), + (ImageDecoderType.WBMP, os.path.join(IMAGES_PATH, "lena.wbmp")), + (ImageDecoderType.JPEG, os.path.join(IMAGES_PATH, "lena.jpeg")), + (ImageDecoderType.WEBP, os.path.join(IMAGES_PATH, "lena.webp")), + (ImageDecoderType.QOI, os.path.join(IMAGES_PATH, "lena.qoi")), + (ImageDecoderType.ETC2, os.path.join(IMAGES_PATH, "bricks-color.etc2.pkm")), + (ImageDecoderType.TH, os.path.join(IMAGES_PATH, "1QcSHQRnh493V4dIh4eXh1h4kJUI.th")) ] @@ -48,7 +48,7 @@ def test_decode_image_with_metadata(param): config = ImageDecoderConfig() config.flags = param[0] decoder = ImageDecoder(config) - decoding_result = decoder.decode(IMAGES_PATH + "/lena_exif.png") + decoding_result = decoder.decode(os.path.join(IMAGES_PATH, "lena_exif.png")) assert_decoded(decoding_result, param[1]) @@ -140,12 +140,12 @@ def test_decode_image_quirks_quality(): config = ImageDecoderConfig() config.quirks = {ImageDecoderQuirks.QUALITY: LowerQuality} decoder = ImageDecoder(config) - decoding_result_lower_quality = decoder.decode(IMAGES_PATH + "/lena.jpeg") + decoding_result_lower_quality = decoder.decode(os.path.join(IMAGES_PATH, "lena.jpeg")) assert_decoded(decoding_result_lower_quality) assert decoding_result_lower_quality.pixbuf.shape == (32, 32, 4) config.quirks = {ImageDecoderQuirks.QUALITY: HigherQuality} decoder = ImageDecoder(config) - decoding_result_higher_quality = decoder.decode(IMAGES_PATH + "/lena.jpeg") + decoding_result_higher_quality = decoder.decode(os.path.join(IMAGES_PATH, "lena.jpeg")) assert_decoded(decoding_result_higher_quality) assert decoding_result_higher_quality.pixbuf.shape == (32, 32, 4) assert decoding_result_lower_quality != decoding_result_higher_quality @@ -176,7 +176,7 @@ def test_decode_image_exif_metadata(): config = ImageDecoderConfig() config.flags = [ImageDecoderFlags.REPORT_METADATA_EXIF] decoder = ImageDecoder(config) - decoding_result = decoder.decode(IMAGES_PATH + "/lena_exif.png") + decoding_result = decoder.decode(os.path.join(IMAGES_PATH, "lena_exif.png")) assert_decoded(decoding_result, 1) assert decoding_result.pixbuf.shape == (32, 32, 4) meta_minfo = decoding_result.reported_metadata[0].minfo @@ -215,7 +215,7 @@ def test_decode_image_invalid_kvp_chunk(): config = ImageDecoderConfig() config.flags = [ImageDecoderFlags.REPORT_METADATA_KVP] decoder = ImageDecoder(config) - decoding_result = decoder.decode(IMAGES_PATH + "/lena.png") + decoding_result = decoder.decode(os.path.join(IMAGES_PATH, "lena.png")) assert_not_decoded(decoding_result, "png: bad text chunk (not Latin-1)", 1) @@ -284,5 +284,53 @@ def test_decode_image_max_incl_metadata_length(): config.max_incl_metadata_length = 8 config.flags = [ImageDecoderFlags.REPORT_METADATA_EXIF] decoder = ImageDecoder(config) - decoding_result = decoder.decode(IMAGES_PATH + "/lena_exif.png") + decoding_result = decoder.decode(os.path.join(IMAGES_PATH, "lena_exif.png")) assert_not_decoded(decoding_result, ImageDecoderError.MaxInclMetadataLengthExceeded) + + +# Multithreading tests + +from concurrent.futures import ThreadPoolExecutor + + +def test_decode_multithreaded(): + config = ImageDecoderConfig() + + def decode(image): + decoder = ImageDecoder(config) + return decoder.decode(image) + + test_image = os.path.join(IMAGES_PATH, "lena.png") + with open(test_image, "rb") as f: + test_image_data = f.read() + + for payload in (test_image, test_image_data): + for num_threads in (1, 2, 4, 8): + with ThreadPoolExecutor(max_workers=num_threads) as executor: + futures = [executor.submit(decode, payload) for _ in range(num_threads)] + results = [future.result() for future in futures] + for result in results: + assert_decoded(result) + assert np.array_equal(results[0].pixbuf, result.pixbuf) + + +def test_decode_multithreaded_with_metadata(): + config = ImageDecoderConfig() + config.flags = [ImageDecoderFlags.REPORT_METADATA_EXIF] + + def decode(image): + decoder = ImageDecoder(config) + return decoder.decode(image) + + test_image = os.path.join(IMAGES_PATH, "lena_exif.png") + num_threads = 4 + + with ThreadPoolExecutor(max_workers=num_threads) as executor: + futures = [executor.submit(decode, test_image) for _ in range(num_threads)] + results = [future.result() for future in futures] + for result in results: + assert_decoded(result, 1) + meta_minfo = result.reported_metadata[0].minfo + meta_bytes = result.reported_metadata[0].data.tobytes() + assert meta_minfo.metadata__fourcc() == 1163413830 + assert meta_bytes[:2] == b"II" From 7b9010253acb46fd0977be32c3066ead37a899bb Mon Sep 17 00:00:00 2001 From: george Date: Sat, 5 Apr 2025 18:01:57 +0300 Subject: [PATCH 3/3] Minor fixes --- src/wuffs-bindings.cpp | 2 +- test/test_aux_image_decoder.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/wuffs-bindings.cpp b/src/wuffs-bindings.cpp index 4e0ceff..9370a81 100644 --- a/src/wuffs-bindings.cpp +++ b/src/wuffs-bindings.cpp @@ -365,7 +365,7 @@ py::enum_( py::class_(aux_m, "ImageDecoder", "Image decoder class.") .def(py::init(), - "Sole constructor.\n\n" + "Sole constructor. Please note that the class is not thread-safe.\n\n" "Args:" "\n config (ImageDecoderConfig): image decoder config.") .def( diff --git a/test/test_aux_image_decoder.py b/test/test_aux_image_decoder.py index d0c4cbf..027e1af 100644 --- a/test/test_aux_image_decoder.py +++ b/test/test_aux_image_decoder.py @@ -20,6 +20,7 @@ (ImageDecoderType.ETC2, os.path.join(IMAGES_PATH, "bricks-color.etc2.pkm")), (ImageDecoderType.TH, os.path.join(IMAGES_PATH, "1QcSHQRnh493V4dIh4eXh1h4kJUI.th")) ] +EXIF_FOURCC = 0x45584946 # Positive test cases @@ -181,7 +182,7 @@ def test_decode_image_exif_metadata(): assert decoding_result.pixbuf.shape == (32, 32, 4) meta_minfo = decoding_result.reported_metadata[0].minfo meta_bytes = decoding_result.reported_metadata[0].data.tobytes() - assert meta_minfo.metadata__fourcc() == 1163413830 # EXIF + assert meta_minfo.metadata__fourcc() == EXIF_FOURCC assert meta_bytes[:2] == b"II" # little endian exif_orientation = 0 cursor = 0 @@ -332,5 +333,5 @@ def decode(image): assert_decoded(result, 1) meta_minfo = result.reported_metadata[0].minfo meta_bytes = result.reported_metadata[0].data.tobytes() - assert meta_minfo.metadata__fourcc() == 1163413830 + assert meta_minfo.metadata__fourcc() == EXIF_FOURCC assert meta_bytes[:2] == b"II"