Thanks for such quick action and initiative on this.

In discord, you said:

So I somehow need to send over information that it was a clear frame so that the receiving end can ... well, clear it, and set the width and height to 0.

1. Why not check for ndi_video_frame->xres/yres == 0 in the source, instead of using picture_aspect_ratio == -1? Are they not set to 0 when receiving empty frames from a filter? A little hesitant here as NDI does not say how a -1 picture_aspect_ratio might effect the library or any other non-DistroAV NDI receivers.

2. What issue are you solving by adding the get_width and get_height on the ndi source? I feel this may actually fix some unrelated issues I have seen with ndi sources, just wondering why it was added for this fix. Also, the API doc for OBS says only that it is required for SYNCHRONOUS sources, which we are not.

3. I would suggest empty_frame instead of is_zero_width_height. However, not sure about adding that to ndi_filter_t as it is a simple test based on width and/or height. Is there another reason it is there?

1. Why not check for ndi_video_frame->xres/yres == 0 in the source, instead of using picture_aspect_ratio == -1? Are they not set to 0 when receiving empty frames from a filter? A little hesitant here as NDI does not say how a -1 picture_aspect_ratio might effect the library or any other non-DistroAV NDI receivers.

As I've mentioned above, I've tried with xres and / or yres set to 0, however it seems to send over an invalid frame and is thus never hitting the ndi_source_thread_process_video2 method. picture_aspect_ratio seemed to be the only one on a quick glance. I have not tested with other NDI receivers yet, don't have any available I think.

2. What issue are you solving by adding the get_width and get_height on the ndi source? I feel this may actually fix some unrelated issues I have seen with ndi sources, just wondering why it was added for this fix. Also, the API doc for OBS says only that it is required for SYNCHRONOUS sources, which we are not.

The main issue is making sure the source is set to 0x0 width and height, mimicing the behavior of the invalid / empty window capture on the sender side (or just window capture in general).
And while yes, the API does say it's required only for sync. sources, it still works for async sources just fine as an override.
I had to do an extra tweak for some methods though so I have access to the NDI source element instead of just the config.

3. I would suggest empty_frame instead of is_zero_width_height. However, not sure about adding that to ndi_filter_t as it is a simple test based on width and/or height. Is there another reason it is there?

I can't override known_width and known_height before I send an empty frame since I have to fill the frame buffer with zeroes when creating said empty frame for NDI to convert and send over. Hence why I used a new variable instead of just setting those values.
I kinda feel we could get away with filling a 1x1 width and height buffer though in the send_empty_frame method. Will try that tomorrow and report back

Okay I ran some more tests - turns out we can use the p_metadata field on the video frame.
I'm not sure if something in my code changed before when I was playing around with some other stuff but it could be that it initially worked, I then did some changes that broke it, and I didn't know... which changes haha

Either way - The NDI documentation mentions

It is very important that you compose legal XML messages for sending. (On receiving metadata, it is important that you support badly formed XML in case a sender did send something incorrect.)

So in this case we are using the string <empty_frame/>. I don't think any other program would listen to this. If we want to go an even better route to make sure, we could rename it to <distro_av_empty_frame/> or similar.

As for the empty frame width and height, I tested it and a 1x1 width and height worked. However, I think one extra frame in its last known width and height is okay and much more readable in code.

The changes you made look good and the justifications make sense. Did a lot of testing and think it is ready to go. Just some minor suggestions:

1. Use bzalloc instead of calloc when creating the empty frame in ndi-filter send_empty_frame.
2. Add the following to the beginning and end of send_empty_frame in ndi-filter.cpp:

    auto name = obs_source_get_name(filter->obs_source);
    obs_log(LOG_DEBUG, "+send_empty_frame('%s')", name);
    ...
    obs_log(LOG_DEBUG, "-send_empty_frame('%s')", name);

3. Add the following to the new if condition in ndi_source_thread_process_video2:

    obs_log(LOG_DEBUG, "Received empty frame ('%s')", source->config->ndi_source_name);

4. Perhaps this comment is better for the is_empty_frame:
   // Used in ndi_filter_raw_video to send empty_frame metadate to NDI
