Merge branch 'develop' into bugfix/yn-0118-editorial-publishing-with-no-audio-product

client/ayon_core/lib/attribute_definitions.py

@@ -604,7 +604,11 @@ class EnumDef(AbstractAttrDef):
 
         if value is None:
             return copy.deepcopy(self.default)
-        return list(self._item_values.intersection(value))
+        return [
+            v
+            for v in value
+            if v in self._item_values
+        ]
 
     def is_value_valid(self, value: Any) -> bool:
         """Check if item is available in possible values."""

client/ayon_core/lib/transcoding.py

@@ -110,6 +110,15 @@ def deprecated(new_destination):
     return _decorator(func)
 
 
+class MissingRGBAChannelsError(ValueError):
+    """Raised when we can't find channels to use as RGBA for conversion in
+    input media.
+
+    This may be other channels than solely RGBA, like Z-channel. The error is
+    raised when no matching 'reviewable' channel was found.
+    """
+
+
 def get_transcode_temp_directory():
     """Creates temporary folder for transcoding.
 
@@ -388,6 +397,10 @@ def get_review_info_by_layer_name(channel_names):
             ...
         ]
 
+    This tries to find suitable outputs good for review purposes, by
+    searching for channel names like RGBA, but also XYZ, Z, N, AR, AG, AB
+    channels.
+
     Args:
         channel_names (list[str]): List of channel names.
 
@@ -396,7 +409,6 @@ def get_review_info_by_layer_name(channel_names):
     """
 
     layer_names_order = []
-    rgba_by_layer_name = collections.defaultdict(dict)
     channels_by_layer_name = collections.defaultdict(dict)
 
     for channel_name in channel_names:
@@ -405,45 +417,95 @@ def get_review_info_by_layer_name(channel_names):
         if "." in channel_name:
             layer_name, last_part = channel_name.rsplit(".", 1)
 
