attr: refactor colourspace conversion to support other formats

This commit enables signalling a colour matrix index using
cicp_matrix_coefficients_idx (ISO/IEC 23001-8 codec independent code
points).

It adds the following options that replace the existing colorTransform
option:

 - colourMatrix: The coded representation according to ISO/IEC 23001-8.

 - convertPlyColourspace: Enables conversion to/from RGB using the
   indicated matrix.

Configuration files are updated to use the new option and to
signal the identity matrix in the case of direct GBR coding.

cfg/octree-liftt-ctc-lossless-geom-lossy-attrs.yaml

@@ -23,7 +23,7 @@ categories:
      #  - automatically derive dist2 based on single initial value by the encoder:
      #    - the initial dist2 is scaled by positionQuantisationScale
      #    - generates dist2 per lod
-     - colorTransform: 1
+     - convertPlyColourspace: 1
      -
        - !conditional '${reflectance8b16b_scale_factor}'
        - hack.reflectanceScale: ${reflectance8b16b_scale_factor}
@@ -71,7 +71,7 @@ categories:
 
     decflags:
      - mode: 1
-     - colorTransform: 1
+     - convertPlyColourspace: 1
      -
        - !conditional '${reflectance8b16b_scale_factor}'
        - hack.reflectanceScale: ${reflectance8b16b_scale_factor}

cfg/octree-liftt-ctc-lossy-geom-lossy-attrs.yaml

@@ -32,7 +32,7 @@ categories:
      #    - generates dist2 per lod
      -
        - !conditional 'defined ${seq_lod} && defined ${seq_dist2}'
-       - colorTransform: 1
+       - convertPlyColourspace: 1
        -
          - !conditional '${reflectance8b16b_scale_factor}'
          - hack.reflectanceScale: ${reflectance8b16b_scale_factor}
@@ -80,7 +80,7 @@ categories:
 
     decflags:
      - mode: 1
-     - colorTransform: 1
+     - convertPlyColourspace: 1
      -
        - !conditional '${reflectance8b16b_scale_factor}'
        - hack.reflectanceScale: ${reflectance8b16b_scale_factor}

cfg/octree-predt-ctc-lossless-geom-lossless-attrs.yaml

@@ -29,7 +29,7 @@ categories:
      #    - generates dist2 per lod
      -
        - !conditional 'defined ${seq_lod} && defined ${seq_dist2}'
-       - colorTransform: 0
+       - convertPlyColourspace: 0
        -
          - !conditional '${reflectance8b16b_scale_factor}'
          - hack.reflectanceScale: ${reflectance8b16b_scale_factor}
@@ -62,11 +62,12 @@ categories:
          - qp: 4
          - qpChromaOffset: 0
          - bitdepth: 8
+         - colourMatrix: 0
          - attribute: color
 
     decflags:
      - mode: 1
-     - colorTransform: 0
+     - convertPlyColourspace: 0
      -
        - !conditional '${reflectance8b16b_scale_factor}'
        - hack.reflectanceScale: ${reflectance8b16b_scale_factor}

cfg/octree-predt-ctc-lossless-geom-nearlossless-attrs.yaml

@@ -19,13 +19,13 @@ categories:
 
      ####
      # attribute coding (common options -- relies on option ordering)
-     #  - avoid measuring loss of colourspace conversion
+     #  - code directly GBR (no need to perform colourspace conversion)
      #  - scale 16bit reflectance data to 8bit
      #  - use predicting transform for lossless conditions
      #  - automatically derive dist2 based on single initial value by the encoder:
      #    - the initial dist2 is scaled by positionQuantisationScale
      #    - generates dist2 per lod
-     - colorTransform: 0
+     - convertPlyColourspace: 0
      -
        - !conditional '${reflectance8b16b_scale_factor}'
        - hack.reflectanceScale: ${reflectance8b16b_scale_factor}
@@ -68,11 +68,12 @@ categories:
            r05: 34
        - qpChromaOffset: 0
        - bitdepth: 8
+       - colourMatrix: 0
        - attribute: color
 
     decflags:
      - mode: 1
-     - colorTransform: 0
+     - convertPlyColourspace: 0
      -
        - !conditional '${reflectance8b16b_scale_factor}'
        - hack.reflectanceScale: ${reflectance8b16b_scale_factor}

cfg/octree-raht-ctc-lossless-geom-lossy-attrs.yaml

@@ -20,7 +20,7 @@ categories:
      # attribute coding (common options -- relies on option ordering)
      #  - uses raht transform
      #  - scale 16bit reflectance data to 8bit
