From f056f9d31b304dda9dff2d723c95cc5d512a53f3 Mon Sep 17 00:00:00 2001 From: AliceMazzoleni99 Date: Tue, 18 Mar 2025 21:42:34 +0100 Subject: [PATCH] reserved enough size in the fifo buffer to reorder all added proper 4 byte alignment --- slsReceiverSoftware/src/DataProcessor.cpp | 89 ++++++++++++------- slsReceiverSoftware/src/DataProcessor.h | 2 +- slsReceiverSoftware/src/GeneralData.h | 14 ++- .../tests/test-ArrangeDataBasedOnBitList.cpp | 6 +- 4 files changed, 75 insertions(+), 36 deletions(-) diff --git a/slsReceiverSoftware/src/DataProcessor.cpp b/slsReceiverSoftware/src/DataProcessor.cpp index 724c5758d..bd89784ff 100644 --- a/slsReceiverSoftware/src/DataProcessor.cpp +++ b/slsReceiverSoftware/src/DataProcessor.cpp @@ -26,6 +26,49 @@ namespace sls { +// TODO: move somewhere else +template struct AlignedData { + T *ptr; // Aligned data pointer + std::unique_ptr[]> buffer; + + AlignedData( + T *p, + std::unique_ptr[]> buf) + : ptr(p), buffer(std::move(buf)) {} +}; + +// TODO: should not be in this file any suggestions to move it to a more +// appropriate file? +// TODO: Add unit test +/* + * AlignData + * Aligns data to a given type T with proper alignment + * @param data: pointer to data + * @param size: size of data to align in bytes + * @return: aligned data + */ +template +void AlignData(AlignedData &aligneddata, char *data, size_t size) { + using AlignedBuffer = + typename std::aligned_storage::type; + std::unique_ptr tempbuffer; + + if (reinterpret_cast(data) % alignof(uint64_t) == 0) { + // If aligned directly cast to pointer + + aligneddata.ptr = reinterpret_cast(data); + + } else { + // Allocate a temporary buffer with proper alignment + tempbuffer = std::make_unique(size / sizeof(T)); + // size = ctbDigitaldbt; + + std::memcpy(tempbuffer.get(), data, size); + aligneddata.buffer = std::move(tempbuffer); + aligneddata.ptr = reinterpret_cast(aligneddata.buffer.get()); + } +} + const std::string DataProcessor::typeName = "DataProcessor"; DataProcessor::DataProcessor(int index) : ThreadObject(index, typeName) { @@ -574,26 +617,11 @@ void DataProcessor::Reorder(size_t &size, char *data) { } // make sure data is aligned to 8 bytes before casting to uint64_t - char *ptr = data + nAnalogDataBytes + ctbDbitOffset; - uint64_t *source = nullptr; - using AlignedBuffer = - std::aligned_storage::type; - std::unique_ptr tempbuffer; + AlignedData aligned_data(nullptr, nullptr); + AlignData(aligned_data, data + nAnalogDataBytes + ctbDbitOffset, + ctbDigitalDataBytes); - if (reinterpret_cast(ptr) % alignof(uint64_t) == 0) { - // If aligned, directly cast to uint64_t pointer - source = reinterpret_cast(ptr); - } else { - // Allocate a temporary buffer with proper alignment - tempbuffer = std::make_unique(ctbDigitalDataBytes / - sizeof(uint64_t)); - - std::memcpy(tempbuffer.get(), ptr, ctbDigitalDataBytes); - source = reinterpret_cast(tempbuffer.get()); - } - - // auto *source = (uint64_t *)(data + nAnalogDataBytes + ctbDbitOffset); - // TODO: leads to unaligned data + uint64_t *source = aligned_data.ptr; const size_t numDigitalSamples = (ctbDigitalDataBytes / sizeof(uint64_t)); @@ -612,13 +640,10 @@ void DataProcessor::Reorder(size_t &size, char *data) { int bitoffset = 0; // reorder - int count = 0; - int written = 0; for (size_t bi = 0; bi < 64; ++bi) { if (bitoffset != 0) { bitoffset = 0; - ++count; ++dest; } @@ -628,10 +653,8 @@ void DataProcessor::Reorder(size_t &size, char *data) { ++bitoffset; if (bitoffset == 8) { bitoffset = 0; - ++count; ++dest; } - ++written; } } @@ -671,7 +694,13 @@ void DataProcessor::ArrangeDbitData(size_t &size, char *data) { return; } - auto *source = (uint64_t *)(data + nAnalogDataBytes + ctbDbitOffset); + AlignedData aligned_data(nullptr, nullptr); + AlignData(aligned_data, data + nAnalogDataBytes + ctbDbitOffset, + ctbDigitalDataBytes); + + uint64_t *source = aligned_data.ptr; + + // auto *source = (uint64_t *)(data + nAnalogDataBytes + ctbDbitOffset); const int numDigitalSamples = (ctbDigitalDataBytes / sizeof(uint64_t)); @@ -747,19 +776,19 @@ void DataProcessor::ArrangeDbitData(size_t &size, char *data) { } } - // copy back to memory and update size - memcpy(data + nAnalogDataBytes, result.data(), - totalNumBytes * sizeof(uint8_t)); - size = totalNumBytes * sizeof(uint8_t) + nAnalogDataBytes + nTransceiverDataBytes; // check if size changed, if so move transceiver data to avoid gap in memory - if (size < nAnalogDataBytes + nDigitalDataBytes + nTransceiverDataBytes) + if (size != nAnalogDataBytes + nDigitalDataBytes + nTransceiverDataBytes) memmove(data + nAnalogDataBytes + totalNumBytes * sizeof(uint8_t), data + nAnalogDataBytes + nDigitalDataBytes, nTransceiverDataBytes); + // copy back to memory and update size + memcpy(data + nAnalogDataBytes, result.data(), + totalNumBytes * sizeof(uint8_t)); + LOG(logDEBUG1) << "totalNumBytes: " << totalNumBytes << " nAnalogDataBytes:" << nAnalogDataBytes << " ctbDbitOffset:" << ctbDbitOffset diff --git a/slsReceiverSoftware/src/DataProcessor.h b/slsReceiverSoftware/src/DataProcessor.h index 3e0aa0b9d..ab58b2fe6 100644 --- a/slsReceiverSoftware/src/DataProcessor.h +++ b/slsReceiverSoftware/src/DataProcessor.h @@ -177,7 +177,7 @@ class DataProcessor : private virtual slsDetectorDefs, public ThreadObject { uint32_t streamingTimerInMs; uint32_t streamingStartFnum; uint32_t currentFreqCount{0}; - struct timespec timerbegin {}; + struct timespec timerbegin{}; bool framePadding; std::vector ctbDbitList; bool reorder{false}; // true if data should be reordered TODO: add as mode diff --git a/slsReceiverSoftware/src/GeneralData.h b/slsReceiverSoftware/src/GeneralData.h index 0425815a3..6d9a5d626 100644 --- a/slsReceiverSoftware/src/GeneralData.h +++ b/slsReceiverSoftware/src/GeneralData.h @@ -60,8 +60,8 @@ class GeneralData { slsDetectorDefs::frameDiscardPolicy frameDiscardMode{ slsDetectorDefs::NO_DISCARD}; - GeneralData(){}; - virtual ~GeneralData(){}; + GeneralData() {}; + virtual ~GeneralData() {}; // Returns the pixel depth in byte, 4 bits being 0.5 byte float GetPixelDepth() { return float(dynamicRange) / 8; } @@ -443,6 +443,7 @@ class ChipTestBoardData : public GeneralData { nDigitalBytes = 0; nTransceiverBytes = 0; int nAnalogChans = 0, nDigitalChans = 0, nTransceiverChans = 0; + uint64_t digital_bytes_reserved = 0; // analog channels (normal, analog/digital readout) if (readoutType == slsDetectorDefs::ANALOG_ONLY || @@ -461,7 +462,12 @@ class ChipTestBoardData : public GeneralData { readoutType == slsDetectorDefs::ANALOG_AND_DIGITAL || readoutType == slsDetectorDefs::DIGITAL_AND_TRANSCEIVER) { nDigitalChans = NCHAN_DIGITAL; - nDigitalBytes = (sizeof(uint64_t) * nDigitalSamples); + // allocate enough memory to support reordering of digital bits + uint32_t num_bytes_per_bit = (nDigitalSamples % 8 == 0) + ? nDigitalSamples / 8 + : nDigitalSamples / 8 + 1; + digital_bytes_reserved = 64 * num_bytes_per_bit; + nDigitalBytes = sizeof(uint64_t) * nDigitalSamples; LOG(logDEBUG1) << "Number of Digital Channels:" << nDigitalChans << " Databytes: " << nDigitalBytes; } @@ -480,7 +486,7 @@ class ChipTestBoardData : public GeneralData { nPixelsX = nAnalogChans + nDigitalChans + nTransceiverChans; dataSize = tengigaEnable ? 8144 : UDP_PACKET_DATA_BYTES; packetSize = headerSizeinPacket + dataSize; - imageSize = nAnalogBytes + nDigitalBytes + nTransceiverBytes; + imageSize = nAnalogBytes + digital_bytes_reserved + nTransceiverBytes; packetsPerFrame = ceil((double)imageSize / (double)dataSize); LOG(logDEBUG1) << "Total Number of Channels:" << nPixelsX diff --git a/slsReceiverSoftware/tests/test-ArrangeDataBasedOnBitList.cpp b/slsReceiverSoftware/tests/test-ArrangeDataBasedOnBitList.cpp index 1235daae1..3139e11e6 100644 --- a/slsReceiverSoftware/tests/test-ArrangeDataBasedOnBitList.cpp +++ b/slsReceiverSoftware/tests/test-ArrangeDataBasedOnBitList.cpp @@ -106,7 +106,11 @@ class DataProcessorTestFixture { void set_data() { delete[] data; - data = new char[get_size()]; + uint64_t max_bytes_per_bit = + num_samples % 8 == 0 ? num_samples / 8 : num_samples / 8 + 1; + uint64_t reserved_size = + get_size() - num_digital_bytes + max_bytes_per_bit * 64; + data = new char[reserved_size]; // set testing data memset(data, dummy_value, num_analog_bytes); // set to dummy value