From 4c62186c9581533f37ec3723e22c5acc4d367f73 Mon Sep 17 00:00:00 2001 From: martimramos Date: Fri, 17 Apr 2026 00:32:03 +0100 Subject: [PATCH] =?UTF-8?q?comfyui:=20drop=20music=20tool=20=E2=80=94=20AC?= =?UTF-8?q?E-Step=20node=20interface=20not=20standardized?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removed comfyui_music and its workflow. The ACE-Step model runs in ComfyUI but the node class names differ across custom node packs (AceStepModelLoader vs native TextEncodeAceStepAudio, etc.), so a bundled workflow would break for most users. Documented the reasoning in the plan doc and listed it as an open question for future work. Users with ACE-Step working can still use the workflow_json override on any tool. Co-Authored-By: Claude Opus 4.6 (1M context) --- .gitignore | 3 +- docs/comfyui-adapter-plan.md | 72 +++---- tests/contracts/test_comfyui_tools.py | 8 +- tools/_comfyui/workflows/ace-step-music.json | 27 --- tools/audio/comfyui_music.py | 211 ------------------- 5 files changed, 31 insertions(+), 290 deletions(-) delete mode 100644 tools/_comfyui/workflows/ace-step-music.json delete mode 100644 tools/audio/comfyui_music.py diff --git a/.gitignore b/.gitignore index ddff2fe..7a7c27a 100644 --- a/.gitignore +++ b/.gitignore @@ -92,4 +92,5 @@ remotion-composer/public/demo-props/test-* remotion-composer/public/demo-props/talking-head-* remotion-composer/public/demo-props/caption-burn-* -venv/ \ No newline at end of file +venv/ +.venv/ diff --git a/docs/comfyui-adapter-plan.md b/docs/comfyui-adapter-plan.md index 126a930..4feae00 100644 --- a/docs/comfyui-adapter-plan.md +++ b/docs/comfyui-adapter-plan.md @@ -229,33 +229,20 @@ workflow_json: string # optional override --- -### `comfyui_music` -- Music Generation +### `comfyui_music` -- Music Generation (not shipped) -| Field | Value | -|-------|-------| -| capability | `music_generation` | -| provider | `comfyui` | -| runtime | `LOCAL_GPU` | -| tier | `GENERATE` | -| stability | `EXPERIMENTAL` | -| capabilities | `text_to_music` | -| dependencies | (runtime: ComfyUI server reachable + ACE-Step model) | -| fallback_tools | `suno_music`, `elevenlabs_music` | -| cost | `$0.00` (local compute) | +We explored adding a `comfyui_music` tool using the ACE-Step 3.5B model. +The model runs well in ComfyUI, but the ComfyUI node interface for +ACE-Step is not standardized -- there are multiple custom node packs with +different class names (`AceStepModelLoader` vs native `TextEncodeAceStepAudio`, +etc.). Shipping a workflow that only works with one specific custom node +pack would break for most users. -**Bundled workflow:** `ace-step-music.json` - -Uses ACE-Step v1 3.5B for text-to-music generation. Workflow to be authored -based on the ComfyUI ACE-Step custom node. - -**Input schema:** - -```yaml -prompt: string # required (music description) -duration: number # seconds (default 30) -seed: integer # optional -output_path: string # where to save the audio -``` +**Future path:** Once a stable, widely-adopted ACE-Step node interface +emerges, or if ComfyUI adds native audio generation support, a +`comfyui_music` tool can be added following the same pattern as the image +and video tools. Users who have ACE-Step working can already use the +`workflow_json` override on any tool to run custom workflows. --- @@ -303,7 +290,7 @@ using OpenMontage's 7-dimension scoring: | Dimension | ComfyUI score | Rationale | |-----------|---------------|-----------| -| Task fit | High | Supports t2i, i2v, t2v, music | +| Task fit | High | Supports t2i, i2v, t2v | | Quality | High | Latest models (FLUX 2, WAN 2.2 14B) | | Control | Highest | Full workflow customization | | Reliability | High | Proven in production | @@ -323,8 +310,7 @@ HeyGen take over transparently. - **FLUX 2 Dev NVFP4** image generation -- Blackwell-optimized, ~60s per image - **WAN 2.2 14B** i2v with 4-step acceleration -- ~3.5 min per 5s clip -- **WAN 2.2 14B** t2v (models downloaded, workflow needed) -- **ACE-Step 3.5B** local music generation (model downloaded, workflow needed) +- **WAN 2.2 14B** t2v (models downloaded, workflow included) ### Future (add models to ComfyUI, no code changes to OpenMontage) @@ -350,17 +336,14 @@ compatibility matrices. ComfyUI is the abstraction layer. | Component | Files | Estimated size | |-----------|-------|----------------| -| Shared client | `tools/_comfyui/client.py` | ~120 lines | -| Image tool | `tools/graphics/comfyui_image.py` | ~130 lines | -| Video tool | `tools/video/comfyui_video.py` | ~160 lines | -| Music tool | `tools/audio/comfyui_music.py` | ~100 lines | -| Workflow templates | `tools/_comfyui/workflows/*.json` | 4 files | -| T2V workflow | `tools/_comfyui/workflows/wan22-t2v-4step.json` | 1 file (to author) | -| Music workflow | `tools/_comfyui/workflows/ace-step-music.json` | 1 file (to author) | -| Tests | `tests/contracts/test_comfyui_*.py` | ~80 lines | -| Docs | `skills/creative/comfyui-workflows.md` | Agent skill file | +| Shared client | `tools/_comfyui/client.py` | ~180 lines | +| Image tool | `tools/graphics/comfyui_image.py` | ~140 lines | +| Video tool | `tools/video/comfyui_video.py` | ~190 lines | +| Workflow templates | `tools/_comfyui/workflows/*.json` | 3 files | +| Tests | `tests/contracts/test_comfyui_tools.py` | ~200 lines | +| Docs | `docs/comfyui-adapter-plan.md` | This file | -**Total:** ~600 lines of Python + 4-6 workflow JSONs. +**Total:** ~500 lines of Python + 3 workflow JSONs. No changes to: `base_tool.py`, `tool_registry.py`, any selector, any existing tool, any pipeline definition, or any schema. @@ -373,13 +356,14 @@ existing tool, any pipeline definition, or any schema. user-provided via a config directory? Bundling gives reproducibility; external gives flexibility. -2. **Model discovery:** ComfyUI has a `/object_info` endpoint that lists - available nodes and models. Should `get_status()` also report which - models are loaded, so the selector can make informed routing decisions? - -3. **Async generation:** ComfyUI supports websocket connections for real-time +2. **Async generation:** ComfyUI supports websocket connections for real-time progress. Worth implementing for long video generations, or is polling sufficient? -4. **Multi-server:** Should the adapter support multiple ComfyUI instances +3. **Multi-server:** Should the adapter support multiple ComfyUI instances (e.g., one for images, one for video) via per-capability URLs? + +4. **Music generation:** ACE-Step works in ComfyUI but the node interface + isn't standardized across custom node packs. Need to either wait for + convergence or find a portable workflow pattern. See the `comfyui_music` + section above for details. diff --git a/tests/contracts/test_comfyui_tools.py b/tests/contracts/test_comfyui_tools.py index 98ab5b3..91de307 100644 --- a/tests/contracts/test_comfyui_tools.py +++ b/tests/contracts/test_comfyui_tools.py @@ -19,9 +19,8 @@ from tools.base_tool import ( ) from tools.graphics.comfyui_image import ComfyUIImage from tools.video.comfyui_video import ComfyUIVideo -from tools.audio.comfyui_music import ComfyUIMusic -TOOLS = [ComfyUIImage, ComfyUIVideo, ComfyUIMusic] +TOOLS = [ComfyUIImage, ComfyUIVideo] WORKFLOW_DIR = Path(__file__).resolve().parent.parent.parent / "tools" / "_comfyui" / "workflows" @@ -97,7 +96,6 @@ EXPECTED_WORKFLOWS = [ "flux2-txt2img.json", "wan22-i2v-4step.json", "wan22-t2v-4step.json", - "ace-step-music.json", ] @@ -222,7 +220,3 @@ class TestModelRequirements: assert len(_REQUIRED_MODELS_T2V) > 0 assert any("t2v" in m.lower() for m in _REQUIRED_MODELS_T2V) - def test_music_tool_has_required_models(self): - from tools.audio.comfyui_music import _REQUIRED_MODELS - assert len(_REQUIRED_MODELS) > 0 - assert any("ace" in m.lower() for m in _REQUIRED_MODELS) diff --git a/tools/_comfyui/workflows/ace-step-music.json b/tools/_comfyui/workflows/ace-step-music.json deleted file mode 100644 index 5f1af02..0000000 --- a/tools/_comfyui/workflows/ace-step-music.json +++ /dev/null @@ -1,27 +0,0 @@ -{ - "1": { - "class_type": "AceStepModelLoader", - "inputs": { - "model": "ace_step_v1_3.5b.safetensors" - } - }, - "2": { - "class_type": "AceStepSampler", - "inputs": { - "model": ["1", 0], - "prompt": "", - "lyrics": "", - "duration": 30.0, - "seed": 42, - "steps": 60, - "cfg": 3.0 - } - }, - "3": { - "class_type": "SaveAudio", - "inputs": { - "audio": ["2", 0], - "filename_prefix": "openmontage_music" - } - } -} diff --git a/tools/audio/comfyui_music.py b/tools/audio/comfyui_music.py deleted file mode 100644 index ebff88f..0000000 --- a/tools/audio/comfyui_music.py +++ /dev/null @@ -1,211 +0,0 @@ -"""ComfyUI music generation via ACE-Step model. - -Generates background music and songs locally using the ACE-Step 3.5B -model running inside a ComfyUI server. Custom workflows are accepted -via the ``workflow_json`` input. -""" - -from __future__ import annotations - -import json -import time -from pathlib import Path -from typing import Any - -from tools.base_tool import ( - BaseTool, - Determinism, - ExecutionMode, - ResourceProfile, - RetryPolicy, - ToolResult, - ToolRuntime, - ToolStability, - ToolStatus, - ToolTier, -) -from tools._comfyui.client import ComfyUIClient, ComfyUIError - -_WORKFLOWS = Path(__file__).resolve().parent.parent / "_comfyui" / "workflows" - -_OUTPUT_NODE = "3" - -_REQUIRED_MODELS = [ - "ace_step_v1_3.5b.safetensors", -] - - -class ComfyUIMusic(BaseTool): - name = "comfyui_music" - version = "0.1.0" - tier = ToolTier.GENERATE - capability = "music_generation" - provider = "comfyui" - stability = ToolStability.EXPERIMENTAL - execution_mode = ExecutionMode.SYNC - determinism = Determinism.SEEDED - runtime = ToolRuntime.LOCAL_GPU - - dependencies = [] - install_instructions = ( - "Start a ComfyUI server and set COMFYUI_SERVER_URL " - "(default http://localhost:8188).\n" - "Requires ACE-Step model (ace_step_v1_3.5b.safetensors) in " - "ComfyUI's checkpoints directory and the ACE-Step custom node installed." - ) - agent_skills = ["music"] - - capabilities = [ - "generate_background_music", - "generate_instrumental", - "generate_song", - "text_to_music", - ] - supports = { - "seed": True, - "duration_control": True, - "lyrics": True, - "custom_workflow": True, - "offline": True, - } - best_for = [ - "local music generation without API costs", - "background music and instrumentals for video production", - "song generation with lyrics", - ] - not_good_for = [ - "setups without a running ComfyUI server", - "highest quality commercial music (use Suno or ElevenLabs)", - ] - fallback = "suno_music" - fallback_tools = ["suno_music", "elevenlabs_music", "freesound_music"] - - input_schema = { - "type": "object", - "required": ["prompt"], - "properties": { - "prompt": { - "type": "string", - "description": "Music style / mood description (e.g. 'upbeat corporate background music')", - }, - "lyrics": { - "type": "string", - "default": "", - "description": "Optional lyrics for song generation", - }, - "duration": { - "type": "number", - "default": 30.0, - "description": "Duration in seconds", - }, - "steps": {"type": "integer", "default": 60}, - "cfg": {"type": "number", "default": 3.0}, - "seed": {"type": "integer", "description": "Random if omitted"}, - "output_path": {"type": "string", "description": "Where to save the audio"}, - "workflow_json": { - "type": "string", - "description": "Optional full ComfyUI workflow JSON (overrides default)", - }, - }, - } - - resource_profile = ResourceProfile( - cpu_cores=2, ram_mb=8000, vram_mb=6000, disk_mb=500, network_required=False, - ) - retry_policy = RetryPolicy(max_retries=1, retryable_errors=["timeout"]) - idempotency_key_fields = ["prompt", "lyrics", "duration", "steps", "seed"] - side_effects = ["writes audio file to output_path"] - user_visible_verification = ["Listen to generated audio for quality and mood match"] - - def __init__(self) -> None: - self._client = ComfyUIClient() - - def get_status(self) -> ToolStatus: - if not self._client.is_available(): - return ToolStatus.UNAVAILABLE - _, missing = self._client.check_models(_REQUIRED_MODELS) - if missing: - return ToolStatus.DEGRADED - return ToolStatus.AVAILABLE - - def estimate_cost(self, inputs: dict[str, Any]) -> float: - return 0.0 - - def estimate_runtime(self, inputs: dict[str, Any]) -> float: - duration = inputs.get("duration", 30.0) - return duration * 2.0 # rough: ~2x realtime - - def execute(self, inputs: dict[str, Any]) -> ToolResult: - if not self._client.is_available(): - return ToolResult( - success=False, - error=self._client.unavailable_reason(), - ) - - if not inputs.get("workflow_json"): - _, missing = self._client.check_models(_REQUIRED_MODELS) - if missing: - return ToolResult( - success=False, - error=( - f"ComfyUI server is running but missing required models: " - f"{', '.join(missing)}.\n" - f"Download them to your ComfyUI checkpoints directory." - ), - ) - - start = time.time() - seed = inputs.get("seed") or ComfyUIClient.random_seed() - duration = inputs.get("duration", 30.0) - output_path = Path( - inputs.get("output_path", f"comfyui_music_{seed}.wav") - ) - - try: - if inputs.get("workflow_json"): - workflow = json.loads(inputs["workflow_json"]) - else: - workflow = ComfyUIClient.load_workflow( - _WORKFLOWS / "ace-step-music.json" - ) - workflow = ComfyUIClient.patch_workflow(workflow, { - "2": { - "prompt": inputs["prompt"], - "lyrics": inputs.get("lyrics", ""), - "duration": duration, - "seed": seed, - "steps": inputs.get("steps", 60), - "cfg": inputs.get("cfg", 3.0), - }, - "3": {"filename_prefix": output_path.stem}, - }) - - paths = self._client.generate( - workflow, - output_node=_OUTPUT_NODE, - dest=output_path, - timeout=int(duration * 4), # generous timeout - ) - - except ComfyUIError as exc: - return ToolResult(success=False, error=str(exc)) - except Exception as exc: - return ToolResult(success=False, error=f"ComfyUI music generation failed: {exc}") - - return ToolResult( - success=True, - data={ - "provider": "comfyui", - "model": "ace-step-v1-3.5b", - "prompt": inputs["prompt"], - "lyrics": inputs.get("lyrics", ""), - "duration": duration, - "output": str(paths[0]), - "format": output_path.suffix.lstrip("."), - }, - artifacts=[str(p) for p in paths], - cost_usd=0.0, - duration_seconds=round(time.time() - start, 2), - seed=seed, - model="ace-step-v1-3.5b", - )