Replace RGBImagePtr by RGBImageCleanup
Although RGBImagePtr, defined using std::unique_ptr, is correct, it is
confusing because it does not free the avifRGBImage struct. Since the
avifRGBImage struct is intended to be allocated on the stack, we should
not address this by forcing the users of RGBImagePtr to allocate
avifRGBImage from the heap.
Replace RGBImagePtr by RGBImageCleanup, which is modeled after
absl::Cleanup and is a lightweight cleanup object whose destructor
calls avifRGBImageFreePixels() on the associated avifRGBImage struct.
Co-authored-by: Wan-Teh Chang <wtc@google.com>
diff --git a/include/avif/avif_cxx.h b/include/avif/avif_cxx.h
index f369978..7a7723c 100644
--- a/include/avif/avif_cxx.h
+++ b/include/avif/avif_cxx.h
@@ -22,7 +22,6 @@
void operator()(avifEncoder * encoder) const { avifEncoderDestroy(encoder); }
void operator()(avifGainMap * gainMap) const { avifGainMapDestroy(gainMap); }
void operator()(avifImage * image) const { avifImageDestroy(image); }
- void operator()(avifRGBImage * image) const { avifRGBImageFreePixels(image); }
};
// Use these unique_ptr to ensure the structs are automatically destroyed.
@@ -30,8 +29,18 @@
using EncoderPtr = std::unique_ptr<avifEncoder, UniquePtrDeleter>;
using GainMapPtr = std::unique_ptr<avifGainMap, UniquePtrDeleter>;
using ImagePtr = std::unique_ptr<avifImage, UniquePtrDeleter>;
+
+// Automatically cleans the ressources of the avifRGBImage.
// To use when RGBImage actually owns the pixels. RGBImage can also be used as a view, in which case it does not own the pixels.
-using RGBImagePtr = std::unique_ptr<avifRGBImage, UniquePtrDeleter>;
+class RGBImageCleanup
+{
+public:
+ RGBImageCleanup(avifRGBImage * rgb) : rgb_(rgb) {}
+ ~RGBImageCleanup() { avifRGBImageFreePixels(rgb_); }
+
+private:
+ avifRGBImage * rgb_ = nullptr;
+};
} // namespace avif
diff --git a/tests/gtest/avifbasictest.cc b/tests/gtest/avifbasictest.cc
index 7a08621..ef30311 100644
--- a/tests/gtest/avifbasictest.cc
+++ b/tests/gtest/avifbasictest.cc
@@ -41,5 +41,17 @@
// testutil::WriteImage(decoded.get(), "/tmp/avifbasictest.png");
}
+TEST(BasicTest, RGBImageCleanup) {
+ // Make sure the following does not create sanitizer bugs.
+ ImagePtr image =
+ testutil::CreateImage(/*width=*/12, /*height=*/34, /*depth=*/8,
+ AVIF_PIXEL_FORMAT_YUV420, AVIF_PLANES_ALL);
+
+ avifRGBImage rgb;
+ avifRGBImageSetDefaults(&rgb, image.get());
+ RGBImageCleanup cleanup(&rgb);
+ ASSERT_EQ(avifRGBImageAllocatePixels(&rgb), AVIF_RESULT_OK);
+}
+
} // namespace
} // namespace avif