Compressor: throw on overflow instead of returning a negative size
Compress() and FrameTransformation::CompressImage() returned int64_t with a negative value meaning "did not fit". That is a footgun: the negative result silently converts to a huge size_t if a caller forgets to check it. Return size_t and instead throw a named CompressionBufferTooSmallException (deriving from JFJochException, Compression category) when the output would not fit the destination buffer. The receiver catches it explicitly and drops just that frame, as before; the offline/GetCompressedImage path uses a worst-case buffer so it never throws. Add a test that a too-small destination throws and a worst-case buffer does not. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -162,3 +162,11 @@ public:
|
||||
msg += " (" + std::string(strerror(errno)) + ")";
|
||||
}
|
||||
};
|
||||
|
||||
// Thrown by JFJochBitShuffleCompressor::Compress when the compressed output does not fit in the
|
||||
// caller-provided destination buffer. Lets callers (e.g. the receiver) drop just that frame.
|
||||
class CompressionBufferTooSmallException : public JFJochException {
|
||||
public:
|
||||
explicit CompressionBufferTooSmallException(const std::string &description)
|
||||
: JFJochException(JFJochExceptionCategory::Compression, description) {}
|
||||
};
|
||||
|
||||
@@ -85,20 +85,20 @@ std::vector<uint8_t> JFJochBitShuffleCompressor::Compress(const void *source, si
|
||||
return tmp;
|
||||
}
|
||||
|
||||
int64_t JFJochBitShuffleCompressor::Compress(void *dest, size_t dest_size, const void *source, size_t nelements, size_t elem_size) {
|
||||
size_t JFJochBitShuffleCompressor::Compress(void *dest, size_t dest_size, const void *source, size_t nelements, size_t elem_size) {
|
||||
auto c_dest = (char *) dest;
|
||||
auto c_source = (char *) source;
|
||||
|
||||
if (algorithm == CompressionAlgorithm::NO_COMPRESSION) {
|
||||
// Trivial case if no compression - copy content
|
||||
if (nelements * elem_size > dest_size)
|
||||
return -1;
|
||||
throw CompressionBufferTooSmallException("compressed output exceeds destination buffer");
|
||||
memcpy(dest, source, nelements * elem_size);
|
||||
return nelements * elem_size;
|
||||
}
|
||||
|
||||
if (dest_size < 12)
|
||||
return -1;
|
||||
throw CompressionBufferTooSmallException("compressed output exceeds destination buffer");
|
||||
|
||||
const size_t block_size = BlockSize(algorithm, elem_size);
|
||||
|
||||
@@ -115,10 +115,10 @@ int64_t JFJochBitShuffleCompressor::Compress(void *dest, size_t dest_size, const
|
||||
size_t compressed_size = 12;
|
||||
|
||||
// Blocks are small relative to the image, so before each one we just check that the
|
||||
// remaining space still covers that block's worst case, and bail out (-1) if not.
|
||||
// remaining space still covers that block's worst case, and throw if not.
|
||||
for (int i = 0; i < num_full_blocks; i++) {
|
||||
if (compressed_size + MaxCompressedBlockSize(algorithm, block_size * elem_size) > dest_size)
|
||||
return -1;
|
||||
throw CompressionBufferTooSmallException("compressed output exceeds destination buffer");
|
||||
compressed_size += CompressBlock(c_dest + compressed_size,
|
||||
c_source + i * block_size * elem_size, block_size, elem_size);
|
||||
}
|
||||
@@ -126,7 +126,7 @@ int64_t JFJochBitShuffleCompressor::Compress(void *dest, size_t dest_size, const
|
||||
size_t last_block_size = reminder_size - reminder_size % BSHUF_BLOCKED_MULT;
|
||||
if (last_block_size > 0) {
|
||||
if (compressed_size + MaxCompressedBlockSize(algorithm, last_block_size * elem_size) > dest_size)
|
||||
return -1;
|
||||
throw CompressionBufferTooSmallException("compressed output exceeds destination buffer");
|
||||
compressed_size += CompressBlock(c_dest + compressed_size,
|
||||
c_source + num_full_blocks * block_size * elem_size, last_block_size, elem_size);
|
||||
}
|
||||
@@ -134,7 +134,7 @@ int64_t JFJochBitShuffleCompressor::Compress(void *dest, size_t dest_size, const
|
||||
size_t leftover_bytes = (reminder_size % BSHUF_BLOCKED_MULT) * elem_size;
|
||||
if (leftover_bytes > 0) {
|
||||
if (compressed_size + leftover_bytes > dest_size)
|
||||
return -1;
|
||||
throw CompressionBufferTooSmallException("compressed output exceeds destination buffer");
|
||||
memcpy(c_dest + compressed_size, c_source + (num_full_blocks * block_size + last_block_size) * elem_size, leftover_bytes);
|
||||
compressed_size += leftover_bytes;
|
||||
}
|
||||
|
||||
@@ -37,7 +37,7 @@ public:
|
||||
explicit JFJochBitShuffleCompressor(CompressionAlgorithm algorithm);
|
||||
|
||||
template<class T>
|
||||
int64_t Compress(void *dest, size_t dest_size, const std::vector<T> &src) {
|
||||
size_t Compress(void *dest, size_t dest_size, const std::vector<T> &src) {
|
||||
return Compress(dest, dest_size, src.data(), src.size(), sizeof(T));
|
||||
};
|
||||
|
||||
@@ -46,7 +46,8 @@ public:
|
||||
return Compress(src.data(), src.size(), sizeof(T));
|
||||
}
|
||||
std::vector<uint8_t> Compress(const void* source, size_t nelements, size_t elem_size);
|
||||
int64_t Compress(void *dest, size_t dest_size, const void* source, size_t nelements, size_t elem_size);
|
||||
// Throws CompressionBufferTooSmallException if the compressed output would not fit dest_size.
|
||||
size_t Compress(void *dest, size_t dest_size, const void* source, size_t nelements, size_t elem_size);
|
||||
};
|
||||
|
||||
template <class T> std::vector<T> bitshuffle(const std::vector<T> &input, size_t block_size) {
|
||||
|
||||
@@ -54,7 +54,7 @@ FrameTransformation::FrameTransformation(const DiffractionExperiment &in_experim
|
||||
}
|
||||
}
|
||||
|
||||
int64_t FrameTransformation::CompressImage(void *output, size_t output_size) {
|
||||
size_t FrameTransformation::CompressImage(void *output, size_t output_size) {
|
||||
return compressor.Compress(output, output_size, precompression_buffer.data(), experiment.GetPixelsNum(),
|
||||
experiment.GetByteDepthImage());
|
||||
}
|
||||
@@ -70,7 +70,7 @@ CompressedImage FrameTransformation::GetCompressedImage() {
|
||||
experiment.GetPixelsNum(),
|
||||
experiment.GetByteDepthImage()));
|
||||
data = reinterpret_cast<uint8_t *>(compressed_buffer.data());
|
||||
size = static_cast<size_t>(CompressImage(compressed_buffer.data(), compressed_buffer.size()));
|
||||
size = CompressImage(compressed_buffer.data(), compressed_buffer.size());
|
||||
}
|
||||
|
||||
return CompressedImage(data, size,experiment.GetXPixelsNum(),
|
||||
|
||||
@@ -24,6 +24,7 @@ public:
|
||||
void ProcessModule(const void *input, uint16_t module_number, int data_stream);
|
||||
void FillNotCollectedModule(uint16_t module_number, int data_stream);
|
||||
CompressedImage GetCompressedImage();
|
||||
int64_t CompressImage(void *output, size_t output_size); // < 0 - error in Compression
|
||||
// Throws CompressionBufferTooSmallException if the image does not fit output_size.
|
||||
size_t CompressImage(void *output, size_t output_size);
|
||||
const void *GetImage() const;
|
||||
};
|
||||
|
||||
@@ -478,17 +478,10 @@ void JFJochReceiverFPGA::FrameTransformationThread(uint32_t threadid) {
|
||||
const size_t slot_size = experiment.GetImageBufferLocationSize();
|
||||
|
||||
const auto compression_start_time = std::chrono::steady_clock::now();
|
||||
int64_t image_size = transformation.CompressImage(writer_buffer + serializer.GetImageAppendOffset(),
|
||||
slot_size - (metadata_size + 32)); // keep 32 byte extra for close, etc.
|
||||
if (image_size < 0) {
|
||||
// The per-image CBOR metadata (spot/reflection lists, ...) leaves
|
||||
// too little room for the compressed image in the buffer slot. Drop
|
||||
// just this frame instead of aborting the whole data collection.
|
||||
logger.Error("Dropping image {} - CBOR metadata too large to store image: "
|
||||
"metadata {} do not fit in buffer slot {} B",
|
||||
message.number, metadata_size, slot_size);
|
||||
loc->release();
|
||||
} else {
|
||||
try {
|
||||
size_t image_size = transformation.CompressImage(
|
||||
writer_buffer + serializer.GetImageAppendOffset(),
|
||||
slot_size - (metadata_size + 32)); // keep 32 byte extra for close, etc.
|
||||
const auto compression_end_time = std::chrono::steady_clock::now();
|
||||
message.compression_time_s = std::chrono::duration<float>(
|
||||
compression_end_time - compression_start_time).count();
|
||||
@@ -519,6 +512,14 @@ void JFJochReceiverFPGA::FrameTransformationThread(uint32_t threadid) {
|
||||
} else
|
||||
loc->release();
|
||||
UpdateMaxImageSent(message.number);
|
||||
} catch (const CompressionBufferTooSmallException &) {
|
||||
// The per-image CBOR metadata (spot/reflection lists, ...) leaves
|
||||
// too little room for the compressed image in the buffer slot. Drop
|
||||
// just this frame instead of aborting the whole data collection.
|
||||
logger.Error("Dropping image {} - CBOR metadata too large to store image: "
|
||||
"metadata {} do not fit in buffer slot {} B",
|
||||
message.number, metadata_size, slot_size);
|
||||
loc->release();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -249,6 +249,24 @@ TEST_CASE("JFJochCompressor_JFJochDecompressor_ZSTD","[ZSTD]") {
|
||||
REQUIRE(memcmp(image.data(), output.data(), x.GetPixelsNum() * sizeof(int32_t)) == 0);
|
||||
}
|
||||
|
||||
TEST_CASE("JFJochCompressor_DestTooSmall_Throws","[ZSTD]") {
|
||||
DiffractionExperiment x(DetJF4M());
|
||||
x.Compression(CompressionAlgorithm::BSHUF_ZSTD).BitDepthImage(32).PixelSigned(true);
|
||||
|
||||
std::vector<int32_t> image(x.GetPixelsNum(), 345);
|
||||
|
||||
JFJochBitShuffleCompressor compressor(x.GetCompressionAlgorithm());
|
||||
|
||||
// A destination far too small for the compressed output must throw, not overflow it.
|
||||
std::vector<char> tiny(64);
|
||||
REQUIRE_THROWS_AS(compressor.Compress(tiny.data(), tiny.size(), image),
|
||||
CompressionBufferTooSmallException);
|
||||
|
||||
// A buffer sized to the worst case never throws.
|
||||
std::vector<char> big(MaxCompressedSize(x.GetCompressionAlgorithm(), x.GetPixelsNum(), sizeof(int32_t)));
|
||||
REQUIRE_NOTHROW(compressor.Compress(big.data(), big.size(), image));
|
||||
}
|
||||
|
||||
TEST_CASE("JFJochCompressor_JFJochDecompressor_LZ4","[ZSTD]") {
|
||||
DiffractionExperiment x(DetJF4M());
|
||||
x.Compression(CompressionAlgorithm::BSHUF_LZ4).BitDepthImage(32).PixelSigned(true);
|
||||
|
||||
Reference in New Issue
Block a user