5. Add parenthesis to: f->is_empty_frame = (width == 0) || (height == 0);
6. Add comment before:
   // If width or height is 0 (when a capture window is closed), then send empty frame

As mentioned in the Discord, I could add an additional fix onto this which would kinda tie into the clear frame functionality.

In the current form, if the Dedicated NDI Output filter gets disabled, it will still send data over to the receiver (not even stick to the last frame).
The fix is rather simple - simply checking !obs_source_enabled(f->obs_source) in the same line as (width == 0) || (height == 0).

Now, there is one more issue - on ndi_filter_destroy it can be that the last known width and height due to that extra fix will end up as 0, meaning when trying to use bzalloc it will do so with a size of 0 and crash.

I'm waiting to hear back if implementing this extra functionality is also wanted or not before I do another push.

After some more trial and error I've managed to fix the disabling and removal of the filter properly.

For some reason NDI doesn't like 1x1 width and height frames so we just make sure we set it to 2x2 instead.

Also fixed formatting (Didn't have clang-format installed, whoops)

Because of concerns with the 2x2 empty frame, could you investigate destroying the NDI sender when a empty frame is received and creating the sender only when a non empty frame is received from the source? I would be happy to test out this kind of solution to closing a window which is being captured. Thanks for your effort so far! We will get a good solution here.

As talked about in the Discord, I've reworked it to destroy the NDI sender instead of sending a clear frame.
It will get created again once it detects proper frame data.

Additionally in the NDI source it's now checking for ndiLib->recv_get_no_connections == 0 and will output an empty frame locally / internally

@Lordmau5 thanks for the PR and the details.
Glad we are not lookign ot use the hack of the 2x2 which could wreak havoc in a lo of case (hardware encoders etc).

Workflow for testing will run and based on latest master.

Please check that the destroy/re-creation works as expected & that there is an entry in the log (should be IIRC)

Note: This will need thorough testing.

I'll be pushing some new code tomorrow that has a better implementation / fix for this, which means it would obviously need another review (and some more testing)

Also, this is probably incompatible with #1266, however I've noticed similar performance improvements with this new way of doing it in first tests.

The inherent main issue with these changes is that it is not proper off-screen rendering anymore (Say you have the source active in another scene - because it's enabled but not active it will not have a render callback and be sent via. NDI)

Hence why more testing is needed by users that have different setups with this and how it would impact them.

Ultimately I very much feel we're at a crossroads and have to choose which option to go for.

Additionally I should mention that this is groundwork for #1279 . Without this, the other PR won't work properly due to the same double render issues.

(Say you have the source active in another scene - because it's enabled but not active it will not have a render callback and be sent via. NDI)

This is a challenge: most users (from what I have seen) use the Ouput filter as a way to "send a specific source to NDI at all time", we are only "actively destroying this" when the filter_source is effectively "removed" or became an invalid source.

An output (General/Filter) should send a clear_frame whenever:
The feed is actively stopped like

• NDI output stopped
• NDI filter is disabled
• The OBS Source of the filter is no longer valid (resolution drop below 2x2px or get to 0x0)

In general the use-cases with filter are:
A. Filter is ACTIVE, Source is Active, Scene is Active (PRV/PGM) or Used on displays > render properly, clear frame only on invalid source.

B. Filter is ACTIVE, Source is Active, Scene is NOT Active (PRV/PGM) nor Used on displays > Should render properly, clear frame only on invalid source.

C. Filter is ACTIVE, Source is NOT Active, Scene is NOT Active (PRV/PGM) nor Used on displays > Should send Clear_frame + Stop render, but keep NDI sender registered (not sure if we can do that).

D. Filter is NOT ACTIVE. nothing else matter > Should send Clear_frame + Stop render, stop NDI sender (NDI source dissapear from network).

@Lordmau5 / @BitRate27 please flg if any of this is not possible, knowing that the part for the Global NDI output stop is not part of this PR.

I've switched it back to a draft for now just like the other PR I did.

I'll play around with some more ideas in the next few days as well as hope that Exeldro might have one, too (the filter chain issue with double rendering is affecting his plugin as well so I opened an issue on his repository)

I'm sure we can figure out a proper solution despite the "roadblocks" in OBS (by that I mean us misusing filters haha)