Skip to content

Conversation

@ischencheng
Copy link
Contributor

@ischencheng ischencheng commented Sep 16, 2025

Summary

Introduces a ComfyUI-to-BMF connector that runs ComfyUI graphs on the BMF engine without bundling GPL3.0 code.
Fixes a crash in FrameSync input manager when inputs arrive at different timestamps (mixed-rate/mixed-type flows).

Part 1 — Feature: ComfyUI Connector

What

Runtime integration that converts a ComfyUI prompt graph into a BMF GraphConfig and executes it in-process.

How

  • demo/comfyui_intergration/bmf_runner.py: Installs a lightweight runtime hook into execution.PromptExecutor.execute (no on-disk modification) and routes execution into BMF when enabled.
  • demo/comfyui_intergration/bridge.py:
    • BmfWorkflowConverter builds a BMF graph from the ComfyUI prompt.
    • ComfyNodeRunner wraps each Comfy node as a BMF module with:
      • zero-copy tensor paths (hmp <-> torch via DLPack),
      • simple model-output caching for loader nodes,
      • progress/preview alignment with comfy_execution.progress,
      • guarded input decoding to avoid null/invalid packets.
  • demo/comfyui_intergration/run_bmf_comfy.py: Launches ComfyUI with the runtime hook installed.
  • demo/comfyui_intergration/setup.sh: One-step environment setup and pinned ComfyUI checkout.

Why

  • Avoid copying GPL-licensed code; only use public modules at runtime.
  • Keep ComfyUI UX: progress bars, previews, and history outputs remain aligned.
  • Zero-copy data flow reduces CPU/GPU overhead.

Part 2 — Bugfix: FrameSync crash under mixed timestamps

  • FrameSync scans inputs, records the next packet per stream (if any), computes the global minimum timestamp min_ts, and only “advances” streams whose next packet timestamp equals min_ts.
  • For streams whose next packet is greater than min_ts, curr_pkt_ remains unset. Previously, the code unconditionally called set_timestamp() on curr_pkt_[stream_id]. Because operator[] default-constructs an empty Packet, this dereferenced an invalid packet and crashed.

Fix in this MR:

  • Add a defensive null-guard before set_timestamp() and only enqueue defined packets for each stream. This prevents creating invalid packets and eliminates the segfault.
  • On the Comfy side, the node runner checks that all input queues are non-empty before executing. If any input is missing, it simply waits, so mixed-rate/mixed-type graphs (e.g., KSampler receiving model/vae at ts=1 and latent at ts=2) no longer crash.

Limitations & follow-ups:

  • Some Comfy nodes truly require a strict “all inputs present” barrier at the min_ts step.
  • If this fix would change FrameSync semantics in an unacceptable way, consider adding a new input manager (e.g., “AlignedFrameSync”).
  • DefaultInputManager/ImmediateInputStreamManager intentionally do not provide this barrier behavior.

Reproducer (why it used to crash)

  • Graph with KSampler(e.g. image2image template in ComfyUI):
    • model/vae from a loader arrive once at ts=1.
    • latent_image arrives later at ts=2 (after VAEEncode finishes).
  • FrameSync selected min_ts=1, advanced model/vae streams, left latent_image unset.
  • Code then attempted set_timestamp() on a default-constructed Packet for latent_image, causing a null dereference.
  • The guard added in this MR prevents that; the runner waits until latent_image arrives before executing the node.

@ischencheng ischencheng changed the title Mr/feat comfyui connector and fix framesync null guard MR/feat comfyui connector and fix framesync null guard Sep 16, 2025
@ischencheng ischencheng changed the title MR/feat comfyui connector and fix framesync null guard feat comfyui connector and fix framesync null guard Sep 16, 2025
@hulibruce hulibruce self-assigned this Sep 30, 2025
Copy link
Collaborator

@hulibruce hulibruce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good Job! Thanks for the contribution.

@hulibruce
Copy link
Collaborator

@ischencheng cloud you please rebase the latest code in master branch? It might fix the CI issue we encountered.

@ischencheng ischencheng force-pushed the mr/feat-comfyui-connector-and-fix-framesync-null-guard branch from 2ae22d5 to 0a0a2b5 Compare October 27, 2025 14:48
@ischencheng
Copy link
Contributor Author

ischencheng commented Oct 27, 2025

@hulibruce Sure—I rebased, and the CI issue is fixed.

@hulibruce hulibruce merged commit 6acdc63 into BabitMF:master Oct 28, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants