Skip to content

Conversation

@eliteprox
Copy link
Collaborator

@eliteprox eliteprox commented May 20, 2025

This change sets the ComfyUI pipeline to resize incoming video streams to match the expected resolution of the workflow by parsing the Empty Latent Image node from the workflow prompt, safely falling back to a default of 512x512

Functional notes:

  • During pipeline warm-up, the resolution from the default workflow is used
  • When a new stream is started, the pipeline is restarted with the resolution for the provided workflow. Incoming streams are cropped and resized to the expected resolution and the output dimensions will be the same
  • Provided resolution must be within 64 pixel increments and within min/max sizes of the pre-compiled engine

Related comfystream changes livepeer/comfystream#250

@eliteprox eliteprox force-pushed the feat/dynamic-resolution branch 2 times, most recently from 290eeee to 53c82eb Compare May 21, 2025 16:09
@eliteprox eliteprox marked this pull request as ready for review May 21, 2025 16:10
Copilot AI review requested due to automatic review settings May 21, 2025 16:10
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request adds support for custom input/output resolution parameters in the comfyui live pipeline by propagating width and height values from the request to the encoder, decoder, and workflow configuration. Key changes include modifying the subscribe, publish, encoder, and decoder functions to accept and use dynamic resolution parameters, updating the default workflow JSON to reflect the new resolutions, and propagating resolution settings in the streamer.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
runner/app/live/trickle/media.py Updated to pass output_width and output_height to decode_in and encode_in.
runner/app/live/trickle/encoder.py Modified to dynamically set video resolution in the encoder.
runner/app/live/trickle/decoder.py Adjusted decoding to enforce requested dimensions.
runner/app/live/streamer/streamer.py Introduced resolution settings but with a potential bug in target_height assignment.
runner/app/live/streamer/protocol/trickle.py Updated to pass resolution parameters when calling media.run_subscribe and media.run_publish.
runner/app/live/pipelines/comfyui_default_workflow.json Updated workflow engine name and array formatting to reflect new resolution support.
runner/app/live/pipelines/comfyui.py Modified initialization to update latent image dimensions and warmup pipeline with dynamic resolutions.
Comments suppressed due to low confidence (1)

runner/app/live/pipelines/comfyui.py:110

  • The variables 'width' and 'height' are not defined in this scope. It may be intended to use new_params.width and new_params.height instead, so please adjust the initialization logic accordingly.
if width is None or height is None:

@eliteprox eliteprox force-pushed the feat/dynamic-resolution branch from 34b499a to 03c8e7f Compare May 22, 2025 19:21
@eliteprox eliteprox marked this pull request as draft May 23, 2025 01:22
@eliteprox eliteprox force-pushed the feat/dynamic-resolution branch 3 times, most recently from 8bf3d8e to bae6941 Compare May 28, 2025 02:37
@eliteprox eliteprox marked this pull request as ready for review May 28, 2025 02:37
@eliteprox eliteprox requested a review from Copilot May 28, 2025 16:12
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces custom resolution support for the ComfyUI live pipeline by parsing the resolution from the workflow’s EmptyLatentImage node. Key changes include:

  • Extracting latent image dimensions from the workflow JSON.
  • Propagating custom output width and height through subscription, decoding, encoding, streaming, and pipeline initialization.
  • Updating default resolution values and adjusting image resizing behavior to use the target resolution.

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
runner/app/live/utils/comfy_utils.py Introduces utility to extract latent image dimensions from workflows.
runner/app/live/trickle/media.py Updates run_subscribe to accept output width and height parameters.
runner/app/live/trickle/encoder.py Adapts video stream options using dynamic resolution values.
runner/app/live/trickle/decoder.py Uses passed resolution values when reformatting video frames.
runner/app/live/streamer/streamer.py Passes target dimensions to protocols and removes hardcoded cropping.
runner/app/live/streamer/protocol/trickle.py Propagates resolution settings to the subscribe function.
runner/app/live/streamer/process_guardian.py Checks for resolution changes and restarts the process if needed.
runner/app/live/pipelines/comfyui_default_workflow.json Updates default workflow dimensions to match the new resolution setup.
runner/app/live/pipelines/comfyui.py Warms up the pipeline using the workflow’s resolution values.
Comments suppressed due to low confidence (2)

runner/app/live/utils/comfy_utils.py:5

  • The default resolution values (384x704) differ from the PR description's fallback of 512x512; please confirm if this discrepancy is intentional or if the documentation should be updated.
DEFAULT_WIDTH = 384

runner/app/live/streamer/streamer.py:198

  • The previous cropping logic for non-square images has been removed in favor of direct resizing, which may lead to image distortion if the aspect ratios differ. Please verify that this change in behavior is intended.
if (width, height) != (self.width, self.height):

@eliteprox eliteprox force-pushed the feat/dynamic-resolution branch 6 times, most recently from 65eeee6 to 84ded4a Compare June 2, 2025 20:26
@eliteprox eliteprox requested review from emranemran and pschroedl June 2, 2025 21:17
@eliteprox eliteprox force-pushed the feat/dynamic-resolution branch 4 times, most recently from 280f87e to 83d6b52 Compare June 5, 2025 19:29
return web.json_response(status.model_dump())


async def handle_restart_pipeline(request: web.Request):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For testing only, can be removed

@eliteprox eliteprox force-pushed the feat/dynamic-resolution branch 4 times, most recently from 290dfaf to e0c7aea Compare June 10, 2025 14:19
@eliteprox eliteprox force-pushed the feat/dynamic-resolution branch from b84b6c0 to 084e11b Compare June 10, 2025 17:32
@eliteprox eliteprox force-pushed the feat/dynamic-resolution branch from 3d18216 to bc5191f Compare June 10, 2025 18:30
self.streamer = streamer or _NoopStreamerCallbacks()

self.process.reset_stream(request_id, manifest_id, stream_id)
self.process.update_params(params)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

likely not needed

if not self.process:
raise RuntimeError("Process not running")

# Check if resolution has changed
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will remove, comfyui pipeline will stop itself

@eliteprox eliteprox marked this pull request as draft June 12, 2025 19:59
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.

2 participants