[WIP] Larger black level separate pattern size support

.clang-tidy

@@ -137,19 +137,19 @@ CheckOptions:
   - key:             readability-braces-around-statements.ShortStatementLines
     value:           '3'
   - key:             readability-function-size.BranchThreshold
-    value:           '29'
+    value:           '0'
   - key:             readability-function-size.LineThreshold
-    value:           '158'
+    value:           '0'
   - key:             readability-function-size.StatementThreshold
-    value:           '139'
+    value:           '0'
   - key:             readability-function-size.ParameterThreshold
-    value:           '6'
+    value:           '0'
   - key:             readability-function-size.NestingThreshold
-    value:           '7'
+    value:           '0'
   - key:             readability-function-size.VariableThreshold
-    value:           '31'
+    value:           '0'
   - key:             readability-function-cognitive-complexity.Threshold
-    value:           '87'
+    value:           '0'
   - key:             readability-simplify-boolean-expr.ChainedConditionalAssignment
     value:           '1'
   - key:             readability-simplify-boolean-expr.ChainedConditionalReturn

bench/.clang-tidy

@@ -0,0 +1,18 @@
+---
+InheritParentConfig: true
+CheckOptions:
+  - key:             readability-function-size.BranchThreshold
+    value:           '4'
+  - key:             readability-function-size.LineThreshold
+    value:           '51'
+  - key:             readability-function-size.StatementThreshold
+    value:           '36'
+  - key:             readability-function-size.ParameterThreshold
+    value:           '6'
+  - key:             readability-function-size.NestingThreshold
+    value:           '4'
+  - key:             readability-function-size.VariableThreshold
+    value:           '9'
+  - key:             readability-function-cognitive-complexity.Threshold
+    value:           '21'
+...

fuzz/.clang-tidy

@@ -0,0 +1,18 @@
+---
+InheritParentConfig: true
+CheckOptions:
+  - key:             readability-function-size.BranchThreshold
+    value:           '21'
+  - key:             readability-function-size.LineThreshold
+    value:           '77'
+  - key:             readability-function-size.StatementThreshold
+    value:           '74'
+  - key:             readability-function-size.ParameterThreshold
+    value:           '6'
+  - key:             readability-function-size.NestingThreshold
+    value:           '6'
+  - key:             readability-function-size.VariableThreshold
+    value:           '23'
+  - key:             readability-function-cognitive-complexity.Threshold
+    value:           '44'
+...

src/librawspeed/.clang-tidy

@@ -0,0 +1,18 @@
+---
+InheritParentConfig: true
+CheckOptions:
+  - key:             readability-function-size.BranchThreshold
+    value:           '27'
+  - key:             readability-function-size.LineThreshold
+    value:           '153'
+  - key:             readability-function-size.StatementThreshold
+    value:           '109'
+  - key:             readability-function-size.ParameterThreshold
+    value:           '6'
+  - key:             readability-function-size.NestingThreshold
+    value:           '6'
+  - key:             readability-function-size.VariableThreshold
+    value:           '30'
+  - key:             readability-function-cognitive-complexity.Threshold
+    value:           '69'
+...

src/librawspeed/adt/Array1DRef.h

@@ -24,6 +24,7 @@
 #include "adt/Invariant.h"
 #include <cstddef>
 #include <type_traits>
