Conversation
1b7dc4e to
3fb8d72
Compare
| call_params["video"] = video_input | ||
|
|
||
| output = self.pipeline(**call_params) | ||
| output_dict = self.pipeline(**call_params) |
There was a problem hiding this comment.
should we extract other keys and forward them to handle processors that produce auxiliary outputs?
There was a problem hiding this comment.
Yes, I think we should actually pass the whole dict into the next processor.
But... I'd prefer to do it in a separate PR to keep this PR dedicated to just the interface change.
One thing to think about is how to pass data between pipeline processors.
- For video, we currently extract individual frames and add then to the output_queue
- If we want to pass all outputs, that poses a question how to pass them:
- One option is what you did here, so treat "video" the same as before and all other params just set as parameter
- More correct and long-term option would be to queue the whole "dict", but then we would need to rethink how the queuing and waiting works, because the each pipeline can produce / consume different n of frames
yondonfu
left a comment
There was a problem hiding this comment.
LGTM
I think this is a step in the right direction. A proper mapping of inputs <> outputs in a pipeline chain likely would benefit from some sort of binding (eg which input binds to which output) and typing system. But, we don't need to solve for that right now and can let any requirements for that emerge with use cases.
Signed-off-by: Rafal Leszko <rafal@livepeer.org>
3fb8d72 to
71cd5bf
Compare
Summary
After reviewing recent PRs (#368 and #312), it’s clear the current
Pipelineinterface no longer matches real usage. This PR updates the interface to support existing and emerging pipeline patterns.Issues
Pipeline.__call__()accepts aninputparameter that is unused by all pipelines.Pipeline.__call__()returns a singletorch.Tensor, which does not support:Changes
inputargument fromPipeline.__call__()dict"video"keyauto_input_sizeto pipeline requirements to support dynamic frame countsCompatibility
Plugins are not yet published, so this change is intentionally not backward-compatible. We could make it backward-compatible, but I think it's not worth it. Better to update all existing pipelines.