Merge pull request #3076 from pypeclub/bugfix/OP-3107_ExtractReview-Sequence-is-not-converted-with-same-names

General: Extract review sequence is not converted with same names

openpype/hosts/webpublisher/plugins/publish/extract_thumbnail.py

@@ -8,7 +8,7 @@ from openpype.lib import (
     run_subprocess,
 
     get_transcode_temp_directory,
-    convert_for_ffmpeg,
+    convert_input_paths_for_ffmpeg,
     should_convert_for_ffmpeg
 )
 
@@ -59,11 +59,9 @@ class ExtractThumbnail(pyblish.api.InstancePlugin):
             if do_convert:
                 convert_dir = get_transcode_temp_directory()
                 filename = os.path.basename(full_input_path)
-                convert_for_ffmpeg(
-                    full_input_path,
+                convert_input_paths_for_ffmpeg(
+                    [full_input_path],
                     convert_dir,
-                    None,
-                    None,
                     self.log
                 )
                 full_input_path = os.path.join(convert_dir, filename)

openpype/lib/__init__.py

@@ -105,6 +105,7 @@ from .transcoding import (
     get_transcode_temp_directory,
     should_convert_for_ffmpeg,
     convert_for_ffmpeg,
+    convert_input_paths_for_ffmpeg,
     get_ffprobe_data,
     get_ffprobe_streams,
     get_ffmpeg_codec_args,
@@ -276,6 +277,7 @@ __all__ = [
     "get_transcode_temp_directory",
     "should_convert_for_ffmpeg",
     "convert_for_ffmpeg",
+    "convert_input_paths_for_ffmpeg",
     "get_ffprobe_data",
     "get_ffprobe_streams",
     "get_ffmpeg_codec_args",

openpype/lib/transcoding.py

@@ -382,6 +382,11 @@ def should_convert_for_ffmpeg(src_filepath):
     return False
 
 
+# Deprecated since 2022 4 20
+# - Reason - Doesn't convert sequences right way: Can't handle gaps, reuse
+#       first frame for all frames and changes filenames when input
+#       is sequence.
+# - use 'convert_input_paths_for_ffmpeg' instead
 def convert_for_ffmpeg(
     first_input_path,
     output_dir,
@@ -409,6 +414,12 @@ def convert_for_ffmpeg(
     if logger is None:
         logger = logging.getLogger(__name__)
 
+    logger.warning((
+        "DEPRECATED: 'openpype.lib.transcoding.convert_for_ffmpeg' is"
+        " deprecated function of conversion for FFMpeg. Please replace usage"
+        " with 'openpype.lib.transcoding.convert_input_paths_for_ffmpeg'"
+    ))
+
     ext = os.path.splitext(first_input_path)[1].lower()
     if ext != ".exr":
         raise ValueError((
@@ -516,6 +527,130 @@ def convert_for_ffmpeg(
     run_subprocess(oiio_cmd, logger=logger)
 
 
+def convert_input_paths_for_ffmpeg(
+    input_paths,
+    output_dir,
+    logger=None
+):
+    """Contert source file to format supported in ffmpeg.
+
+    Currently can convert only exrs. The input filepaths should be files
+    with same type. Information about input is loaded only from first found
+    file.
+
+    Filenames of input files are kept so make sure that output directory
+    is not the same directory as input files have.
+    - This way it can handle gaps and can keep input filenames without handling
+        frame template
+
+    Args:
+        input_paths (str): Paths that should be converted. It is expected that
+            contains single file or image sequence of samy type.
+        output_dir (str): Path to directory where output will be rendered.
+            Must not be same as input's directory.
+        logger (logging.Logger): Logger used for logging.
+
+    Raises:
+        ValueError: If input filepath has extension not supported by function.
+            Currently is supported only ".exr" extension.
+    """
+    if logger is None:
+        logger = logging.getLogger(__name__)
+
+    first_input_path = input_paths[0]
+    ext = os.path.splitext(first_input_path)[1].lower()
+    if ext != ".exr":
+        raise ValueError((
+            "Function 'convert_for_ffmpeg' currently support only"
+            " \".exr\" extension. Got \"{}\"."
+        ).format(ext))
+
+    input_info = get_oiio_info_for_input(first_input_path)
+
+    # Change compression only if source compression is "dwaa" or "dwab"
+    #   - they're not supported in ffmpeg
+    compression = input_info["attribs"].get("compression")
+    if compression in ("dwaa", "dwab"):
+        compression = "none"
+
+    # Collect channels to export
+    channel_names = input_info["channelnames"]
+    review_channels = get_convert_rgb_channels(channel_names)
+    if review_channels is None:
+        raise ValueError(
+            "Couldn't find channels that can be used for conversion."
+        )
+
+    red, green, blue, alpha = review_channels
+    input_channels = [red, green, blue]
+    channels_arg = "R={},G={},B={}".format(red, green, blue)
+    if alpha is not None:
+        channels_arg += ",A={}".format(alpha)
+        input_channels.append(alpha)
+    input_channels_str = ",".join(input_channels)
+
+    for input_path in input_paths:
+        # Prepare subprocess arguments
+        oiio_cmd = [
+            get_oiio_tools_path(),
+
+            # Don't add any additional attributes
+            "--nosoftwareattrib",
+        ]
+        # Add input compression if available
+        if compression:
+            oiio_cmd.extend(["--compression", compression])
+
+        oiio_cmd.extend([
+            # Tell oiiotool which channels should be loaded
+            # - other channels are not loaded to memory so helps to
+            #       avoid memory leak issues
+            "-i:ch={}".format(input_channels_str), input_path,
+            # Tell oiiotool which channels should be put to top stack
+            #   (and output)
+            "--ch", channels_arg
+        ])
+
+        for attr_name, attr_value in input_info["attribs"].items():
+            if not isinstance(attr_value, str):
+                continue
+
+            # Remove attributes that have string value longer than allowed
+            #   length for ffmpeg or when containt unallowed symbols
+            erase_reason = "Missing reason"
+            erase_attribute = False
+            if len(attr_value) > MAX_FFMPEG_STRING_LEN:
+                erase_reason = "has too long value ({} chars).".format(
+                    len(attr_value)
+                )
+
+            if erase_attribute:
+                for char in NOT_ALLOWED_FFMPEG_CHARS:
+                    if char in attr_value:
+                        erase_attribute = True
+                        erase_reason = (
+                            "contains unsupported character \"{}\"."
+                        ).format(char)
+                        break
+
+            if erase_attribute:
+                # Set attribute to empty string
+                logger.info((
+                    "Removed attribute \"{}\" from metadata because {}."
+                ).format(attr_name, erase_reason))
+                oiio_cmd.extend(["--eraseattrib", attr_name])
+
+        # Add last argument - path to output
+        base_filename = os.path.basename(input_path)
+        output_path = os.path.join(output_dir, base_filename)
+        oiio_cmd.extend([
+            "-o", output_path
+        ])
+
+        logger.debug("Conversion command: {}".format(" ".join(oiio_cmd)))
+        run_subprocess(oiio_cmd, logger=logger)
+
+
 # FFMPEG functions
 def get_ffprobe_data(path_to_file, logger=None):
     """Load data about entered filepath via ffprobe.

openpype/plugins/publish/extract_burnin.py

@@ -16,7 +16,7 @@ from openpype.lib import (
     run_openpype_process,
 
     get_transcode_temp_directory,
-    convert_for_ffmpeg,
+    convert_input_paths_for_ffmpeg,
     should_convert_for_ffmpeg,
 
     CREATE_NO_WINDOW
@@ -187,8 +187,13 @@ class ExtractBurnin(openpype.api.Extractor):
             repre_files = repre["files"]
             if isinstance(repre_files, (tuple, list)):
                 filename = repre_files[0]
+                src_filepaths = [
+                    os.path.join(src_repre_staging_dir, filename)
+                    for filename in repre_files
+                ]
             else:
                 filename = repre_files
+                src_filepaths = [os.path.join(src_repre_staging_dir, filename)]
 
             first_input_path = os.path.join(src_repre_staging_dir, filename)
             # Determine if representation requires pre conversion for ffmpeg
@@ -209,11 +214,9 @@ class ExtractBurnin(openpype.api.Extractor):
                 new_staging_dir = get_transcode_temp_directory()
                 repre["stagingDir"] = new_staging_dir
 
-                convert_for_ffmpeg(
-                    first_input_path,
+                convert_input_paths_for_ffmpeg(
+                    src_filepaths,
                     new_staging_dir,
-                    _temp_data["frameStart"],
-                    _temp_data["frameEnd"],
                     self.log
                 )
 

openpype/plugins/publish/extract_jpeg_exr.py

@@ -8,7 +8,7 @@ from openpype.lib import (
     path_to_subprocess_arg,
 
     get_transcode_temp_directory,
-    convert_for_ffmpeg,
+    convert_input_paths_for_ffmpeg,
     should_convert_for_ffmpeg
 )
 
@@ -79,11 +79,9 @@ class ExtractJpegEXR(pyblish.api.InstancePlugin):
             if do_convert:
                 convert_dir = get_transcode_temp_directory()
                 filename = os.path.basename(full_input_path)
-                convert_for_ffmpeg(
-                    full_input_path,
+                convert_input_paths_for_ffmpeg(
+                    [full_input_path],
                     convert_dir,
-                    None,
-                    None,
                     self.log
                 )
                 full_input_path = os.path.join(convert_dir, filename)

openpype/plugins/publish/extract_review.py

@@ -18,7 +18,7 @@ from openpype.lib import (
     path_to_subprocess_arg,
 
     should_convert_for_ffmpeg,
-    convert_for_ffmpeg,
+    convert_input_paths_for_ffmpeg,
     get_transcode_temp_directory
 )
 import speedcopy
@@ -194,16 +194,20 @@ class ExtractReview(pyblish.api.InstancePlugin):
             src_repre_staging_dir = repre["stagingDir"]
             # Receive filepath to first file in representation
             first_input_path = None
+            input_filepaths = []
             if not self.input_is_sequence(repre):
                 first_input_path = os.path.join(
                     src_repre_staging_dir, repre["files"]
                 )
+                input_filepaths.append(first_input_path)
             else:
                 for filename in repre["files"]:
-                    first_input_path = os.path.join(
+                    filepath = os.path.join(
                         src_repre_staging_dir, filename
                     )
-                    break
+                    input_filepaths.append(filepath)
+                    if first_input_path is None:
+                        first_input_path = filepath
 
             # Skip if file is not set
             if first_input_path is None:
@@ -230,13 +234,9 @@ class ExtractReview(pyblish.api.InstancePlugin):
                 new_staging_dir = get_transcode_temp_directory()
                 repre["stagingDir"] = new_staging_dir
 
-                frame_start = instance.data["frameStart"]
-                frame_end = instance.data["frameEnd"]
-                convert_for_ffmpeg(
-                    first_input_path,
+                convert_input_paths_for_ffmpeg(
+                    input_filepaths,
                     new_staging_dir,
-                    frame_start,
-                    frame_end,
                     self.log
                 )