diff --git a/packages/flamenco-worker-python/flamenco_worker/runner.py b/packages/flamenco-worker-python/flamenco_worker/runner.py index 5a656c64754a2156987ebf9477331086b79dae91..f43c9e116bfeb05a30031cb2d34f5410e7ffd738 100644 --- a/packages/flamenco-worker-python/flamenco_worker/runner.py +++ b/packages/flamenco-worker-python/flamenco_worker/runner.py @@ -6,6 +6,7 @@ import asyncio.subprocess import logging import re import typing +from pathlib import Path import attr @@ -454,7 +455,6 @@ class BlenderRenderCommand(AbstractSubprocessCommand): self.re_path_not_found = re.compile(r"Warning: Path '.*' not found") def validate(self, settings: dict): - from pathlib import Path import shlex blender_cmd, err = self._setting(settings, 'blender_cmd', True) @@ -491,6 +491,12 @@ class BlenderRenderCommand(AbstractSubprocessCommand): return None async def execute(self, settings: dict): + cmd = self._build_blender_cmd(settings) + + await self.worker.register_task_update(activity='Starting Blender') + await self.subprocess(cmd) + + def _build_blender_cmd(self, settings): cmd = settings['blender_cmd'][:] cmd += [ '--factory-startup', @@ -506,9 +512,7 @@ class BlenderRenderCommand(AbstractSubprocessCommand): cmd.extend(['--render-format', settings['format']]) if settings.get('frames'): cmd.extend(['--render-frame', settings['frames']]) - - await self.worker.register_task_update(activity='Starting Blender') - await self.subprocess(cmd) + return cmd def parse_render_line(self, line: str) -> typing.Optional[dict]: """Parses a single line of render progress. @@ -567,10 +571,35 @@ class BlenderRenderCommand(AbstractSubprocessCommand): return '> %s' % line -@command_executor('merge-exr') -class MergeExrCommand(AbstractSubprocessCommand): +@command_executor('blender_render_progressive') +class BlenderRenderProgressiveCommand(BlenderRenderCommand): + def validate(self, settings: dict): + err = super().validate(settings) + if err: return err + + cycles_num_chunks, err = self._setting(settings, 'cycles_num_chunks', True, int) + if err: return err + if cycles_num_chunks < 1: + return '"cycles_num_chunks" must be a positive integer' + + cycles_chunk, err = self._setting(settings, 'cycles_chunk', True, int) + if err: return err + if cycles_chunk < 1: + return '"cycles_chunk" must be a positive integer' + + def _build_blender_cmd(self, settings): + cmd = super()._build_blender_cmd(settings) + + return cmd + [ + '--', + '--cycles-resumable-num-chunks', str(settings['cycles_num_chunks']), + '--cycles-resumable-current-chunk', str(settings['cycles_chunk']), + ] + + +@command_executor('merge_progressive_renders') +class MergeProgressiveRendersCommand(AbstractSubprocessCommand): def validate(self, settings: dict): - from pathlib import Path import shlex blender_cmd, err = self._setting(settings, 'blender_cmd', True) @@ -606,8 +635,6 @@ class MergeExrCommand(AbstractSubprocessCommand): if err: return err async def execute(self, settings: dict): - from pathlib import Path - import shutil import tempfile blendpath = Path(__file__).with_name('merge-exr.blend') @@ -623,9 +650,9 @@ class MergeExrCommand(AbstractSubprocessCommand): # set up node properties and render settings. output = Path(settings['output']) - output.mkdir(parents=True, exist_ok=True) + output.parent.mkdir(parents=True, exist_ok=True) - with tempfile.TemporaryDirectory(dir=str(output)) as tmpdir: + with tempfile.TemporaryDirectory(dir=str(output.parent)) as tmpdir: tmppath = Path(tmpdir) assert tmppath.exists() @@ -638,6 +665,19 @@ class MergeExrCommand(AbstractSubprocessCommand): await self.subprocess(cmd) # move output files into the correct spot. - shutil.move(str(tmppath / 'merged0001.exr'), str(output)) - shutil.move(str(tmppath / 'preview.jpg'), str(output.with_suffix('.jpg'))) + await self.move(tmppath / 'merged0001.exr', output) + await self.move(tmppath / 'preview.jpg', output.with_suffix('.jpg')) + + async def move(self, src: Path, dst: Path): + """Moves a file to another location.""" + + import shutil + + self._log.info('Moving %s to %s', src, dst) + + assert src.is_file() + assert not dst.exists() or dst.is_file() + assert dst.exists() or dst.parent.exists() + await self.worker.register_log('Moving %s to %s', src, dst) + shutil.move(str(src), str(dst)) diff --git a/packages/flamenco-worker-python/tests/test_runner_blender_render.py b/packages/flamenco-worker-python/tests/test_runner_blender_render.py index 2b875678e82ad6efc1c166d6c3e92c41d3bd5199..9958a940e74e4ca90431608175169fe71a744d31 100644 --- a/packages/flamenco-worker-python/tests/test_runner_blender_render.py +++ b/packages/flamenco-worker-python/tests/test_runner_blender_render.py @@ -65,38 +65,29 @@ class BlenderRenderTest(AbstractCommandTest): self.loop.run_until_complete(self.cmd.process_line(line)) self.fworker.register_task_update.assert_called_once_with(activity=line) - @patch('pathlib.Path') - def test_cli_args(self, mock_path): + def test_cli_args(self): """Test that CLI arguments in the blender_cmd setting are handled properly.""" + from pathlib import Path import subprocess from mock_responses import CoroMock + filepath = str(Path(__file__).parent) settings = { - 'blender_cmd': '/path/to/blender --with --cli="args for CLI"', + # Point blender_cmd to this file so that we're sure it exists. + 'blender_cmd': '%s --with --cli="args for CLI"' % __file__, 'chunk_size': 100, 'frames': '1..2', 'format': 'JPEG', - 'filepath': '/path/to/output', + 'filepath': filepath, } - mock_path.Path.exists.side_effect = [True, True] - cse = CoroMock() cse.coro.return_value.wait = CoroMock(return_value=0) with patch('asyncio.create_subprocess_exec', new=cse) as mock_cse: self.loop.run_until_complete(self.cmd.run(settings)) - mock_path.assert_has_calls([ - call('/path/to/blender'), - call().exists(), - call().exists().__bool__(), - call('/path/to/output'), - call().exists(), - call().exists().__bool__(), - ]) - mock_cse.assert_called_once_with( - '/path/to/blender', + __file__, '--with', '--cli=args for CLI', '--factory-startup', @@ -104,7 +95,7 @@ class BlenderRenderTest(AbstractCommandTest): '--debug-cycles', '-noaudio', '--background', - '/path/to/output', + filepath, '--render-format', 'JPEG', '--render-frame', '1..2', stdin=subprocess.DEVNULL, diff --git a/packages/flamenco-worker-python/tests/test_runner_merge_exr.py b/packages/flamenco-worker-python/tests/test_runner_merge_exr.py index 294ba88742c15a236d36e65d7fcc0d3d4a333957..e09d61f4c36318467cc15fa9a671666416ef27f7 100644 --- a/packages/flamenco-worker-python/tests/test_runner_merge_exr.py +++ b/packages/flamenco-worker-python/tests/test_runner_merge_exr.py @@ -3,17 +3,17 @@ from pathlib import Path from test_runner import AbstractCommandTest -class MergeExrCommandTest(AbstractCommandTest): +class MergeProgressiveRendersCommandTest(AbstractCommandTest): def setUp(self): super().setUp() - from flamenco_worker.runner import MergeExrCommand + from flamenco_worker.runner import MergeProgressiveRendersCommand import tempfile self.tmpdir = tempfile.TemporaryDirectory() self.mypath = Path(__file__).parent - self.cmd = MergeExrCommand( + self.cmd = MergeProgressiveRendersCommand( worker=self.fworker, task_id='12345', command_idx=0, @@ -41,4 +41,6 @@ class MergeExrCommandTest(AbstractCommandTest): # Assuming that if the files exist, the merge was ok. self.assertTrue(output.exists()) + self.assertTrue(output.is_file()) self.assertTrue(output.with_suffix('.jpg').exists()) + self.assertTrue(output.with_suffix('.jpg').is_file())