From 7905d18e6713b1307c2ea33fc3f4cb07577999b0 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Mon, 25 Jul 2022 12:40:03 +0200 Subject: [PATCH 1/8] check if instance have representations as first thing --- openpype/plugins/publish/extract_thumbnail.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/openpype/plugins/publish/extract_thumbnail.py b/openpype/plugins/publish/extract_thumbnail.py index 7933595b89..789b6c75bc 100644 --- a/openpype/plugins/publish/extract_thumbnail.py +++ b/openpype/plugins/publish/extract_thumbnail.py @@ -29,7 +29,17 @@ class ExtractThumbnail(pyblish.api.InstancePlugin): ffmpeg_args = None def process(self, instance): - self.log.info("subset {}".format(instance.data['subset'])) + subset_name = instance.data["subset"] + instance_repres = instance.data.get("representations") + if not instance_repres: + self.log.debug(( + "Instance {} does not have representations. Skipping" + ).format(subset_name)) + return + + self.log.info( + "Processing instance with subset name {}".format(subset_name) + ) # skip crypto passes. # TODO: This is just a quick fix and has its own side-effects - it is From 89705a69d1df6ba69792290d56220e2f2bd317f7 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Mon, 25 Jul 2022 12:43:10 +0200 Subject: [PATCH 2/8] move instance review key check earlier and move the logic to method --- openpype/plugins/publish/extract_thumbnail.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/openpype/plugins/publish/extract_thumbnail.py b/openpype/plugins/publish/extract_thumbnail.py index 789b6c75bc..839618bcdd 100644 --- a/openpype/plugins/publish/extract_thumbnail.py +++ b/openpype/plugins/publish/extract_thumbnail.py @@ -41,6 +41,11 @@ class ExtractThumbnail(pyblish.api.InstancePlugin): "Processing instance with subset name {}".format(subset_name) ) + # Skip if instance does not have review + if not self._is_review_instance(instance): + self.log.info("Skipping - no review set on instance.") + return + # skip crypto passes. # TODO: This is just a quick fix and has its own side-effects - it is # affecting every subset name with `crypto` in its name. @@ -51,11 +56,6 @@ class ExtractThumbnail(pyblish.api.InstancePlugin): self.log.info("Skipping crypto passes.") return - # Skip if review not set. - if not instance.data.get("review", True): - self.log.info("Skipping - no review set on instance.") - return - if self._already_has_thumbnail(instance): self.log.info("Thumbnail representation already present.") return @@ -116,6 +116,13 @@ class ExtractThumbnail(pyblish.api.InstancePlugin): # There is no need to create more then one thumbnail break + def _is_review_instance(self, instance): + # TODO: We should probably handle "not creating" of thumbnail + # other way then checking for "review" key on instance data? + if instance.data.get("review", True): + return True + return False + def _already_has_thumbnail(self, instance): for repre in instance.data.get("representations", []): self.log.info("repre {}".format(repre)) From f82c97dc6aae62007de239c30fe8304b561a2a3b Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Mon, 25 Jul 2022 12:45:44 +0200 Subject: [PATCH 3/8] check existing thumbnail before crypto check --- openpype/plugins/publish/extract_thumbnail.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/openpype/plugins/publish/extract_thumbnail.py b/openpype/plugins/publish/extract_thumbnail.py index 839618bcdd..51624a3cc7 100644 --- a/openpype/plugins/publish/extract_thumbnail.py +++ b/openpype/plugins/publish/extract_thumbnail.py @@ -41,11 +41,16 @@ class ExtractThumbnail(pyblish.api.InstancePlugin): "Processing instance with subset name {}".format(subset_name) ) - # Skip if instance does not have review + # Skip if instance have 'review' key in data set to 'False' if not self._is_review_instance(instance): self.log.info("Skipping - no review set on instance.") return + # Check if already has thumbnail created + if self._already_has_thumbnail(instance_repres): + self.log.info("Thumbnail representation already present.") + return + # skip crypto passes. # TODO: This is just a quick fix and has its own side-effects - it is # affecting every subset name with `crypto` in its name. @@ -56,9 +61,6 @@ class ExtractThumbnail(pyblish.api.InstancePlugin): self.log.info("Skipping crypto passes.") return - if self._already_has_thumbnail(instance): - self.log.info("Thumbnail representation already present.") - return filtered_repres = self._get_filtered_repres(instance) for repre in filtered_repres: @@ -123,12 +125,11 @@ class ExtractThumbnail(pyblish.api.InstancePlugin): return True return False - def _already_has_thumbnail(self, instance): - for repre in instance.data.get("representations", []): + def _already_has_thumbnail(self, repres): + for repre in repres: self.log.info("repre {}".format(repre)) if repre["name"] == "thumbnail": return True - return False def _get_filtered_repres(self, instance): From 239414ffff9d9ec0e03c6b5239e7206317d5b9fa Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Mon, 25 Jul 2022 12:49:52 +0200 Subject: [PATCH 4/8] try to create thumbnail from all filtered representations --- openpype/plugins/publish/extract_thumbnail.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/openpype/plugins/publish/extract_thumbnail.py b/openpype/plugins/publish/extract_thumbnail.py index 51624a3cc7..cb1af12586 100644 --- a/openpype/plugins/publish/extract_thumbnail.py +++ b/openpype/plugins/publish/extract_thumbnail.py @@ -63,6 +63,14 @@ class ExtractThumbnail(pyblish.api.InstancePlugin): filtered_repres = self._get_filtered_repres(instance) + if not filtered_repres: + self.log.info(( + "Instance don't have representations" + " that can be used as source for thumbnail. Skipping" + )) + return + + thumbnail_created = False for repre in filtered_repres: repre_files = repre["files"] if not isinstance(repre_files, (list, tuple)): @@ -81,7 +89,6 @@ class ExtractThumbnail(pyblish.api.InstancePlugin): jpeg_file = filename + "jpg" full_output_path = os.path.join(stagingdir, jpeg_file) - thumbnail_created = False # Try to use FFMPEG if OIIO is not supported (for cases when # oiiotool isn't available) if not is_oiio_supported(): @@ -96,10 +103,9 @@ class ExtractThumbnail(pyblish.api.InstancePlugin): self.log.info("Converting with FFMPEG because input can't be read by OIIO.") # noqa thumbnail_created = self.create_thumbnail_ffmpeg(full_input_path, full_output_path) # noqa - # Skip the rest of the process if the thumbnail wasn't created + # Skip representation and try next one if wasn't created if not thumbnail_created: - self.log.warning("Thumbanil has not been created.") - return + continue new_repre = { "name": "thumbnail", @@ -118,6 +124,9 @@ class ExtractThumbnail(pyblish.api.InstancePlugin): # There is no need to create more then one thumbnail break + if not thumbnail_created: + self.log.warning("Thumbanil has not been created.") + def _is_review_instance(self, instance): # TODO: We should probably handle "not creating" of thumbnail # other way then checking for "review" key on instance data? From 6a018364b2961d21dab66657b12613838ba54750 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Mon, 25 Jul 2022 12:53:27 +0200 Subject: [PATCH 5/8] create custom staging dir for thumbnail representation --- openpype/plugins/publish/extract_thumbnail.py | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/openpype/plugins/publish/extract_thumbnail.py b/openpype/plugins/publish/extract_thumbnail.py index cb1af12586..2715aa4db4 100644 --- a/openpype/plugins/publish/extract_thumbnail.py +++ b/openpype/plugins/publish/extract_thumbnail.py @@ -1,4 +1,5 @@ import os +import tempfile import pyblish.api from openpype.lib import ( @@ -8,8 +9,6 @@ from openpype.lib import ( run_subprocess, path_to_subprocess_arg, - - execute, ) @@ -57,11 +56,10 @@ class ExtractThumbnail(pyblish.api.InstancePlugin): # This must be solved properly, maybe using tags on # representation that can be determined much earlier and # with better precision. - if 'crypto' in instance.data['subset'].lower(): + if "crypto" in subset_name.lower(): self.log.info("Skipping crypto passes.") return - filtered_repres = self._get_filtered_repres(instance) if not filtered_repres: self.log.info(( @@ -70,6 +68,15 @@ class ExtractThumbnail(pyblish.api.InstancePlugin): )) return + # Create temp directory for thumbnail + # - this is to avoid "override" of source file + dst_staging = tempfile.mkdtemp(prefix="pyblish_tmp_") + self.log.debug( + "Create temp directory {} for thumbnail".formap(dst_staging) + ) + # Store new staging to cleanup paths + instance.context.data["cleanupFullPaths"].append(dst_staging) + thumbnail_created = False for repre in filtered_repres: repre_files = repre["files"] @@ -79,15 +86,12 @@ class ExtractThumbnail(pyblish.api.InstancePlugin): file_index = int(float(len(repre_files)) * 0.5) input_file = repre_files[file_index] - stagingdir = os.path.normpath(repre["stagingDir"]) - - full_input_path = os.path.join(stagingdir, input_file) + src_staging = os.path.normpath(repre["stagingDir"]) + full_input_path = os.path.join(src_staging, input_file) self.log.info("input {}".format(full_input_path)) filename = os.path.splitext(input_file)[0] - if not filename.endswith('.'): - filename += "." - jpeg_file = filename + "jpg" - full_output_path = os.path.join(stagingdir, jpeg_file) + jpeg_file = filename + ".jpg" + full_output_path = os.path.join(dst_staging, jpeg_file) # Try to use FFMPEG if OIIO is not supported (for cases when # oiiotool isn't available) @@ -111,7 +115,7 @@ class ExtractThumbnail(pyblish.api.InstancePlugin): "name": "thumbnail", "ext": "jpg", "files": jpeg_file, - "stagingDir": stagingdir, + "stagingDir": dst_staging, "thumbnail": True, "tags": ["thumbnail"] } From d1987eed02ba4ca842cd820ef0947adbb4240d2b Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Mon, 25 Jul 2022 12:53:58 +0200 Subject: [PATCH 6/8] removed unneeded f string --- openpype/plugins/publish/extract_thumbnail.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/openpype/plugins/publish/extract_thumbnail.py b/openpype/plugins/publish/extract_thumbnail.py index 2715aa4db4..d944c341e5 100644 --- a/openpype/plugins/publish/extract_thumbnail.py +++ b/openpype/plugins/publish/extract_thumbnail.py @@ -167,12 +167,12 @@ class ExtractThumbnail(pyblish.api.InstancePlugin): def create_thumbnail_oiio(self, src_path, dst_path): self.log.info("outputting {}".format(dst_path)) oiio_tool_path = get_oiio_tools_path() - oiio_cmd = [oiio_tool_path, "-a", - src_path, "-o", - dst_path - ] - subprocess_exr = " ".join(oiio_cmd) - self.log.info(f"running: {subprocess_exr}") + oiio_cmd = [ + oiio_tool_path, + "-a", src_path, + "-o", dst_path + ] + self.log.info("running: {}".format(" ".join(oiio_cmd))) try: run_subprocess(oiio_cmd, logger=self.log) return True From adc9b9303ba090dd1125f83b5d619306d230c5b9 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Mon, 25 Jul 2022 12:55:29 +0200 Subject: [PATCH 7/8] reduced order of thumbnail creation to 2 conditions --- openpype/plugins/publish/extract_thumbnail.py | 28 ++++++++++++------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/openpype/plugins/publish/extract_thumbnail.py b/openpype/plugins/publish/extract_thumbnail.py index d944c341e5..c1eee71376 100644 --- a/openpype/plugins/publish/extract_thumbnail.py +++ b/openpype/plugins/publish/extract_thumbnail.py @@ -78,6 +78,7 @@ class ExtractThumbnail(pyblish.api.InstancePlugin): instance.context.data["cleanupFullPaths"].append(dst_staging) thumbnail_created = False + oiio_supported = is_oiio_supported() for repre in filtered_repres: repre_files = repre["files"] if not isinstance(repre_files, (list, tuple)): @@ -93,19 +94,26 @@ class ExtractThumbnail(pyblish.api.InstancePlugin): jpeg_file = filename + ".jpg" full_output_path = os.path.join(dst_staging, jpeg_file) - # Try to use FFMPEG if OIIO is not supported (for cases when - # oiiotool isn't available) - if not is_oiio_supported(): - thumbnail_created = self.create_thumbnail_ffmpeg(full_input_path, full_output_path) # noqa - else: + if oiio_supported: + self.log.info("Trying to convert with OIIO") # If the input can read by OIIO then use OIIO method for # conversion otherwise use ffmpeg - self.log.info("Trying to convert with OIIO") # noqa - thumbnail_created = self.create_thumbnail_oiio(full_input_path, full_output_path) # noqa + thumbnail_created = self.create_thumbnail_oiio( + full_input_path, full_output_path + ) - if not thumbnail_created: - self.log.info("Converting with FFMPEG because input can't be read by OIIO.") # noqa - thumbnail_created = self.create_thumbnail_ffmpeg(full_input_path, full_output_path) # noqa + # Try to use FFMPEG if OIIO is not supported or for cases when + # oiiotool isn't available + if not thumbnail_created: + if oiio_supported: + self.log.info(( + "Converting with FFMPEG because input" + " can't be read by OIIO." + )) + + thumbnail_created = self.create_thumbnail_ffmpeg( + full_input_path, full_output_path + ) # Skip representation and try next one if wasn't created if not thumbnail_created: From 642d6ef407630ef2a9dad37551b6725569a7b4d7 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Tue, 26 Jul 2022 11:15:48 +0200 Subject: [PATCH 8/8] fix typo --- openpype/plugins/publish/extract_thumbnail.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openpype/plugins/publish/extract_thumbnail.py b/openpype/plugins/publish/extract_thumbnail.py index c1eee71376..c154275322 100644 --- a/openpype/plugins/publish/extract_thumbnail.py +++ b/openpype/plugins/publish/extract_thumbnail.py @@ -72,7 +72,7 @@ class ExtractThumbnail(pyblish.api.InstancePlugin): # - this is to avoid "override" of source file dst_staging = tempfile.mkdtemp(prefix="pyblish_tmp_") self.log.debug( - "Create temp directory {} for thumbnail".formap(dst_staging) + "Create temp directory {} for thumbnail".format(dst_staging) ) # Store new staging to cleanup paths instance.context.data["cleanupFullPaths"].append(dst_staging)