feat: NDI support & input sources abstraction#439
Conversation
Signed-off-by: gioelecerati <[email protected]>
860abd5 to
8b721a9
Compare
Signed-off-by: gioelecerati <[email protected]>
Signed-off-by: gioelecerati <[email protected]>
Signed-off-by: gioelecerati <[email protected]>
There was a problem hiding this comment.
I skimmed through the backend part. Nice work @gioelecerati I think it's in the good direction, I'll do the complete review when you finish your PR.
Some early comments:
- I see that right now you copy a lot of logic from the Spout received logic, but do I understand correctly that you plan to replace the spout logic and now we'll only use the input source?
- Now you've implemented the input, but I guess you're planning to do the same for the output. Is that correct?
- As a side note:
frame_processor.pyis pretty overloaded. I see that you created a separate packagecore/inputs. That's great. Also when you work on_SpoutFrameclass (or whatever you rename it to, consider moving it to a separate file. The same with some functions that are not strictly related to FrameProcessor. Thanks.
| # Handle generic input source settings | ||
| if "input_source" in parameters: | ||
| input_source_config = parameters.pop("input_source") | ||
| self._update_input_source(input_source_config) |
There was a problem hiding this comment.
Do we plan to allow changing the input_source when the stream is ongoing?
These parameters here are like "runtime" parameters. There are also the "intial_parameters" used during the initialization.
There was a problem hiding this comment.
On the UI I am disallowing to switch from NDI to other inputs and viceversa, but it's just a patch because I am not sure how resolution changes are handled - for example, my NDI source was 1920x1080, switching to the 512x512 example video kept the output video as 1920x1080 but stretched, if you have any insight about this I will work to improve it
src/scope/server/frame_processor.py
Outdated
| return sources | ||
|
|
||
|
|
||
| class _SpoutFrame: |
There was a problem hiding this comment.
If it's generic now, then I think we should rename it. Also, I think you can move it out of frame_processor.py
Yes, the spout receiver should use the new input source abstraction in the end
Yes! Would like to make a follow-up PR making a NDI sender. I need to hook it up with the NDI sdk detection, so we can display the option only if the runtime is running on the machine |
Signed-off-by: gioelecerati <[email protected]>
|
@leszko Marked this as ready for review after addressing your comments and tested it on windows
|
leszko
left a comment
There was a problem hiding this comment.
Very nice PR @gioelecerati . Tested NDI and Spout in Windows and everything works smoothly! 🎉
Added some inline comments, but other than that the PR looks good.
| export const getInputSourceResolution = async ( | ||
| sourceType: string, | ||
| identifier: string, | ||
| timeoutMs = 5000 | ||
| ): Promise<InputSourceResolution> => { | ||
| const response = await fetch( | ||
| `/api/v1/input-sources/${sourceType}/sources/${encodeURIComponent(identifier)}/resolution?timeout_ms=${timeoutMs}`, | ||
| { | ||
| method: "GET", | ||
| headers: { "Content-Type": "application/json" }, | ||
| } | ||
| ); | ||
|
|
||
| if (!response.ok) { | ||
| const errorText = await response.text(); | ||
| throw new Error( | ||
| `Probe resolution failed: ${response.status} ${response.statusText}: ${errorText}` | ||
| ); | ||
| } | ||
|
|
||
| return response.json(); | ||
| }; |
There was a problem hiding this comment.
How did we handle the resolution before? I mean the resolution that the Spout sent, didn't we need a similar logic?
There was a problem hiding this comment.
No Spout wasn't probing, it just had 512x512 hardcoded. Which I tried to do the same with NDI, but I was having the output trimmed or stretched depending on how I was applying it
This is also part of the reason why I went for not allowing source switch while you're on NDI, since the output will have the initial parameters resolution set as the NDI source resolution, ending up with a stretched output if you switch to camera or other source
Is there a way to change the resolution mid stream or are we planning to have that hot reloading parameters for w & h?
Either case, if it's possible to change res, I will address it in a follow up PR
src/scope/core/frames.py
Outdated
| """Wraps a raw numpy array to match the VideoFrame interface. | ||
|
|
||
| Allows code that expects a VideoFrame (with .to_ndarray()) to work | ||
| with plain numpy arrays from input sources like Spout and NDI. |
There was a problem hiding this comment.
I wonder for Spout, do we need the conversion from Tensor to VideoFrame? Isn't Spout all about keeping everything in GPU?
There was a problem hiding this comment.
Ooof actually this rawframe is not used anymore, I will delete it
Not sure if I am knowledgeable enough about spout to answer your question tho, can you elaborate?
Signed-off-by: gioelecerati <[email protected]>
Signed-off-by: gioelecerati <[email protected]>
Changes
How to test
In a follow-up PR