changeset 3834:219de90c1f43

Added a flag to use an extra buffer in PamReader to ensure (malloc-like) alignment of the image buffer. This flag is NOT turned on by default and, in Orthanc, is only used by the Unit tests.
author Benjamin Golinvaux <bgo@osimis.io>
date Wed, 15 Apr 2020 16:58:28 +0200
parents 83ea6939293d
children cbe847575c62
files Core/Images/PamReader.cpp Core/Images/PamReader.h UnitTestsSources/ImageTests.cpp
diffstat 3 files changed, 109 insertions(+), 2 deletions(-) [+]
line wrap: on
line diff
--- a/Core/Images/PamReader.cpp	Fri Apr 10 17:56:12 2020 +0200
+++ b/Core/Images/PamReader.cpp	Wed Apr 15 16:58:28 2020 +0200
@@ -200,7 +200,30 @@
     }
 
     size_t offset = content_.size() - pitch * height;
-    AssignWritable(format, width, height, pitch, &content_[offset]);
+
+    {
+      intptr_t bufferAddr = reinterpret_cast<intptr_t>(&content_[offset]);
+      if((bufferAddr % 8) == 0)
+        LOG(TRACE) << "PamReader::ParseContent() image address = " << bufferAddr;
+      else
+        LOG(TRACE) << "PamReader::ParseContent() image address = " << bufferAddr << " (not a multiple of 8!)";
+    }
+    
+    // if we want to enforce alignment, we need to use a freshly allocated
+    // buffer, since we have no alignment guarantees on the original one
+    if (enforceAligned_)
+    {
+      if (alignedImageBuffer_ != NULL)
+        free(alignedImageBuffer_);
+      alignedImageBuffer_ = malloc(pitch * height);
+      memcpy(alignedImageBuffer_, &content_[offset], pitch* height);
+      content_ = "";
+      AssignWritable(format, width, height, pitch, alignedImageBuffer_);
+    }
+    else
+    {
+      AssignWritable(format, width, height, pitch, &content_[offset]);
+    }
 
     // Byte swapping if needed
     if (bytesPerChannel != 1 &&
@@ -231,7 +254,7 @@
             at SAFE_HEAP_LOAD_i32_2_2 (wasm-function[251132]:39)
             at __ZN7Orthanc9PamReader12ParseContentEv (wasm-function[11457]:8088)
 
-          Web Assenmbly IS LITTLE ENDIAN!
+          Web Assembly IS LITTLE ENDIAN!
 
           Perhaps in htobe16 ?
           */
--- a/Core/Images/PamReader.h	Fri Apr 10 17:56:12 2020 +0200
+++ b/Core/Images/PamReader.h	Wed Apr 15 16:58:28 2020 +0200
@@ -46,9 +46,45 @@
   private:
     void ParseContent();
     
+    /**
+    Whether we want to use the default malloc alignment in the image buffer,
+    at the expense of an extra copy
+    */
+    bool enforceAligned_;
+
+    /**
+    This is actually a copy of wrappedContent_, but properly aligned.
+
+    It is only used if the enforceAligned parameter is set to true in the
+    constructor.
+    */
+    void* alignedImageBuffer_;
+    
+    /**
+    Points somewhere in the content_ buffer.      
+    */
+    ImageAccessor wrappedContent_;
+
+    /**
+    Raw content (file bytes or answer from the server, for instance). 
+    */
     std::string content_;
 
   public:
+    /**
+    See doc for field enforceAligned_
+    */
+    PamReader(bool enforceAligned = false) :
+      enforceAligned_(enforceAligned),
+      alignedImageBuffer_(NULL)
+    {
+    }
+
+    virtual ~PamReader()
+    {
+      // freeing NULL is OK
+      free(alignedImageBuffer_);
+    }
 
 #if ORTHANC_SANDBOXED == 0
     void ReadFromFile(const std::string& filename);
--- a/UnitTestsSources/ImageTests.cpp	Fri Apr 10 17:56:12 2020 +0200
+++ b/UnitTestsSources/ImageTests.cpp	Wed Apr 15 16:58:28 2020 +0200
@@ -410,6 +410,28 @@
   }
 
   {
+    // true means "enforce alignment by using a temporary buffer"
+    Orthanc::PamReader r(true);
+    r.ReadFromMemory(s);
+
+    ASSERT_EQ(r.GetFormat(), Orthanc::PixelFormat_Grayscale16);
+    ASSERT_EQ(r.GetWidth(), width);
+    ASSERT_EQ(r.GetHeight(), height);
+
+    v = 0;
+    for (unsigned int y = 0; y < height; y++)
+    {
+      const uint16_t* p = reinterpret_cast<const uint16_t*>
+        ((const uint8_t*)r.GetConstBuffer() + y * r.GetPitch());
+      ASSERT_EQ(p, r.GetConstRow(y));
+      for (unsigned int x = 0; x < width; x++, p++, v++)
+      {
+        ASSERT_EQ(v, *p);
+      }
+    }
+  }
+
+  {
     Orthanc::TemporaryFile tmp;
     tmp.Write(s);
 
@@ -432,4 +454,30 @@
       }
     }
   }
+
+  {
+    Orthanc::TemporaryFile tmp;
+    tmp.Write(s);
+
+    // true means "enforce alignment by using a temporary buffer"
+    Orthanc::PamReader r2(true);
+    r2.ReadFromFile(tmp.GetPath());
+
+    ASSERT_EQ(r2.GetFormat(), Orthanc::PixelFormat_Grayscale16);
+    ASSERT_EQ(r2.GetWidth(), width);
+    ASSERT_EQ(r2.GetHeight(), height);
+
+    v = 0;
+    for (unsigned int y = 0; y < height; y++)
+    {
+      const uint16_t* p = reinterpret_cast<const uint16_t*>
+        ((const uint8_t*)r2.GetConstBuffer() + y * r2.GetPitch());
+      ASSERT_EQ(p, r2.GetConstRow(y));
+      for (unsigned int x = 0; x < width; x++, p++, v++)
+      {
+        ASSERT_EQ(*p, v);
+      }
+    }
+  }
+
 }