-     - colorTransform: 1
+     - convertPlyColourspace: 1
      -
        - !conditional '${reflectance8b16b_scale_factor}'
        - hack.reflectanceScale: ${reflectance8b16b_scale_factor}
@@ -58,7 +58,7 @@ categories:
 
     decflags:
      - mode: 1
-     - colorTransform: 1
+     - convertPlyColourspace: 1
      -
        - !conditional '${reflectance8b16b_scale_factor}'
        - hack.reflectanceScale: ${reflectance8b16b_scale_factor}

cfg/octree-raht-ctc-lossy-geom-lossy-attrs.yaml

@@ -25,7 +25,7 @@ categories:
      # attribute coding (common options -- relies on option ordering)
      #  - use raht
      #  - scale 16bit reflectance data to 8bit
-     - colorTransform: 1
+     - convertPlyColourspace: 1
      -
        - !conditional '${reflectance8b16b_scale_factor}'
        - hack.reflectanceScale: ${reflectance8b16b_scale_factor}
@@ -63,7 +63,7 @@ categories:
 
     decflags:
      - mode: 1
-     - colorTransform: 1
+     - convertPlyColourspace: 1
      -
        - !conditional '${reflectance8b16b_scale_factor}'
        - hack.reflectanceScale: ${reflectance8b16b_scale_factor}

cfg/trisoup-liftt-ctc-lossy-geom-lossy-attrs.yaml

@@ -28,7 +28,7 @@ categories:
      #  - automatically derive dist2 based on single initial value by the encoder:
      #    - the initial dist2 is scaled by positionQuantisationScale
      #    - generates dist2 per lod
-     - colorTransform: 1
+     - convertPlyColourspace: 1
      - transformType: 2
      - numberOfNearestNeighborsInPrediction: 3
      - levelOfDetailCount: ${seq_lod}
@@ -58,7 +58,7 @@ categories:
 
     decflags:
      - mode: 1
-     - colorTransform: 1
+     - convertPlyColourspace: 1
 
     pcerrorflags:
      - dropdups: 2

cfg/trisoup-raht-ctc-lossy-geom-lossy-attrs.yaml

@@ -25,7 +25,7 @@ categories:
      ####
      # attribute coding (common options -- relies on option ordering)
      #  - use raht
-     - colorTransform: 1
+     - convertPlyColourspace: 1
      - transformType: 1
 
      ##
@@ -46,7 +46,7 @@ categories:
 
     decflags:
      - mode: 1
-     - colorTransform: 1
+     - convertPlyColourspace: 1
 
     pcerrorflags:
      - dropdups: 2

doc/README.options.md

@@ -83,14 +83,13 @@ If outputting non-integer point co-ordinates (eg, due to the output
 geometry scaling), the precision of the binary and ASCII versions are
 not identical.
 
-### `--colourTransform=0|1`
-Controls the use of a colour space transformation before attribute
-coding and after decoding.
-
-  | Value | Description            |
-  |:-----:| ---------------------- |
-  | 0     | none                   |
-  | 1     | RGB to YCbCr (Rec.709) |
+### `--convertPlyColourspace=0|1`
+Controls the conversion of ply RGB colour attributes to/from the
+colourspace set by an attribute's `colourMatrix` before attribute
+coding and after decoding.  When disabled (0), or if there is no
+converter available for the requested `colourMatrix`, no conversion
+happens; however the `colourMatrix` value is still written to the
+bitstream.
 
 ### `--hack.reflectanceScale=0|1`
 Some input data uses 8-bit reflectance data scaled by 255 and represented
@@ -286,6 +285,28 @@ Saves the current attribute configuration for coding the named attribute.
 This option must be specified after the options corresponding to
 the attribute.
 
+### `--colourMatrix=INT-VALUE`
+Indicates the colourspace of the coded attribute values according to
+the ISO/IEC 23001-8 Codec Independent Code Points for ColourMatrix.
+When used in conjunction with `convertPlyColourspace=1`, a colourspace
+conversion will be performed at the input/output of the encoder and
+decoder if supported.
+
+  | Value | RGB converter | Description                               |
+  |:-----:|:-------------:|------------------------------------------ |
+  | 0     | n/a           | Direct coding (eg, RGB, XYZ)              |
+  | 1     | Yes           | YCbCr ITU-R BT.709                        |
+  | 2     | n/a           | Unspecified                               |
+  | 3     | n/a           | Reserved                                  |
+  | 4     | No            | USA Title 47 CFR 73.682 (a)(20)           |
+  | 5     | No            | YCbCr ITU-R BT.601                        |
+  | 6     | No            | YCbCr SMPTE 170M                          |
+  | 7     | No            | YCbCr SMPTE 240M                          |
+  | 8     | No            | YCgCo / YCgCoR                            |
+  | 9     | No            | YCbCr ITU-R BT.2020                       |
+  | 10    | No            | YCbCr ITU-R BT.2020 (constant luminance)  |
+  | 11    | No            | YDzDx SMPTE ST 2085                       |
+
 ### `--bitdepth=INT-VALUE`
 The bitdepth of the attribute data.  NB, this is not necessarily the
 same as the bitdepth of the PLY property.  