-        channels_by_layer_name[layer_name][channel_name] = last_part
+        # R, G, B, A or X, Y, Z, N, AR, AG, AB, RED, GREEN, BLUE, ALPHA
-        if last_part.lower() not in {
+        channel = last_part.upper()
-            "r", "red",
+        if channel not in {
-            "g", "green",
+            # Detect RGBA channels
-            "b", "blue",
+            "R", "G", "B", "A",
-            "a", "alpha"
+            # Support fully written out rgba channel names
+            "RED", "GREEN", "BLUE", "ALPHA",
+            # Allow detecting of x, y and z channels, and normal channels
+            "X", "Y", "Z", "N",
+            # red, green and blue alpha/opacity, for colored mattes
+            "AR", "AG", "AB"
         }:
             continue
 
         if layer_name not in layer_names_order:
             layer_names_order.append(layer_name)
-        # R, G, B or A
+
-        channel = last_part[0].upper()
+        channels_by_layer_name[layer_name][channel] = channel_name
-        rgba_by_layer_name[layer_name][channel] = channel_name
 
     # Put empty layer or 'rgba' to the beginning of the list
     # - if input has R, G, B, A channels they should be used for review
-    # NOTE They are iterated in reversed order because they're inserted to
+    def _sort(_layer_name: str) -> int:
-    #   the beginning of 'layer_names_order' -> last added will be first.
+        # Prioritize "" layer name
-    for name in reversed(["", "rgba"]):
+        # Prioritize layers with RGB channels
-        if name in layer_names_order:
+        if _layer_name == "rgba":
-            layer_names_order.remove(name)
+            return 0
-            layer_names_order.insert(0, name)
+
+        if _layer_name == "":
+            return 1
+
+        channels = channels_by_layer_name[_layer_name]
+        if all(channel in channels for channel in "RGB"):
+            return 2
+        return 10
+    layer_names_order.sort(key=_sort)
 
     output = []
     for layer_name in layer_names_order:
-        rgba_layer_info = rgba_by_layer_name[layer_name]
+        channel_info = channels_by_layer_name[layer_name]
-        red = rgba_layer_info.get("R")
+
-        green = rgba_layer_info.get("G")
+        alpha = channel_info.get("A")
-        blue = rgba_layer_info.get("B")
+
-        if not red or not green or not blue:
+        # RGB channels
+        if all(channel in channel_info for channel in "RGB"):
+            rgb = "R", "G", "B"
+
+        # RGB channels using fully written out channel names
+        elif all(
+            channel in channel_info
+            for channel in ("RED", "GREEN", "BLUE")
+        ):
+            rgb = "RED", "GREEN", "BLUE"
+            alpha = channel_info.get("ALPHA")
+
+        # XYZ channels (position pass)
+        elif all(channel in channel_info for channel in "XYZ"):
+            rgb = "X", "Y", "Z"
+
+        # Colored mattes (as defined in OpenEXR Channel Name standards)
+        elif all(channel in channel_info for channel in ("AR", "AG", "AB")):
+            rgb = "AR", "AG", "AB"
+
+        # Luminance channel (as defined in OpenEXR Channel Name standards)
+        elif "Y" in channel_info:
+            rgb = "Y", "Y", "Y"
+
+        # Has only Z channel (Z-depth layer)
+        elif "Z" in channel_info:
+            rgb = "Z", "Z", "Z"
+
+        # Has only A channel (Alpha layer)
+        elif "A" in channel_info:
+            rgb = "A", "A", "A"
+            alpha = None
+
+        else:
+            # No reviewable channels found
             continue
+
+        red = channel_info[rgb[0]]
+        green = channel_info[rgb[1]]
+        blue = channel_info[rgb[2]]
         output.append({
             "name": layer_name,
             "review_channels": {
                 "R": red,
                 "G": green,
                 "B": blue,
-                "A": rgba_layer_info.get("A"),
+                "A": alpha,
             }
         })
     return output
@@ -1467,8 +1529,9 @@ def get_oiio_input_and_channel_args(oiio_input_info, alpha_default=None):
     review_channels = get_convert_rgb_channels(channel_names)
 
     if review_channels is None:
-        raise ValueError(
+        raise MissingRGBAChannelsError(
-            "Couldn't find channels that can be used for conversion."
+            "Couldn't find channels that can be used for conversion "
+            f"among channels: {channel_names}."
         )
 
     red, green, blue, alpha = review_channels

client/ayon_core/pipeline/create/structures.py

@@ -137,6 +137,7 @@ class AttributeValues:
             if value is None:
                 continue
             converted_value = attr_def.convert_value(value)
+            # QUESTION Could we just use converted value all the time?
             if converted_value == value:
                 self._data[attr_def.key] = value
 
@@ -245,11 +246,11 @@ class AttributeValues:
 
     def _update(self, value):
         changes = {}
-        for key, value in dict(value).items():
+        for key, key_value in dict(value).items():
-            if key in self._data and self._data.get(key) == value:
+            if key in self._data and self._data.get(key) == key_value:
                 continue
-            self._data[key] = value
+            self._data[key] = key_value
-            changes[key] = value
+            changes[key] = key_value
         return changes
 
     def _pop(self, key, default):

client/ayon_core/plugins/publish/extract_color_transcode.py

@@ -11,6 +11,7 @@ from ayon_core.lib import (
     is_oiio_supported,
 )
 from ayon_core.lib.transcoding import (
+    MissingRGBAChannelsError,
     oiio_color_convert,
 )
 
@@ -111,7 +112,17 @@ class ExtractOIIOTranscode(publish.Extractor):
                 self.log.warning("Config file doesn't exist, skipping")
                 continue
 
+            # Get representation files to convert
+            if isinstance(repre["files"], list):
+                repre_files_to_convert = copy.deepcopy(repre["files"])
+            else:
+                repre_files_to_convert = [repre["files"]]
+
+            # Process each output definition
             for output_def in profile_output_defs:
+                # Local copy to avoid accidental mutable changes
+                files_to_convert = list(repre_files_to_convert)
+
                 output_name = output_def["name"]
                 new_repre = copy.deepcopy(repre)
 
@@ -122,11 +133,6 @@ class ExtractOIIOTranscode(publish.Extractor):
                 )
                 new_repre["stagingDir"] = new_staging_dir
 
-                if isinstance(new_repre["files"], list):
-                    files_to_convert = copy.deepcopy(new_repre["files"])
-                else:
-                    files_to_convert = [new_repre["files"]]
-
                 output_extension = output_def["extension"]
                 output_extension = output_extension.replace('.', '')
                 self._rename_in_representation(new_repre,
@@ -168,30 +174,49 @@ class ExtractOIIOTranscode(publish.Extractor):
                 additional_command_args = (output_def["oiiotool_args"]
                                            ["additional_command_args"])
 
-                files_to_convert = self._translate_to_sequence(
+                sequence_files = self._translate_to_sequence(files_to_convert)
-                    files_to_convert)
+                self.log.debug("Files to convert: {}".format(sequence_files))
-                self.log.debug("Files to convert: {}".format(files_to_convert))
+                missing_rgba_review_channels = False
-                for file_name in files_to_convert:
+                for file_name in sequence_files:
+                    if isinstance(file_name, clique.Collection):
+                        # Convert to filepath that can be directly converted
+                        # by oiio like `frame.1001-1025%04d.exr`
+                        file_name: str = file_name.format(
+                            "{head}{range}{padding}{tail}"
+                        )
+
                     self.log.debug("Transcoding file: `{}`".format(file_name))
                     input_path = os.path.join(original_staging_dir,
                                               file_name)
                     output_path = self._get_output_file_path(input_path,
                                                              new_staging_dir,
                                                              output_extension)
+                    try:
+                        oiio_color_convert(
+                            input_path=input_path,
+                            output_path=output_path,
+                            config_path=config_path,
+                            source_colorspace=source_colorspace,
+                            target_colorspace=target_colorspace,
+                            target_display=target_display,
+                            target_view=target_view,
+                            source_display=source_display,
+                            source_view=source_view,
+                            additional_command_args=additional_command_args,
+                            logger=self.log
+                        )
+                    except MissingRGBAChannelsError as exc:
+                        missing_rgba_review_channels = True
+                        self.log.error(exc)
+                        self.log.error(
+                            "Skipping OIIO Transcode. Unknown RGBA channels"
+                            f" for colorspace conversion in file: {input_path}"
+                        )
+                        break
 
-                    oiio_color_convert(
+                if missing_rgba_review_channels:
-                        input_path=input_path,
+                    # Stop processing this representation
-                        output_path=output_path,
+                    break
-                        config_path=config_path,
-                        source_colorspace=source_colorspace,
-                        target_colorspace=target_colorspace,
-                        target_display=target_display,
-                        target_view=target_view,
-                        source_display=source_display,
-                        source_view=source_view,
-                        additional_command_args=additional_command_args,
-                        logger=self.log
-                    )
 
                 # cleanup temporary transcoded files
                 for file_name in new_repre["files"]:
@@ -217,11 +242,11 @@ class ExtractOIIOTranscode(publish.Extractor):
                         added_review = True
 
                 # If there is only 1 file outputted then convert list to
-                # string, cause that'll indicate that its not a sequence.
+                # string, because that'll indicate that it is not a sequence.
                 if len(new_repre["files"]) == 1:
                     new_repre["files"] = new_repre["files"][0]
 
-                # If the source representation has "review" tag, but its not
+                # If the source representation has "review" tag, but it's not
                 # part of the output definition tags, then both the
                 # representations will be transcoded in ExtractReview and
                 # their outputs will clash in integration.
@@ -271,42 +296,34 @@ class ExtractOIIOTranscode(publish.Extractor):
         new_repre["files"] = renamed_files
 
     def _translate_to_sequence(self, files_to_convert):
-        """Returns original list or list with filename formatted in single
+        """Returns original list or a clique.Collection of a sequence.
-        sequence format.
 
-        Uses clique to find frame sequence, in this case it merges all frames
+        Uses clique to find frame sequence Collection.
-        into sequence format (FRAMESTART-FRAMEEND#) and returns it.
+        If sequence not found, it returns original list.
-        If sequence not found, it returns original list
 
         Args:
             files_to_convert (list): list of file names
         Returns:
-            (list) of [file.1001-1010#.exr] or [fileA.exr, fileB.exr]
+            list[str | clique.Collection]: List of filepaths or a list
+                of Collections (usually one, unless there are holes)
         """
         pattern = [clique.PATTERNS["frames"]]
         collections, _ = clique.assemble(
             files_to_convert, patterns=pattern,
             assume_padded_when_ambiguous=True)
-
         if collections:
             if len(collections) > 1:
                 raise ValueError(
                     "Too many collections {}".format(collections))
 
             collection = collections[0]
-            frames = list(collection.indexes)
+            # TODO: Technically oiiotool supports holes in the sequence as well
-            if collection.holes().indexes:
+            #  using the dedicated --frames argument to specify the frames.
-                return files_to_convert
+            #  We may want to use that too so conversions of sequences with
-
+            #  holes will perform faster as well.
-            # Get the padding from the collection
+            # Separate the collection so that we have no holes/gaps per
-            # This is the number of digits used in the frame numbers
+            # collection.
-            padding = collection.padding
+            return collection.separate()
-
-            frame_str = "{}-{}%0{}d".format(frames[0], frames[-1], padding)
-            file_name = "{}{}{}".format(collection.head, frame_str,
-                                        collection.tail)
-
-            files_to_convert = [file_name]
 
         return files_to_convert
 

client/ayon_core/plugins/publish/extract_review.py

@@ -361,14 +361,14 @@ class ExtractReview(pyblish.api.InstancePlugin):
             if not filtered_output_defs:
                 self.log.debug((
                     "Repre: {} - All output definitions were filtered"
-                    " out by single frame filter. Skipping"
+                    " out by single frame filter. Skipped."
                 ).format(repre["name"]))
                 continue
 
             # Skip if file is not set
             if first_input_path is None:
                 self.log.warning((
-                    "Representation \"{}\" have empty files. Skipped."
+                    "Representation \"{}\" has empty files. Skipped."
                 ).format(repre["name"]))
                 continue
 

client/ayon_core/plugins/publish/extract_thumbnail.py

@@ -17,6 +17,7 @@ from ayon_core.lib import (
     run_subprocess,
 )
 from ayon_core.lib.transcoding import (
+    MissingRGBAChannelsError,
     oiio_color_convert,
     get_oiio_input_and_channel_args,
     get_oiio_info_for_input,
@@ -477,7 +478,16 @@ class ExtractThumbnail(pyblish.api.InstancePlugin):
             return False
 
         input_info = get_oiio_info_for_input(src_path, logger=self.log)
-        input_arg, channels_arg = get_oiio_input_and_channel_args(input_info)
+        try:
+            input_arg, channels_arg = get_oiio_input_and_channel_args(
+                input_info
+            )
+        except MissingRGBAChannelsError:
+            self.log.debug(
+                "Unable to find relevant reviewable channel for thumbnail "
+                "creation"
+            )
+            return False
         oiio_cmd = get_oiio_tool_args(
             "oiiotool",
             input_arg, src_path,

client/ayon_core/tools/publisher/models/create.py

@@ -1,5 +1,6 @@
 import logging
 import re
+import copy
 from typing import (
     Union,
     List,
@@ -1098,7 +1099,7 @@ class CreateModel:
                     creator_attributes[key] = attr_def.default
 
                 elif attr_def.is_value_valid(value):
-                    creator_attributes[key] = value
+                    creator_attributes[key] = copy.deepcopy(value)
 
     def _set_instances_publish_attr_values(
         self, instance_ids, plugin_name, key, value

tests/client/ayon_core/lib/test_transcoding.py

@@ -0,0 +1,158 @@
+import unittest
+
+from ayon_core.lib.transcoding import (
+    get_review_info_by_layer_name
+)
+
+
+class GetReviewInfoByLayerName(unittest.TestCase):
+    """Test responses from `get_review_info_by_layer_name`"""
+    def test_rgba_channels(self):
+
+        # RGB is supported
+        info = get_review_info_by_layer_name(["R", "G", "B"])
+        self.assertEqual(info, [{
+            "name": "",
+            "review_channels": {
+                "R": "R",
+                "G": "G",
+                "B": "B",
+                "A": None,
+            }
+        }])
+
+        # rgb is supported
+        info = get_review_info_by_layer_name(["r", "g", "b"])
+        self.assertEqual(info, [{
+            "name": "",
+            "review_channels": {
+                "R": "r",
+                "G": "g",
+                "B": "b",
+                "A": None,
+            }
+        }])
+
+        # diffuse.[RGB] is supported
+        info = get_review_info_by_layer_name(
+            ["diffuse.R", "diffuse.G", "diffuse.B"]
+        )
+        self.assertEqual(info, [{
+            "name": "diffuse",
+            "review_channels": {
+                "R": "diffuse.R",
+                "G": "diffuse.G",
+                "B": "diffuse.B",
+                "A": None,
+            }
+        }])
+
+        info = get_review_info_by_layer_name(["R", "G", "B", "A"])
+        self.assertEqual(info, [{
+            "name": "",
+            "review_channels": {
+                "R": "R",
+                "G": "G",
+                "B": "B",
+                "A": "A",
+            }
+        }])
+
+    def test_z_channel(self):
+
+        info = get_review_info_by_layer_name(["Z"])
+        self.assertEqual(info, [{
+            "name": "",
+            "review_channels": {
+                "R": "Z",
+                "G": "Z",
+                "B": "Z",
+                "A": None,
+            }
+        }])
+
+        info = get_review_info_by_layer_name(["Z", "A"])
+        self.assertEqual(info, [{
+            "name": "",
+            "review_channels": {
+                "R": "Z",
+                "G": "Z",
+                "B": "Z",
+                "A": "A",
+            }
+        }])
+
+    def test_ar_ag_ab_channels(self):
+
+        info = get_review_info_by_layer_name(["AR", "AG", "AB"])
+        self.assertEqual(info, [{
+            "name": "",
+            "review_channels": {
+                "R": "AR",
+                "G": "AG",
+                "B": "AB",
+                "A": None,
+            }
+        }])
+
+        info = get_review_info_by_layer_name(["AR", "AG", "AB", "A"])
+        self.assertEqual(info, [{
+            "name": "",
+            "review_channels": {
+                "R": "AR",
+                "G": "AG",
+                "B": "AB",
+                "A": "A",
+            }
+        }])
+
+    def test_unknown_channels(self):
+        info = get_review_info_by_layer_name(["hello", "world"])
+        self.assertEqual(info, [])
+
+    def test_rgba_priority(self):
+        """Ensure main layer, and RGB channels are prioritized
+
+        If both Z and RGB channels are present for a layer name, then RGB
+        should be prioritized and the Z channel should be ignored.
+
+        Also, the alpha channel from another "layer name" is not used. Note
+        how the diffuse response does not take A channel from the main layer.
+
+        """
+
+        info = get_review_info_by_layer_name([
+            "Z",
+            "diffuse.R", "diffuse.G", "diffuse.B",
+            "R", "G", "B", "A",
+            "specular.R", "specular.G", "specular.B", "specular.A",
+        ])
+        self.assertEqual(info, [
+            {
+                "name": "",
+                "review_channels": {
+                    "R": "R",
+                    "G": "G",
+                    "B": "B",
+                    "A": "A",
+                },
+            },
+            {
+                "name": "diffuse",
+                "review_channels": {
+                    "R": "diffuse.R",
+                    "G": "diffuse.G",
+                    "B": "diffuse.B",
+                    "A": None,
+                },
+            },
+            {
+                "name": "specular",
+                "review_channels": {
+                    "R": "specular.R",
+                    "G": "specular.G",
+                    "B": "specular.B",
+                    "A": "specular.A",
+                },
+            },
+        ])