reserved enough size in the fifo buffer to reorder all added proper 4 byte alignment

slsReceiverSoftware/src/DataProcessor.cpp

@@ -26,6 +26,49 @@
 
 namespace sls {
 
+// TODO: move somewhere else
+template <typename T> struct AlignedData {
+    T *ptr; // Aligned data pointer
+    std::unique_ptr<std::aligned_storage_t<sizeof(T), alignof(T)>[]> buffer;
+
+    AlignedData(
+        T *p,
+        std::unique_ptr<std::aligned_storage_t<sizeof(T), alignof(T)>[]> 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 <typename T>
+void AlignData(AlignedData<T> &aligneddata, char *data, size_t size) {
+    using AlignedBuffer =
+        typename std::aligned_storage<sizeof(T), alignof(T)>::type;
+    std::unique_ptr<AlignedBuffer[]> tempbuffer;
+
+    if (reinterpret_cast<uintptr_t>(data) % alignof(uint64_t) == 0) {
+        // If aligned directly cast to pointer
+
+        aligneddata.ptr = reinterpret_cast<T *>(data);
+
+    } else {
+        // Allocate a temporary buffer with proper alignment
+        tempbuffer = std::make_unique<AlignedBuffer[]>(size / sizeof(T));
+        // size = ctbDigitaldbt;
+
+        std::memcpy(tempbuffer.get(), data, size);
+        aligneddata.buffer = std::move(tempbuffer);
+        aligneddata.ptr = reinterpret_cast<T *>(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;
+    AlignedData<uint64_t> aligned_data(nullptr, nullptr);
-    uint64_t *source = nullptr;
+    AlignData<uint64_t>(aligned_data, data + nAnalogDataBytes + ctbDbitOffset,
-    using AlignedBuffer =
+                        ctbDigitalDataBytes);
-        std::aligned_storage<sizeof(uint64_t), alignof(uint64_t)>::type;
-    std::unique_ptr<AlignedBuffer[]> tempbuffer;
 
-    if (reinterpret_cast<uintptr_t>(ptr) % alignof(uint64_t) == 0) {
+    uint64_t *source = aligned_data.ptr;
-        // If aligned, directly cast to uint64_t pointer
-        source = reinterpret_cast<uint64_t *>(ptr);
-    } else {
-        // Allocate a temporary buffer with proper alignment
-        tempbuffer = std::make_unique<AlignedBuffer[]>(ctbDigitalDataBytes /
-                                                       sizeof(uint64_t));
-
-        std::memcpy(tempbuffer.get(), ptr, ctbDigitalDataBytes);
-        source = reinterpret_cast<uint64_t *>(tempbuffer.get());
-    }
-
-    // auto *source = (uint64_t *)(data + nAnalogDataBytes + ctbDbitOffset);
-    //  TODO: leads to unaligned data
 
     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<uint64_t> aligned_data(nullptr, nullptr);
+    AlignData<uint64_t>(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

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<int> ctbDbitList;
     bool reorder{false}; // true if data should be reordered TODO: add as mode

slsReceiverSoftware/src/GeneralData.h

@@ -60,8 +60,8 @@ class GeneralData {
     slsDetectorDefs::frameDiscardPolicy frameDiscardMode{
         slsDetectorDefs::NO_DISCARD};
 
-    GeneralData(){};
+    GeneralData() {};
-    virtual ~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

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