tmc3/TMC3.cpp

@@ -82,8 +82,8 @@ struct Parameters {
   pcc::EncoderParams encoder;
   pcc::DecoderParams decoder;
 
-  // todo(df): this should be per-attribute
-  ColorTransform colorTransform;
+  // perform attribute colourspace conversion on ply input/output.
+  bool convertColourspace;
 
   // todo(df): this should be per-attribute
   int reflectanceScale;
@@ -141,6 +141,11 @@ private:
 
 //============================================================================
 
+void convertToGbr(const SequenceParameterSet& sps, PCCPointSet3& cloud);
+void convertFromGbr(const SequenceParameterSet& sps, PCCPointSet3& cloud);
+
+//============================================================================
+
 int
 main(int argc, char* argv[])
 {
@@ -203,17 +208,19 @@ readUInt(std::istream& in, T& val)
 
 namespace pcc {
 static std::istream&
-operator>>(std::istream& in, AxisOrder& val)
+operator>>(std::istream& in, ColourMatrix& val)
 {
   return readUInt(in, val);
 }
 }  // namespace pcc
 
+namespace pcc {
 static std::istream&
-operator>>(std::istream& in, ColorTransform& val)
+operator>>(std::istream& in, AxisOrder& val)
 {
   return readUInt(in, val);
 }
+}  // namespace pcc
 
 namespace pcc {
 static std::istream&
@@ -231,6 +238,31 @@ operator>>(std::istream& in, PartitionMethod& val)
 }
 }  // namespace pcc
 
+namespace pcc {
+static std::ostream&
+operator<<(std::ostream& out, const ColourMatrix& val)
+{
+  switch (val) {
+  case ColourMatrix::kIdentity: out << "0 (Identity)"; break;
+  case ColourMatrix::kBt709: out << "1 (Bt709)"; break;
+  case ColourMatrix::kUnspecified: out << "2 (Unspecified)"; break;
+  case ColourMatrix::kReserved_3: out << "3 (Reserved)"; break;
+  case ColourMatrix::kUsa47Cfr73dot682a20:
+    out << "4 (Usa47Cfr73dot682a20)";
+    break;
+  case ColourMatrix::kBt601: out << "5 (Bt601)"; break;
+  case ColourMatrix::kSmpte170M: out << "6 (Smpte170M)"; break;
+  case ColourMatrix::kSmpte240M: out << "7 (Smpte240M)"; break;
+  case ColourMatrix::kYCgCo: out << "8 (kYCgCo)"; break;
+  case ColourMatrix::kBt2020Ncl: out << "9 (Bt2020Ncl)"; break;
+  case ColourMatrix::kBt2020Cl: out << "10 (Bt2020Cl)"; break;
+  case ColourMatrix::kSmpte2085: out << "11 (Smpte2085)"; break;
+  default: out << "Unknown"; break;
+  }
+  return out;
+}
+}  // namespace pcc
+
 namespace pcc {
 static std::ostream&
 operator<<(std::ostream& out, const AxisOrder& val)
@@ -391,14 +423,11 @@ ParseParameters(int argc, char* argv[], Parameters& params)
     params.outputBinaryPly, false,
     "Output ply files using binary (or otherwise ascii) format")
 
-  // general
-  // todo(df): this should be per-attribute
-  ("colorTransform",
-    params.colorTransform, COLOR_TRANSFORM_RGB_TO_YCBCR,
-    "The colour transform to be applied:\n"
-    "  0: none\n"
-    "  1: RGB to YCbCr (Rec.709)")
+  ("convertPlyColourspace",
+    params.convertColourspace, true,
+    "Convert ply colourspace according to attribute colourMatrix")
 
+  // general
   // todo(df): this should be per-attribute
   ("hack.reflectanceScale",
     params.reflectanceScale, 1,
@@ -527,6 +556,13 @@ ParseParameters(int argc, char* argv[], Parameters& params)
     params_attr.desc.attr_bitdepth, 8,
     "Attribute bitdepth")
 
+  // todo(df): this should be per-attribute
+  ("colourMatrix",
+    params_attr.desc.cicp_matrix_coefficients_idx, ColourMatrix::kBt709,
+    "Matrix used in colourspace conversion\n"
+    "  0: none (identity)\n"
+    "  1: ITU-T BT.709")
+
   ("transformType",
     params_attr.aps.attr_encoding, AttributeEncoding::kPredictingTransform,
     "Coding method to use for attribute:\n"
@@ -713,10 +749,25 @@ ParseParameters(int argc, char* argv[], Parameters& params)
     auto& attr_aps = params.encoder.aps[it.second];
     auto& attr_enc = params.encoder.attr[it.second];
 
-    // Avoid wasting bits signalling chroma quant step size for reflectance
+    // default values for attribute
+    attr_sps.cicp_colour_primaries_idx = 2;
+    attr_sps.cicp_transfer_characteristics_idx = 2;
+    attr_sps.cicp_video_full_range_flag = true;
+
     if (it.first == "reflectance") {
+      // Avoid wasting bits signalling chroma quant step size for reflectance
       attr_aps.aps_chroma_qp_offset = 0;
       attr_enc.abh.attr_layer_qp_delta_chroma.clear();
+
+      // There is no matrix for reflectace
+      attr_sps.cicp_matrix_coefficients_idx = ColourMatrix::kUnspecified;
+      attr_sps.attr_num_dimensions = 1;
+      attr_sps.attributeLabel = KnownAttributeLabel::kReflectance;
+    }
+
+    if (it.first == "color") {
+      attr_sps.attr_num_dimensions = 3;
+      attr_sps.attributeLabel = KnownAttributeLabel::kColour;
     }
 
     bool isLifting =
@@ -960,9 +1011,8 @@ SequenceEncoder::compressOneFrame(Stopwatch* clock)
 
   clock->start();
 
-  if (params->colorTransform == COLOR_TRANSFORM_RGB_TO_YCBCR) {
-    convertGbrToYCbCrBt709(pointCloud);
-  }
+  if (params->convertColourspace)
+    convertFromGbr(params->encoder.sps, pointCloud);
 
   if (params->reflectanceScale > 1 && pointCloud.hasReflectances()) {
     const auto pointCount = pointCloud.getPointCount();
@@ -995,9 +1045,8 @@ SequenceEncoder::compressOneFrame(Stopwatch* clock)
   clock->stop();
 
   if (!params->reconstructedDataPath.empty()) {
-    if (params->colorTransform == COLOR_TRANSFORM_RGB_TO_YCBCR) {
-      convertYCbCrBt709ToGbr(*reconPointCloud);
-    }
+    if (params->convertColourspace)
+      convertToGbr(params->encoder.sps, *reconPointCloud);
 
     if (params->reflectanceScale > 1 && reconPointCloud->hasReflectances()) {
       const auto pointCount = reconPointCloud->getPointCount();
@@ -1036,13 +1085,13 @@ SequenceEncoder::onPostRecolour(const PCCPointSet3& cloud)
   std::string plyName{expandNum(params->postRecolorPath, frameNum)};
 
   // todo(df): stop the clock
-  if (params->colorTransform != COLOR_TRANSFORM_RGB_TO_YCBCR) {
+  if (!params->convertColourspace) {
     ply::write(cloud, _plyAttrNames, plyName, !params->outputBinaryPly);
     return;
   }
 
   PCCPointSet3 tmpCloud(cloud);
-  convertYCbCrBt709ToGbr(tmpCloud);
+  convertToGbr(params->encoder.sps, tmpCloud);
   ply::write(tmpCloud, _plyAttrNames, plyName, !params->outputBinaryPly);
 }
 
@@ -1104,9 +1153,8 @@ SequenceDecoder::onOutputCloud(
   // copy the point cloud in order to modify it according to the output options
   PCCPointSet3 pointCloud(decodedPointCloud);
 
-  if (params->colorTransform == COLOR_TRANSFORM_RGB_TO_YCBCR) {
-    convertYCbCrBt709ToGbr(pointCloud);
-  }
+  if (params->convertColourspace)
+    convertToGbr(sps, pointCloud);
 
   if (params->reflectanceScale > 1 && pointCloud.hasReflectances()) {
     const auto pointCount = pointCloud.getPointCount();
@@ -1142,3 +1190,50 @@ SequenceDecoder::onOutputCloud(
   // todo(df): frame number should be derived from the bitstream
   frameNum++;
 }
+
+//============================================================================
+
+const AttributeDescription*
+findColourAttrDesc(const SequenceParameterSet& sps)
+{
+  // todo(df): don't assume that there is only one colour attribute in the sps
+  for (const auto& desc : sps.attributeSets) {
+    if (desc.attributeLabel == KnownAttributeLabel::kColour)
+      return &desc;
+  }
+  return nullptr;
+}
+
+//----------------------------------------------------------------------------
+
+void
+convertToGbr(const SequenceParameterSet& sps, PCCPointSet3& cloud)
+{
+  const AttributeDescription* attrDesc = findColourAttrDesc(sps);
+  if (!attrDesc)
+    return;
+
+  switch (attrDesc->cicp_matrix_coefficients_idx) {
+  case ColourMatrix::kBt709: convertYCbCrBt709ToGbr(cloud); break;
+
+  default: break;
+  }
+}
+
+//----------------------------------------------------------------------------
+
+void
+convertFromGbr(const SequenceParameterSet& sps, PCCPointSet3& cloud)
+{
+  const AttributeDescription* attrDesc = findColourAttrDesc(sps);
+  if (!attrDesc)
+    return;
+
+  switch (attrDesc->cicp_matrix_coefficients_idx) {
+  case ColourMatrix::kBt709: convertGbrToYCbCrBt709(cloud); break;
+
+  default: break;
+  }
+}
+
+//============================================================================

tmc3/TMC3.h

@@ -42,12 +42,6 @@
 
 #include "pcc_chrono.h"
 
-enum ColorTransform
-{
-  COLOR_TRANSFORM_NONE = 0,
-  COLOR_TRANSFORM_RGB_TO_YCBCR = 1
-};
-
 struct Parameters;
 
 typedef pcc::chrono::Stopwatch<pcc::chrono::utime_inc_children_clock>

tmc3/encoder.cpp

@@ -289,21 +289,6 @@ PCCTMC3Encoder3::fixupParameterSets(EncoderParams* params)
     auto& attr_enc = params->attr[it.second];
     attr_sps.attr_instance_id = it.second;
 
-    attr_sps.cicp_colour_primaries_idx = 2;
-    attr_sps.cicp_transfer_characteristics_idx = 2;
-    attr_sps.cicp_matrix_coefficients_idx = 2;
-    attr_sps.cicp_video_full_range_flag = true;
-
-    if (it.first == "color") {
-      attr_sps.attr_num_dimensions = 3;
-      attr_sps.attributeLabel = KnownAttributeLabel::kColour;
-    }
-
-    if (it.first == "reflectance") {
-      attr_sps.attr_num_dimensions = 1;
-      attr_sps.attributeLabel = KnownAttributeLabel::kReflectance;
-    }
-
     // the encoder options may not specify sufficient offsets for the number
     // of layers used by the sytax: extend with last value as appropriate
     if (!attr_enc.abh.attr_layer_qp_delta_luma.empty()) {

tmc3/hls.h

@@ -88,6 +88,14 @@ struct AttributeLabel {
 
   //--------------------------------------------------------------------------
 
+  friend bool
+  operator!=(const AttributeLabel& lhs, const KnownAttributeLabel& rhs)
+  {
+    return !(lhs == rhs);
+  }
+
+  //--------------------------------------------------------------------------
+
   bool known_attribute_label_flag() const
   {
     switch (KnownAttributeLabel(attribute_label_four_bytes)) {
@@ -133,6 +141,24 @@ enum class AxisOrder
   kXYZ_7 = 7,
 };
 
+//============================================================================
+// ISO/IEC 23001-8 codec independent code points
+enum class ColourMatrix : uint8_t
+{
+  kIdentity = 0,
+  kBt709 = 1,
+  kUnspecified = 2,
+  kReserved_3 = 3,
+  kUsa47Cfr73dot682a20 = 4,
+  kBt601 = 5,
+  kSmpte170M = 6,
+  kSmpte240M = 7,
+  kYCgCo = 8,
+  kBt2020Ncl = 9,
+  kBt2020Cl = 10,
+  kSmpte2085 = 11,
+};
+
 //============================================================================
 
 // invariant properties
@@ -142,7 +168,7 @@ struct AttributeDescription {
   int attr_bitdepth;
   int cicp_colour_primaries_idx;
   int cicp_transfer_characteristics_idx;
-  int cicp_matrix_coefficients_idx;
+  ColourMatrix cicp_matrix_coefficients_idx;
   bool cicp_video_full_range_flag;
 
   AttributeLabel attributeLabel;