Improve the MID Raw buffer

- Allow to navigate on RDHs
- Prevent crash on empty HBs
- Save the last HB page in case it was not fully consumed (this prevents the invalidation of the pointer to the last RDH)

Author: dstocco

Detectors/MUON/MID/Raw/include/MIDRaw/RawBuffer.h

@@ -28,14 +28,21 @@ template <typename T>
 class RawBuffer
 {
  public:
-  void setBuffer(gsl::span<const T> bytes, bool keepUnconsumed = true);
+  enum class ResetMode {
+    all,            // Reset buffer and indexes
+    keepUnconsumed, // Keep the last unconsumed HB
+    bufferOnly      // Do not reset indexes
+  };
+
+  void setBuffer(gsl::span<const T> bytes, ResetMode resetMode = ResetMode::keepUnconsumed);
 
   /// Gets the current RDH
   const header::RAWDataHeader* getRDH() { return mRDH; }
 
   unsigned int next(unsigned int nBits);
   T next();
-  bool nextHeader(gsl::span<const T> bytes);
+
+  bool nextHeader();
 
   bool hasNext(unsigned int nBytes);
 
@@ -46,9 +53,10 @@ class RawBuffer
  private:
   gsl::span<const T> mBytes{};                                    /// gsl span with encoded information
   gsl::span<const T> mCurrentBuffer{};                            /// gsl span with the current encoded information
-  std::vector<T> mUnconsumed;                                     /// Unconsumed buffer
+  std::vector<T> mUnconsumed{};                                   /// Unconsumed buffer
   size_t mElementIndex{0};                                        /// Index of the current element in the buffer
   size_t mBitIndex{0};                                            /// Index of the current bit
+  size_t mHeaderIndex{0};                                         /// Index of the current header
   size_t mNextHeaderIndex{0};                                     /// Index of the next header
   size_t mEndOfPayloadIndex{0};                                   /// Index of the end of payload
   const unsigned int mElementSizeInBytes{sizeof(T)};              /// Element size in bytes

Detectors/MUON/MID/Raw/src/RawBuffer.cxx

@@ -27,6 +27,10 @@ template <typename T>
 unsigned int RawBuffer<T>::next(unsigned int nBits)
 {
   /// Reads the next nBits
+  if (mNextHeaderIndex == 0) {
+    // This is the first entry
+    nextPayload();
+  }
   unsigned int value = 0;
   for (int ibit = 0; ibit < nBits; ++ibit) {
     if (mBitIndex == mElementSizeInBits) {
@@ -51,31 +55,42 @@ T RawBuffer<T>::next()
   return mBytes[mElementIndex++];
 }
 
+template <typename T>
+bool RawBuffer<T>::nextHeader()
+{
+  /// Goes to next RDH
+  if (mNextHeaderIndex >= mBytes.size()) {
+    // This is the end of the buffer
+    if (mUnconsumed.empty()) {
+      mElementIndex = mNextHeaderIndex;
+      mEndOfPayloadIndex = mNextHeaderIndex;
+      return false;
+    }
+    // We were reading the unconsumed part: switch to new buffer
+    mUnconsumed.clear();
+    reset();
+    mBytes = mCurrentBuffer;
+  }
+  mHeaderIndex = mNextHeaderIndex;
+  mRDH = reinterpret_cast<const header::RAWDataHeader*>(&mBytes[mHeaderIndex]);
+  mEndOfPayloadIndex = mHeaderIndex + (mRDH->memorySize / mElementSizeInBytes);
+  // Go to end of header, i.e. beginning of payload
+  mElementIndex = mHeaderIndex + (mRDH->headerSize / mElementSizeInBytes);
+  mNextHeaderIndex = mHeaderIndex + mRDH->offsetToNext / mElementSizeInBytes;
+  mBitIndex = 0;
+
+  return true;
+}
+
 template <typename T>
 bool RawBuffer<T>::nextPayload()
 {
   /// Goes to next payload
   while (mElementIndex == mEndOfPayloadIndex) {
-    if (mNextHeaderIndex == mBytes.size()) {
-      // This is the end of the buffer
-      if (mUnconsumed.empty()) {
-        mElementIndex = mNextHeaderIndex;
-        mEndOfPayloadIndex = mNextHeaderIndex;
-        return false;
-      }
-      // We were reading the unconsumed part: switch to new buffer
-      mUnconsumed.clear();
-      reset();
-      mBytes = mCurrentBuffer;
+    if (!nextHeader()) {
+      return false;
     }
-    mRDH = reinterpret_cast<const header::RAWDataHeader*>(&mBytes[mNextHeaderIndex]);
-    mEndOfPayloadIndex = mNextHeaderIndex + (mRDH->memorySize / mElementSizeInBytes);
-    // Go to end of header, i.e. beginning of payload
-    mElementIndex = mNextHeaderIndex + (mRDH->headerSize / mElementSizeInBytes);
-    mNextHeaderIndex += mRDH->offsetToNext / mElementSizeInBytes;
-    mBitIndex = 0;
   }
-
   return true;
 }
 
@@ -84,6 +99,7 @@ void RawBuffer<T>::reset()
 {
   /// Rewind bytes
   mElementIndex = 0;
+  mHeaderIndex = 0;
   mNextHeaderIndex = 0;
   mEndOfPayloadIndex = 0;
   mBitIndex = 0;
@@ -92,21 +108,23 @@ void RawBuffer<T>::reset()
 }
 
 template <typename T>
-void RawBuffer<T>::setBuffer(gsl::span<const T> bytes, bool keepUnconsumed)
+void RawBuffer<T>::setBuffer(gsl::span<const T> bytes, ResetMode resetMode)
 {
   /// Sets the buffer and reset the internal indexes
-  if (keepUnconsumed && !mUnconsumed.empty()) {
+  if (resetMode == ResetMode::keepUnconsumed && !mUnconsumed.empty()) {
     // There are some unconsumed bytes from the previous buffer
-    mNextHeaderIndex = mUnconsumed.size();
-    mEndOfPayloadIndex -= mElementIndex;
-    mElementIndex = 0;
+    mNextHeaderIndex -= mHeaderIndex;
+    mEndOfPayloadIndex -= mHeaderIndex;
+    mElementIndex -= mHeaderIndex;
+    mHeaderIndex = 0;
     mBytes = gsl::span<const T>(mUnconsumed);
   } else {
     mBytes = bytes;
-    reset();
+    if (resetMode != ResetMode::bufferOnly) {
+      reset();
+    }
   }
   mCurrentBuffer = bytes;
-  nextPayload();
 }
 
 template <typename T>
@@ -144,29 +162,16 @@ bool RawBuffer<T>::hasNext(unsigned int nBytes)
   // With lot of memory left in the buffer but no payload
   nextPayload();
   bool isOk = mCurrentBuffer.size() + mUnconsumed.size() - mNextHeaderIndex + mEndOfPayloadIndex - mElementIndex >= nBytes / mElementSizeInBytes;
-  if (!isOk) {
+  if (!isOk && mElementIndex != mCurrentBuffer.size()) {
     // Store the remaining bits for further use
-    // We need to do it here because we know that the vector of which the mBytes is just a spas, is valid
-    // If we do it in set buffer, this might not be the case anymore
-    std::copy(mBytes.begin() + mElementIndex, mBytes.end(), std::back_inserter(mUnconsumed));
+    // We need to do it here because the vector of which the mBytes is just a span might not be valid afterwards
+    // (e.g. when we do the next setBuffer)
+    // If we do not want to invalidate the mRDH pointer, we need to copy bytes from the last header
+    mUnconsumed.insert(mUnconsumed.end(), mBytes.begin() + mHeaderIndex, mBytes.end());
   }
   return isOk;
 }
 
-template <typename T>
-bool RawBuffer<T>::nextHeader(gsl::span<const T> bytes)
-{
-  /// Go to next header
-  /// This is only useful when one reads from file
-  /// and wants to read up to the next header to find out how many bytes to read next
-  mBytes = bytes;
-  if (mElementIndex >= mBytes.size()) {
-    reset();
-  }
-  mEndOfPayloadIndex = mElementIndex;
-  return nextPayload();
-}
-
 template class RawBuffer<raw::RawUnit>;
 template class RawBuffer<uint8_t>;
 

Detectors/MUON/MID/Raw/src/RawFileReader.cxx

@@ -53,6 +53,7 @@ void RawFileReader<T>::clear()
 {
   /// Clears the bytes and counters
   mBytes.clear();
+  mBuffer.setBuffer(mBytes, RawBuffer<T>::ResetMode::all);
   mHBCounters.fill(0);
 }
 
@@ -136,22 +137,25 @@ bool RawFileReader<T>::readHB(bool sendCompleteHBs)
     }
     replaceRDH(headerIndex);
     // We use the buffer only to correctly initialize the RDH
-    mBuffer.nextHeader(mBytes);
+    mBuffer.setBuffer(mBytes, RawBuffer<T>::ResetMode::bufferOnly);
+    mBuffer.nextHeader();
     isHBClosed = mBuffer.isHBClosed();
     gbtId = mBuffer.getRDH()->feeId;
-    // CAVEAT: do not call mBuffer.getRDH() beyond this line
-    // To save memory/CPU time, the RawBuffer does not hold a copy of the buffer,
-    // but just a span of it.
-    // If we add bytes to mBytes, the vector can go beyond the capacity
-    // and the memory is re-allocated.
-    // If this happens, the span is no longer valid, and we can no longer
-    // call mBuffer.getRDH() until we pass it mBytes again
     if (gbtId >= crateparams::sNGBTs) {
       // FIXME: this is a problem of the header of some test files
       gbtId = 0;
     }
     if (mBuffer.getRDH()->offsetToNext > raw::sHeaderSizeInBytes) {
       read(mBuffer.getRDH()->offsetToNext - raw::sHeaderSizeInBytes);
+      // CAVEAT: to save memory / CPU time, the RawBuffer does not hold a copy of the buffer,
+      // but just a span of it.
+      // If we add bytes to mBytes, the vector can go beyond the capacity
+      // and the memory is re-allocated.
+      // If this happens, the span is no longer valid, and we can no longer
+      // call mBuffer.getRDH() until we pass it mBytes again
+      // To do so, you need to call:
+      // mBuffer.setBuffer(mBytes, RawBuffer<T>::ResetMode::bufferOnly);
+      // mBuffer.nextHeader();
     }
     if (!sendCompleteHBs) {
       break;