+#include <vector>
 
 namespace rawspeed {
 
@@ -76,6 +77,15 @@ template <class T> class Array1DRef final {
       : data(reinterpret_cast<T*>(RHS.data)),
         numElts(sizeof(T2) * RHS.numElts) {}
 
+  template <typename AllocatorType =
+                typename std::vector<cvless_value_type>::allocator_type>
+  static Array1DRef<T>
+  create(std::vector<cvless_value_type, AllocatorType>& storage, int numElts) {
+    using VectorTy = std::remove_reference_t<decltype(storage)>;
+    storage = VectorTy(numElts);
+    return {storage.data(), numElts};
+  }
+
   [[nodiscard]] CroppedArray1DRef<T> getCrop(int offset, int numElts) const;
 
   [[nodiscard]] int RAWSPEED_READONLY size() const;

src/librawspeed/common/RawImage.h

@@ -160,7 +160,7 @@ class RawImageData : public ErrorLog {
   bool isCFA{true};
   ColorFilterArray cfa;
   int blackLevel = -1;
-  std::array<int, 4> blackLevelSeparateStorage;
+  std::vector<int> blackLevelSeparateStorage;
   Array2DRef<int> blackLevelSeparate;
   int whitePoint = 65536;
   std::vector<BlackArea> blackAreas;

src/librawspeed/common/RawImageDataFloat.cpp

@@ -87,7 +87,7 @@ void RawImageDataFloat::calculateBlackAreas() {
     }
   }
 
-  blackLevelSeparate = Array2DRef(blackLevelSeparateStorage.data(), 2, 2);
+  blackLevelSeparate = Array2DRef<int>::create(blackLevelSeparateStorage, 2, 2);
   auto blackLevelSeparate1D = *blackLevelSeparate.getAsArray1DRef();
 
   if (!totalpixels) {

src/librawspeed/common/RawImageDataU16.cpp

@@ -108,7 +108,7 @@ void RawImageDataU16::calculateBlackAreas() {
     }
   }
 
-  blackLevelSeparate = Array2DRef(blackLevelSeparateStorage.data(), 2, 2);
+  blackLevelSeparate = Array2DRef<int>::create(blackLevelSeparateStorage, 2, 2);
   auto blackLevelSeparate1D = *blackLevelSeparate.getAsArray1DRef();
 
   if (!totalpixels) {

src/librawspeed/decoders/ArwDecoder.cpp

@@ -646,7 +646,7 @@ void ArwDecoder::GetWB() const {
       if (bl->count != 4)
         ThrowRDE("Black Level has %d entries instead of 4", bl->count);
       mRaw->blackLevelSeparate =
-          Array2DRef(mRaw->blackLevelSeparateStorage.data(), 2, 2);
+          Array2DRef<int>::create(mRaw->blackLevelSeparateStorage, 2, 2);
       auto blackLevelSeparate1D = *mRaw->blackLevelSeparate.getAsArray1DRef();
       for (int i = 0; i < 4; ++i)
         blackLevelSeparate1D(i) = bl->getU16(i) >> mShiftDownScaleForExif;

src/librawspeed/decoders/Cr2Decoder.cpp

@@ -407,7 +407,7 @@ bool Cr2Decoder::decodeCanonColorData() const {
   mRaw->whitePoint = wb->getU16(levelOffsets->second);
 
   mRaw->blackLevelSeparate =
-      Array2DRef(mRaw->blackLevelSeparateStorage.data(), 2, 2);
+      Array2DRef<int>::create(mRaw->blackLevelSeparateStorage, 2, 2);
   auto blackLevelSeparate1D = *mRaw->blackLevelSeparate.getAsArray1DRef();
   for (int c = 0; c != 4; ++c)
     blackLevelSeparate1D(c) = wb->getU16(c + levelOffsets->first);

src/librawspeed/decoders/DngDecoder.cpp

@@ -503,6 +503,9 @@ RawImage DngDecoder::decodeRawInternal() {
   if (cpp < 1 || cpp > 4)
     ThrowRDE("Unsupported samples per pixel count: %u.", cpp);
 
+  if (mRaw->isCFA && cpp != 1)
+    ThrowRDE("For CFA images, expecting 1 sample per pixel, got: %u.", cpp);
+
   mRaw->setCpp(cpp);
 
   // Now load the image
@@ -633,7 +636,7 @@ void DngDecoder::handleMetadata(const TiffIFD* raw) {
     mRaw->blackAreas.clear();
     mRaw->blackLevel = 0;
     mRaw->blackLevelSeparate =
-        Array2DRef(mRaw->blackLevelSeparateStorage.data(), 2, 2);
+        Array2DRef<int>::create(mRaw->blackLevelSeparateStorage, 2, 2);
     auto blackLevelSeparate1D = *mRaw->blackLevelSeparate.getAsArray1DRef();
     std::fill(blackLevelSeparate1D.begin(), blackLevelSeparate1D.end(), 0);
     // FIXME: why do we provide both the `blackLevel` and `blackLevelSeparate`?
@@ -791,6 +794,45 @@ bool DngDecoder::decodeMaskedAreas(const TiffIFD* raw) const {
   return !mRaw->blackAreas.empty();
 }
 
+void DngDecoder::decodeBlackLevelDelta(const TiffIFD* raw, TiffTag tag,
+                                       int patSize, int dimSize, auto z) const {
+  using BlackType = decltype(mRaw->blackLevelSeparate)::value_type;
+
+  if (!raw->hasEntry(tag))
+    return;
+
+  const TiffEntry* blackleveldeltah = raw->getEntry(tag);
+  if (static_cast<int>(blackleveldeltah->count) < dimSize)
+    ThrowRDE("BLACKLEVELDELTAH array is too small");
+
+  std::vector<float> sumStorage;
+  auto sum = Array1DRef<float>::create(sumStorage, patSize);
+  std::fill(sum.begin(), sum.end(), 0);
+
+  std::vector<int> numStorage;
+  auto num = Array1DRef<int>::create(numStorage, patSize);
+  std::fill(num.begin(), num.end(), 0);
+
+  for (int y = 0; y < dimSize; y++) {
+    sum(y % patSize) += blackleveldeltah->getFloat(y);
+    num(y % patSize) += 1;
+  }
+
+  for (int y = 0; y < mRaw->blackLevelSeparate.height; y++) {
+    for (int x = 0; x < mRaw->blackLevelSeparate.width; x++) {
+      const int i = z(y, x);
+      const float value = sum(i) / float(num(i));
+      if (static_cast<double>(value) < std::numeric_limits<BlackType>::min() ||
+          static_cast<double>(value) > std::numeric_limits<BlackType>::max())
+        ThrowRDE("Error decoding black level");
+
+      auto& out = mRaw->blackLevelSeparate(y, x);
+      if (__builtin_sadd_overflow(out, implicit_cast<int>(value), &out))
+        ThrowRDE("Integer overflow when calculating black level");
+    }
+  }
+}
+
 bool DngDecoder::decodeBlackLevels(const TiffIFD* raw) const {
   iPoint2D blackdim(1, 1);
   if (raw->hasEntry(TiffTag::BLACKLEVELREPEATDIM)) {
@@ -813,96 +855,35 @@ bool DngDecoder::decodeBlackLevels(const TiffIFD* raw) const {
   if (!raw->hasEntry(TiffTag::BLACKLEVEL))
     return true;
 
-  if (mRaw->getCpp() != 1)
-    return false;
-
   const TiffEntry* black_entry = raw->getEntry(TiffTag::BLACKLEVEL);
-  if (black_entry->count < blackdim.area())
+  if (black_entry->count != mRaw->getCpp() * blackdim.area())
     ThrowRDE("BLACKLEVEL entry is too small");
 
   using BlackType = decltype(mRaw->blackLevelSeparate)::value_type;
 
-  if (blackdim.x < 2 || blackdim.y < 2) {
-    // We so not have enough to fill all individually, read a single and copy it
-    float value = black_entry->getFloat();
-
-    if (static_cast<double>(value) < std::numeric_limits<BlackType>::min() ||
-        static_cast<double>(value) > std::numeric_limits<BlackType>::max())
-      ThrowRDE("Error decoding black level");
-
-    mRaw->blackLevelSeparate =
-        Array2DRef(mRaw->blackLevelSeparateStorage.data(), 2, 2);
-    auto blackLevelSeparate1D = *mRaw->blackLevelSeparate.getAsArray1DRef();
-    for (int y = 0; y < 2; y++) {
-      for (int x = 0; x < 2; x++)
-        blackLevelSeparate1D(y * 2 + x) = implicit_cast<int>(value);
-    }
-  } else {
-    mRaw->blackLevelSeparate =
-        Array2DRef(mRaw->blackLevelSeparateStorage.data(), 2, 2);
-    auto blackLevelSeparate1D = *mRaw->blackLevelSeparate.getAsArray1DRef();
-    for (int y = 0; y < 2; y++) {
-      for (int x = 0; x < 2; x++) {
-        float value = black_entry->getFloat(y * blackdim.x + x);
+  mRaw->blackLevelSeparate = Array2DRef<int>::create(
+      mRaw->blackLevelSeparateStorage, mRaw->getCpp() * blackdim.x, blackdim.y);
 
-        if (static_cast<double>(value) <
-                std::numeric_limits<BlackType>::min() ||
-            static_cast<double>(value) > std::numeric_limits<BlackType>::max())
-          ThrowRDE("Error decoding black level");
-
-        blackLevelSeparate1D(y * 2 + x) = implicit_cast<int>(value);
-      }
-    }
-  }
-
-  // DNG Spec says we must add black in deltav and deltah
-  if (raw->hasEntry(TiffTag::BLACKLEVELDELTAV)) {
-    const TiffEntry* blackleveldeltav =
-        raw->getEntry(TiffTag::BLACKLEVELDELTAV);
-    if (static_cast<int>(blackleveldeltav->count) < mRaw->dim.y)
-      ThrowRDE("BLACKLEVELDELTAV array is too small");
-    std::array<float, 2> black_sum = {{}};
-    for (int i = 0; i < mRaw->dim.y; i++)
-      black_sum[i & 1] += blackleveldeltav->getFloat(i);
+  for (int y = 0; y < mRaw->blackLevelSeparate.height; y++) {
+    for (int x = 0; x < mRaw->blackLevelSeparate.width; x++) {
+      float value =
+          black_entry->getFloat(y * mRaw->blackLevelSeparate.width + x);
 
-    auto blackLevelSeparate1D = *mRaw->blackLevelSeparate.getAsArray1DRef();
-    for (int i = 0; i < 4; i++) {
-      const float value =
-          black_sum[i >> 1] / static_cast<float>(mRaw->dim.y) * 2.0F;
       if (static_cast<double>(value) < std::numeric_limits<BlackType>::min() ||
           static_cast<double>(value) > std::numeric_limits<BlackType>::max())
         ThrowRDE("Error decoding black level");
 
-      if (__builtin_sadd_overflow(blackLevelSeparate1D(i),
-                                  implicit_cast<int>(value),
-                                  &blackLevelSeparate1D(i)))
-        ThrowRDE("Integer overflow when calculating black level");
+      mRaw->blackLevelSeparate(y, x) = implicit_cast<int>(value);
     }
   }
 
-  if (raw->hasEntry(TiffTag::BLACKLEVELDELTAH)) {
-    const TiffEntry* blackleveldeltah =
-        raw->getEntry(TiffTag::BLACKLEVELDELTAH);
-    if (static_cast<int>(blackleveldeltah->count) < mRaw->dim.x)
-      ThrowRDE("BLACKLEVELDELTAH array is too small");
-    std::array<float, 2> black_sum = {{}};
-    for (int i = 0; i < mRaw->dim.x; i++)
-      black_sum[i & 1] += blackleveldeltah->getFloat(i);
-
-    auto blackLevelSeparate1D = *mRaw->blackLevelSeparate.getAsArray1DRef();
-    for (int i = 0; i < 4; i++) {
-      const float value =
-          black_sum[i & 1] / static_cast<float>(mRaw->dim.x) * 2.0F;
-      if (static_cast<double>(value) < std::numeric_limits<BlackType>::min() ||
-          static_cast<double>(value) > std::numeric_limits<BlackType>::max())
-        ThrowRDE("Error decoding black level");
+  // DNG Spec says we must add black in deltav and deltah
+  decodeBlackLevelDelta(raw, TiffTag::BLACKLEVELDELTAV, blackdim.y, mRaw->dim.y,
+                        [](int row, int col) { return row; });
+  decodeBlackLevelDelta(
+      raw, TiffTag::BLACKLEVELDELTAH, blackdim.x, mRaw->dim.x,
+      [cpp = mRaw->getCpp()](int row, int col) { return col / cpp; });
 
-      if (__builtin_sadd_overflow(blackLevelSeparate1D(i),
-                                  implicit_cast<int>(value),
-                                  &blackLevelSeparate1D(i)))
-        ThrowRDE("Integer overflow when calculating black level");
-    }
-  }
   return true;
 }
 
@@ -914,7 +895,7 @@ void DngDecoder::setBlack(const TiffIFD* raw) const {
   // Black defaults to 0
   // FIXME: is this the right thing to do?
   mRaw->blackLevelSeparate =
-      Array2DRef(mRaw->blackLevelSeparateStorage.data(), 2, 2);
+      Array2DRef<int>::create(mRaw->blackLevelSeparateStorage, 2, 2);
   auto blackLevelSeparate1D = *mRaw->blackLevelSeparate.getAsArray1DRef();
   std::fill(blackLevelSeparate1D.begin(), blackLevelSeparate1D.end(), 0);
 

src/librawspeed/decoders/DngDecoder.h

@@ -55,6 +55,8 @@ class DngDecoder final : public AbstractTiffDecoder {
   void decodeData(const TiffIFD* raw, uint32_t sample_format) const;
   void handleMetadata(const TiffIFD* raw);
   bool decodeMaskedAreas(const TiffIFD* raw) const;
+  void decodeBlackLevelDelta(const TiffIFD* raw, TiffTag tag, int patSize,
+                             int dimSize, auto z) const;
   bool decodeBlackLevels(const TiffIFD* raw) const;
   void setBlack(const TiffIFD* raw) const;
 

src/librawspeed/decoders/NefDecoder.cpp

@@ -628,7 +628,7 @@ void NefDecoder::decodeMetaDataInternal(const CameraMetaData* meta) {
       ThrowRDE("Bad bit per pixel: %i", bitPerPixel);
     const int sh = 14 - bitPerPixel;
     mRaw->blackLevelSeparate =
-        Array2DRef(mRaw->blackLevelSeparateStorage.data(), 2, 2);
+        Array2DRef<int>::create(mRaw->blackLevelSeparateStorage, 2, 2);
     auto blackLevelSeparate1D = *mRaw->blackLevelSeparate.getAsArray1DRef();
     blackLevelSeparate1D(0) = bl->getU16(0) >> sh;
     blackLevelSeparate1D(1) = bl->getU16(1) >> sh;

src/librawspeed/decoders/OrfDecoder.cpp

@@ -317,7 +317,7 @@ void OrfDecoder::decodeMetaDataInternal(const CameraMetaData* meta) {
       // Order is assumed to be RGGB
       if (blackEntry->count == 4) {
         mRaw->blackLevelSeparate =
-            Array2DRef(mRaw->blackLevelSeparateStorage.data(), 2, 2);
+            Array2DRef<int>::create(mRaw->blackLevelSeparateStorage, 2, 2);
         auto blackLevelSeparate1D = *mRaw->blackLevelSeparate.getAsArray1DRef();
         for (int i = 0; i < 4; i++) {
           auto c = mRaw->cfa.getColorAt(i & 1, i >> 1);

src/librawspeed/decoders/PefDecoder.cpp

@@ -132,7 +132,7 @@ void PefDecoder::decodeMetaDataInternal(const CameraMetaData* meta) {
         mRootIFD->getEntryRecursive(static_cast<TiffTag>(0x200));
     if (black->count == 4) {
       mRaw->blackLevelSeparate =
-          Array2DRef(mRaw->blackLevelSeparateStorage.data(), 2, 2);
+          Array2DRef<int>::create(mRaw->blackLevelSeparateStorage, 2, 2);
       auto blackLevelSeparate1D = *mRaw->blackLevelSeparate.getAsArray1DRef();
       for (int i = 0; i < 4; i++)
         blackLevelSeparate1D(i) = black->getU32(i);