From 885fc34e6b80f95920a09b9b6e81e548382efdad Mon Sep 17 00:00:00 2001 From: CalamitousFelicitousness Date: Sun, 16 Aug 2026 07:38:43 +0100 Subject: [PATCH] refactor(minimax): delegate tab reference marshalling to the core The tab held the only code that built video and audio references, sniffed the file type itself, and dropped anything it did not recognize: an unknown extension, a file that had gone missing, and any decode failure were all skipped without a word, leaving a request that generated from fewer references than were uploaded. Reference marshalling now goes through the same funnel the api path uses, and runs before the load, so a rejected file costs nothing and says which file and why. The workflow comes from the registry row, which is where the loader reads it from as well. - a rejected input returns its reason to the output box, since the general handler only reaches the log - references uploaded against a keyframe workflow warn instead of vanishing: the accordion hides on a row change but the files it held do not - guard p.close() in the finally, which the model-not-loaded return has always reached before p exists --- modules/minimax/minimax_ui.py | 7 +-- modules/minimax/minimax_video.py | 79 ++++++++++++++++---------------- test/test-video-references.py | 72 +++++++++++++++++++++++++++++ 3 files changed, 116 insertions(+), 42 deletions(-) diff --git a/modules/minimax/minimax_ui.py b/modules/minimax/minimax_ui.py index 455901c5b..2be8e60a7 100644 --- a/modules/minimax/minimax_ui.py +++ b/modules/minimax/minimax_ui.py @@ -4,7 +4,7 @@ from modules import ui_sections, ui_symbols from modules.ui_components import ToolButton from modules.logger import log from modules.video_models.models_def import models -from modules.minimax import minimax_video +from modules.minimax import minimax_video, minimax_references debug = log.trace if os.environ.get('SD_VIDEO_DEBUG', None) is not None else lambda *args, **kwargs: None @@ -41,8 +41,9 @@ def create_ui(prompt, _negative, styles, overrides, script_inputs, mp4_fps, mp4_ with gr.Row(): last_image = gr.Image(label='Last image', elem_id='minimax_last_image', type='pil', image_mode='RGB', width=256, height=256) with gr.Accordion(open=False, label="Reference media", elem_id='minimax_reference_accordion', visible=True) as reference_accordion: - gr.HTML("""Upload up to 9 images, 3 videos, and 3 audio files
- The total number of files must not exceed 12

""", elem_id='minimax_reference_media_info', elem_classes=['smaller']) + caps = minimax_references.get_reference_caps('ref2va') + gr.HTML(f"""Upload up to {caps.max_images} images, {caps.max_videos} videos, and {caps.max_audios} audio files
+ The total number of files must not exceed {caps.max_references}

""", elem_id='minimax_reference_media_info', elem_classes=['smaller']) reference_media = gr.Files(label="Reference media", interactive=True, elem_id="minimax_reference_media", visible=True) with gr.Column(elem_id='minimax-output-column', scale=2) as _column_output: diff --git a/modules/minimax/minimax_video.py b/modules/minimax/minimax_video.py index 60e6ffe46..0804f8128 100644 --- a/modules/minimax/minimax_video.py +++ b/modules/minimax/minimax_video.py @@ -1,4 +1,3 @@ -import os import time from PIL import Image import numpy as np @@ -37,40 +36,32 @@ def load_model(model: str): return None -def prepare_inputs(workflow: str, p: processing.StableDiffusionProcessingVideo, init_image: Image.Image | None, last_image: Image.Image | None, reference_media: list | None): - from diffusers.modular_pipelines.minimax_h3 import MiniMaxH3ImageReference, MiniMaxH3VideoReference, MiniMaxH3AudioReference - if workflow == 'fl2va': - if init_image is not None: - p.task_args['image'] = init_image - if last_image is not None: - p.task_args['last_image'] = last_image - log.debug(f'Prepare inputs: workflow={workflow} first={init_image} last={last_image}') - if workflow == 'ref2va': - if reference_media is None or len(reference_media) == 0: - return - files = [] - references = [] - for fn in reference_media: - try: - if hasattr(fn, 'name'): # gradio tempfile wrapper as files end up uploaded and not embedded - fn = fn.name - if not os.path.exists(fn): - log.warning(f'Prepare inputs: workflow={workflow} file="{fn}" not found') - continue - if fn.lower().endswith((".png", ".jpg", ".jpeg", ".webp")): - files.append(fn) - references.append(MiniMaxH3ImageReference.from_file(fn)) - elif fn.lower().endswith((".mp4", ".mov", ".avi")): - files.append(fn) - references.append(MiniMaxH3VideoReference.from_file(fn)) - elif fn.lower().endswith((".wav", ".mp3", ".flac", ".aac")): - files.append(fn) - references.append(MiniMaxH3AudioReference.from_file(fn)) - except Exception as e: - log.error(f'Prepare inputs: workflow={workflow} file="{fn}" {e}') - if len(references) > 0: - p.task_args['references'] = references - log.debug(f'Prepare inputs: workflow={workflow} files={files}') +def unwrap_file(entry): + """The path behind a gradio file entry: an upload arrives as a tempfile wrapper or a dict, not a path.""" + if hasattr(entry, 'name'): + return entry.name + if isinstance(entry, dict) and 'name' in entry: + return entry['name'] + return entry + + +def prepare_inputs(workflow: str | None, init_image: Image.Image | None, last_image: Image.Image | None, reference_media: list | None) -> dict: + """The task args a workflow conditions on, resolved before the model load so a rejected request costs nothing.""" + from modules.minimax import minimax_references + if minimax_references.get_reference_caps(workflow) is not None: + entries = [unwrap_file(entry) for entry in (reference_media or [])] + references = minimax_references.resolve(workflow, entries, init_image) + log.debug(f'Prepare inputs: workflow={workflow} references={len(references)}') + return {'references': references} + task_args = {} + if init_image is not None: + task_args['image'] = init_image + if last_image is not None: + task_args['last_image'] = last_image + if reference_media: + log.warning(f'Video: op=reference workflow={workflow} references not supported, ignoring: count={len(reference_media)}') + log.debug(f'Prepare inputs: workflow={workflow} first={init_image} last={last_image}') + return task_args def generate(task_id, _ui_state, @@ -90,7 +81,7 @@ def generate(task_id, _ui_state, **_kwargs, ): video_utils.check_av() - from modules.video_models import video_minimax + from modules.video_models import video_minimax, video_run progress.add_task_to_queue(task_id) with call_queue.get_lock(): @@ -100,12 +91,18 @@ def generate(task_id, _ui_state, timer.process.reset() # init vars + p = None + workflow = None # the incoming argument is the ui's display label, so the row and then the load supply the real one pixels = None num_frames = 0 video_file = None aac_sample_rate = 32000 try: + # resolved off the registry row so a bad reference is rejected before the load, the same as on the api path + selected = models_def.find(engine, model) + workflow = getattr(selected, 'workflow', None) + task_args = prepare_inputs(workflow, init_image, last_image, reference_media) workflow = load_model(model) # override workflow based on loaded model if not workflow: progress.finish_task(task_id) @@ -135,7 +132,7 @@ def generate(task_id, _ui_state, p.scripts = scripts_manager.scripts_video p.script_args = args - prepare_inputs(workflow, p, init_image, last_image, reference_media) + p.task_args.update(task_args) _processed: processing.Processed = scripts_manager.scripts_video.run(p, *args) processed = processing.process_images(p) @@ -191,14 +188,18 @@ def generate(task_id, _ui_state, if audio is not None: del audio + except video_run.VideoError as e: # a rejected input, so the reason belongs in the output box and not only in the log + log.error(f'Video: engine="{engine}" model="{model}" workflow={workflow} {e}') + return None, f'Error: {e}' except Exception as e: log.error(f'Video: engine="{engine}" model="{model}" workflow={workflow} {e}') errors.display(e, 'Video') finally: - jobid = getattr(shared.sd_model, 'sdnext_phaseid', None) # previous jobid if any + jobid = getattr(shared.sd_model, 'sdnext_phaseid', None) if shared.sd_loaded else None # sd_model loads on access, and a request rejected before the load must not trigger one shared.state.end(jobid) # clear the previous job if exists progress.finish_task(task_id) - p.close() + if p is not None: # a request rejected before the processing object exists has nothing to close + p.close() t1 = time.time() resolution = f'{w}x{h}' if num_frames > 0 else None diff --git a/test/test-video-references.py b/test/test-video-references.py index 2290e7b3e..edd1d4cd8 100644 --- a/test/test-video-references.py +++ b/test/test-video-references.py @@ -145,6 +145,12 @@ def stub_video_reference(frames: int, fps: float): return types.SimpleNamespace(frames=[None] * frames, fps=fps, kind='video') +def make_png(name: str, width: int = 64, height: int = 64) -> str: + fn = os.path.join(tmpdir, name) + image(width, height).save(fn) + return fn + + def make_wav(name: str, seconds: float = 1.0, rate: int = 32000, channels: int = 1) -> str: fn = os.path.join(tmpdir, name) with wave.open(fn, 'wb') as handle: @@ -545,6 +551,13 @@ def test_built_image_is_rgb(): assert built[0].image.mode == 'RGB', f'mode={built[0].image.mode}' +def test_corrupt_file_is_rejected_cleanly(): + if not has_diffusers(): + return 'diffusers not installed' + # an unreadable file has to surface as a rejection naming it, not as a traceback out of the decoder + return expect_error(lambda: refs.resolve('ref2va', [touch('corrupt.png')]), 'decode failed') + + def test_short_video_is_rejected_after_the_decode(): if not has_av(): return 'av not installed' @@ -597,6 +610,53 @@ def test_core_resolves_a_reference_model(): return True +# ============================================================ +# Tab delegation +# ============================================================ + +def test_tab_unwraps_gradio_file_entries(): + from modules.minimax import minimax_video + fn = touch('unwrap.png') + assert minimax_video.unwrap_file(fn) == fn, 'a plain path was rewritten' + assert minimax_video.unwrap_file(types.SimpleNamespace(name=fn)) == fn, 'a tempfile wrapper was not unwrapped' + assert minimax_video.unwrap_file({'name': fn}) == fn, 'a dict entry was not unwrapped' + + +def test_tab_keeps_keyframes_on_a_keyframe_workflow(): + from modules.minimax import minimax_video + task_args = minimax_video.prepare_inputs('fl2va', image(), image(), None) + assert sorted(task_args) == ['image', 'last_image'], f'{sorted(task_args)}' + + +def test_tab_reports_references_it_cannot_use(): + # uploads survive the accordion hiding when the row changes, and dropping them silently reads as a working request + from modules.minimax import minimax_video + seen = [] + original = minimax_video.log.warning + minimax_video.log.warning = lambda msg, *a, **k: seen.append(str(msg)) + try: + task_args = minimax_video.prepare_inputs('fl2va', image(), None, [touch('stale.png')]) + finally: + minimax_video.log.warning = original + assert any('not supported' in message for message in seen), f'{seen}' + assert 'references' not in task_args, 'a keyframe workflow claimed the references' + + +def test_tab_rejections_name_the_reason(): + from modules.minimax import minimax_video + return expect_error(lambda: minimax_video.prepare_inputs('ref2va', None, None, [touch('notes.txt')]), 'unsupported media type') + + +def test_tab_resolves_references_through_the_shared_funnel(): + from modules.minimax import minimax_video + if not has_diffusers(): + return 'diffusers not installed' + task_args = minimax_video.prepare_inputs('ref2va', image(), None, [types.SimpleNamespace(name=make_png('tab.png'))]) + assert list(task_args) == ['references'], f'{list(task_args)}' + assert [reference.kind for reference in task_args['references']] == ['image'], f'{task_args}' + return True + + # ============================================================ # Runner # ============================================================ @@ -692,6 +752,7 @@ def run_all(): test_built_references_keep_the_request_order, test_built_video_carries_its_frame_rate, test_built_image_is_rgb, + test_corrupt_file_is_rejected_cleanly, test_short_video_is_rejected_after_the_decode, ]: run_test(cat, fn) @@ -707,6 +768,17 @@ def run_all(): ]: run_test(cat, fn) + log.warning('=== tab delegation ===') + cat = category('tab') + for fn in [ + test_tab_unwraps_gradio_file_entries, + test_tab_keeps_keyframes_on_a_keyframe_workflow, + test_tab_reports_references_it_cannot_use, + test_tab_rejections_name_the_reason, + test_tab_resolves_references_through_the_shared_funnel, + ]: + run_test(cat, fn) + log.warning('=== Results ===') total_passed = 0 total_failed = 0