diff --git a/common/JFJochException.h b/common/JFJochException.h index d2ecdb0f..4c132a5a 100644 --- a/common/JFJochException.h +++ b/common/JFJochException.h @@ -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) {} +}; diff --git a/compression/JFJochCompressor.cpp b/compression/JFJochCompressor.cpp index 4540db96..3470e728 100644 --- a/compression/JFJochCompressor.cpp +++ b/compression/JFJochCompressor.cpp @@ -85,20 +85,20 @@ std::vector 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; } diff --git a/compression/JFJochCompressor.h b/compression/JFJochCompressor.h index a6a3f97c..644900d2 100644 --- a/compression/JFJochCompressor.h +++ b/compression/JFJochCompressor.h @@ -37,7 +37,7 @@ public: explicit JFJochBitShuffleCompressor(CompressionAlgorithm algorithm); template - int64_t Compress(void *dest, size_t dest_size, const std::vector &src) { + size_t Compress(void *dest, size_t dest_size, const std::vector &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 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 std::vector bitshuffle(const std::vector &input, size_t block_size) { diff --git a/receiver/FrameTransformation.cpp b/receiver/FrameTransformation.cpp index 82ec2d8c..1f824217 100644 --- a/receiver/FrameTransformation.cpp +++ b/receiver/FrameTransformation.cpp @@ -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(compressed_buffer.data()); - size = static_cast(CompressImage(compressed_buffer.data(), compressed_buffer.size())); + size = CompressImage(compressed_buffer.data(), compressed_buffer.size()); } return CompressedImage(data, size,experiment.GetXPixelsNum(), diff --git a/receiver/FrameTransformation.h b/receiver/FrameTransformation.h index 870f52e5..586a37ad 100644 --- a/receiver/FrameTransformation.h +++ b/receiver/FrameTransformation.h @@ -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; }; diff --git a/receiver/JFJochReceiverFPGA.cpp b/receiver/JFJochReceiverFPGA.cpp index 6fe2a27e..5ba633b2 100644 --- a/receiver/JFJochReceiverFPGA.cpp +++ b/receiver/JFJochReceiverFPGA.cpp @@ -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( 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(); } } } diff --git a/tests/ZSTDCompressorTest.cpp b/tests/ZSTDCompressorTest.cpp index 0d0d4262..ba2fe8a4 100644 --- a/tests/ZSTDCompressorTest.cpp +++ b/tests/ZSTDCompressorTest.cpp @@ -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 image(x.GetPixelsNum(), 345); + + JFJochBitShuffleCompressor compressor(x.GetCompressionAlgorithm()); + + // A destination far too small for the compressed output must throw, not overflow it. + std::vector tiny(64); + REQUIRE_THROWS_AS(compressor.Compress(tiny.data(), tiny.size(), image), + CompressionBufferTooSmallException); + + // A buffer sized to the worst case never throws. + std::vector 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);