Merge pull request #3 from dev0x13/release-gil-on-image-decoding

dev0x13 · web-flow · commit d1fcdfa53cd8 · 2025-04-05T18:04:31.000+03:00
Release GIL on image decoder invocations
diff --git 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
@@ -1,7 +1,5 @@
 #pragma once
 
-#include <pybind11/numpy.h>
-
 #include <map>
 #include <string>
 #include <unordered_set>
@@ -138,8 +136,8 @@ struct ImageDecoderConfig {
   uint64_t max_incl_metadata_length =
       wuffs_aux::DecodeImageArgMaxInclMetadataLength::DefaultValue().repr;
   std::vector<ImageDecoderType> 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<uint8_t> data;
+  std::vector<uint8_t> data;
 
   MetadataEntry(const wuffs_base__more_information& minfo,
-                pybind11::array_t<uint8_t>&& data)
+                std::vector<uint8_t>&& 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<uint8_t> pixbuf;
+  std::vector<uint8_t> pixbuf;
   std::vector<MetadataEntry> 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<uint8_t>{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<size_t>{
-              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
@@ -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_<wuffs_aux_wrap::PixelFormat>(
                                             "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<uint8_t>(pybind11::buffer_info(
+                self.data.data(), sizeof(uint8_t),
+                pybind11::format_descriptor<uint8_t>::value, 1,
+                {self.data.size()}, {1}));
+          },
+          "np.array: Parsed metadata (1D uint8 Numpy array).");
 
   py::class_<wuffs_aux_wrap::ImageDecoderConfig>(aux_m, "ImageDecoderConfig",
                                                  "Image decoder configuration.")
@@ -319,9 +325,31 @@ py::enum_<wuffs_aux_wrap::PixelFormat>(
       "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<uint8_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<size_t, kNumDimensions> shape = {height, width,
+                                                              channels};
+            const std::array<size_t, kNumDimensions> strides = {
+                width * channels, channels, 1};
+
+            return pybind11::array_t<uint8_t>(pybind11::buffer_info(
+                self.pixbuf.data(), sizeof(uint8_t),
+                pybind11::format_descriptor<uint8_t>::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",
@@ -337,14 +365,15 @@ py::enum_<wuffs_aux_wrap::PixelFormat>(
   py::class_<wuffs_aux_wrap::ImageDecoder>(aux_m, "ImageDecoder",
                                            "Image decoder class.")
       .def(py::init<const wuffs_aux_wrap::ImageDecoderConfig&>(),
-           "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(
           "decode",
           [](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<uint8_t*>(data_view.ptr), data_view.size);
           },
@@ -358,6 +387,7 @@ py::enum_<wuffs_aux_wrap::PixelFormat>(
           [](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"
diff --git a/test/test_aux_image_decoder.py b/test/test_aux_image_decoder.py
@@ -6,20 +6,21 @@
 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"))
 ]
+EXIF_FOURCC = 0x45584946
 
 
 # Positive test cases
@@ -48,7 +49,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 +141,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,12 +177,12 @@ 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
     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
@@ -215,7 +216,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 +285,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() == EXIF_FOURCC
+            assert meta_bytes[:2] == b"II"

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,5 @@`
`1`	`1`	`.idea/`
	`2`	`+.vscode/`
`2`	`3`	`.cache/`
`3`	`4`	`cmake-build-debug/`
`4`	`5`	`cmake-build-release/`