Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Source: add support for auto-disconnecting NDI connection #993

Merged
merged 1 commit into from
May 10, 2024

Conversation

haakonnessjoen
Copy link
Contributor

  • Adds a new source property called "Behavior" that lets you choose between the old behavior "Keep source connected" which keeps the NDI source connected at all times, and the new feature "Disconnect source when not visible" which will disconnect NDI to preserve CPU and network resources while sources are not visible.

  • Adds a "Keep last frame when disconnected" option for the previous behavior setting, so that you can choose if the last frame should be kept, or that the source texture should reset when disconnected.

This might solve issue #880

This PR fixes issues I have had with having more than a few NDI sources, it is easy to use up a 1 Gbit ethernet connection if each source pulls 200mbit/s of network data. It's was impossible to add the 20 sources I wanted to add, but with this feature, it is possible. It behaves like the vlc plugin, which also has support for choosing if the source should stop the playback while the source is not active.

* Adds a new source property called "Behavior" that lets you
  choose between the old behavior "Keep source connected" which keeps the
  NDI source connected at all times, and the new feature "Disconnect
  source when not visible" which will disconnect NDI to preserve CPU and
  network resources while sources are not visible.

* Adds a "Keep last frame when disconnected" option for the previous
  behavior setting, so that you can choose if the last frame should be
  kept, or that the source texture should reset when disconnected.

This might solve issue DistroAV#880
@Trouffman
Copy link
Collaborator

A stop / start on the source detail would be a good addition as well so sources cane be paused (properly by stopping the network pull).

@haakonnessjoen
Copy link
Contributor Author

A stop / start on the source detail would be a good addition as well so sources cane be paused (properly by stopping the network pull).

Is this necessary if you have the option for it to only connect when it is visible/active? Do you have an example use case?

@paulpv
Copy link
Member

paulpv commented Apr 25, 2024

Per my #880 (comment) comment, stopping the NDI stream can be undesirable because it can cause a delay when starting back up.

I understand that this PR makes it an option to turn off the NDI stream and defaults to the existing behavior.

Let me think about this a little bit; nudge me if I take over a week.

@paulpv paulpv self-assigned this Apr 25, 2024
@haakonnessjoen
Copy link
Contributor Author

I understand that in some cases it would ve undesirable, but this is also a feature in the built in vlc plugin. So it is an existing workflow in obs. Also the only way to handle more than a few ndi sources in obs without using up all your resources. In my humble opinion it should be up to the user if they want to risk the delay. With the «remember last frame» feature, it is seldom noticable, especially if you use fade transitions. I just wanted to add this. Please think about it :)

@paulpv paulpv self-requested a review May 10, 2024 08:35
@paulpv paulpv merged commit 4177d8b into DistroAV:master May 10, 2024
6 checks passed
@paulpv
Copy link
Member

paulpv commented Aug 9, 2024

@haakonnessjoen A few questions about your intentions behind this merged PR.

The options are you wrote them are:
image

  • Behavior: (dropdown)
    1. Keep source connected
    2. Disconnect source when not visible
  • Keep last frame when disconnected (checkbox)

This UI makes it possible to enable Keep last frame when disconnected when Keep source connected.

In your opinion of this change, does that make any sense?

I am thinking of changing the UI for this to be:

  • Visibility Behavior: (dropdown)
    1. Not-Visible: Keep Active
    2. Not-Visible: Pause; Visible: Show Black Frame & Resume
    3. Not-Visible: Pause; Visible: Show Last Frame & Resume

image

This would make it impossible/undefined to use Keep last frame when disconnected when Keep source connected.

Does this seem reasonable?

We are talking about this more in:

And I am preparing a PR to make some changes to make this more understandable to the masses. 😆

If you aren't already, feel free to join our Discord server to talk more about this:
https://discord.gg/ZuTxbUK3ug

CC @Trouffman

@haakonnessjoen
Copy link
Contributor Author

I guess it makes sense. If I understand #1046 correctly you could think that not checking this check box would make the source go black if the source disconnects? That could maybe be a feature that would be nice to have in addition though. An option to clear the frame if no new frame has been received in x amount of time. Or should it be combined with this feature, and then the options could stay the way they were?

If I recall correctly, I made it/worded it this way since the VLC plugin has similar options. So I made it so you find options that you might be familiar with if you are a VLC user. But maybe the VLC plugin also handles sources being removed with the same option? If not we should maybe try to get the VLC plugins options changed too?

@paulpv
Copy link
Member

paulpv commented Aug 9, 2024

So you would not object if we tweaked the feature a little bit to make more sense to... me/us? 😄

If I recall correctly, I made it/worded it this way since the VLC plugin has similar options. So I made it so you find options that you might be familiar with if you are a VLC user. But maybe the VLC plugin also handles sources being removed with the same option? If not we should maybe try to get the VLC plugins options changed too?

Can VLC receive and play NDI sources? I played with the VLC NDI Plugin a bit today and see how to enable it to output NDI audio and video, but I do not see any way for it to view NDI sources.

@paulpv
Copy link
Member

paulpv commented Aug 9, 2024

you could think that not checking this check box would make the source go black if the source disconnects? That could maybe be a feature that would be nice to have in addition though. An option to clear the frame if no new frame has been received in x amount of time. Or should it be combined with this feature, and then the options could stay the way they were?

It might be useful to have an option around the if (ndiLib->recv_get_no_connections(ndi_receiver) == 0) line (https://github.com/DistroAV/DistroAV/blob/master/src/obs-ndi-source.cpp#L605) that if the video transitions between a receiving state (connections != 0) and a not receiving state (connections == 0) then appropriately clear the video or not based on the checkbox.

Regardless, we'll probably play with this feature a bit. Just a heads up. Thank you for your code contribution!

@haakonnessjoen
Copy link
Contributor Author

So you would not object if we tweaked the feature a little bit to make more sense to... me/us? 😄

No, I have nothing against it. :) As long as it still does the main thing I added 😅

If I recall correctly, I made it/worded it this way since the VLC plugin has similar options. So I made it so you find options that you might be familiar with if you are a VLC user. But maybe the VLC plugin also handles sources being removed with the same option? If not we should maybe try to get the VLC plugins options changed too?

Can VLC receive and play NDI sources? I played with the VLC NDI Plugin a bit today and see how to enable it to output NDI audio and video, but I do not see any way for it to view NDI sources.

No, not NDI, just external video sources in general. But it has the same feature in regards to letting it disconnect from the video-source when the source/plugin is not visible, to free resources/bandwidth.

@BitRate27
Copy link
Contributor

I had the following bug using the behavior setting:
0. Select Studio Mode

  1. Create a scene "Scene 1" with two ndi sources: NDI 1 and NDI 2
  2. Hide NDI 2
  3. Change the behavior for NDI 2 to Disconnect
  4. Exit OBS
  5. Restart OBS
  6. Select "Scene 1" (should be default)
  7. Unhide NDI 2
    Expected behavior: NDI 2 shown in preview
    Actual behavior: NDI 2 is not shown

It seems there is an issue with OBS studio not calling our show or hide callback when a source is shown/hidden by clicking the eye icon in Studio Mode. Seems to work in normal mode. Also, if we try to work around this issue by checking if the source is actually shown in the thread, obs_source_showing always reports true, no matter what the eye icon is set to.

@haakonnessjoen
Copy link
Contributor Author

This is known issue with OBS. Or at least for me. This also happens with the VLC plugin for other video sources.

I would guess this is a "feature" in OBS, since in studio mode you have preview/program. And things in preview is not considered live right. So sources are only told that they are active when they are on the "program" output. The reason I think this is considered a feature, is that it makes it easier for people to build systems that will have a "tally" that tells that the source is in fact "live", and not only in a preview output.

@paulpv
Copy link
Member

paulpv commented Aug 12, 2024

I had the following bug using the behavior setting: 0. Select Studio Mode

  1. Create a scene "Scene 1" with two ndi sources: NDI 1 and NDI 2
  2. Hide NDI 2
  3. Change the behavior for NDI 2 to Disconnect
  4. Exit OBS
  5. Restart OBS
  6. Select "Scene 1" (should be default)
  7. Unhide NDI 2
    Expected behavior: NDI 2 shown in preview
    Actual behavior: NDI 2 is not shown

It seems there is an issue with OBS studio not calling our show or hide callback when a source is shown/hidden by clicking the eye icon in Studio Mode. Seems to work in normal mode. Also, if we try to work around this issue by checking if the source is actually shown in the thread, obs_source_showing always reports true, no matter what the eye icon is set to.

I think this is precisely what @Trouffman fixed in the 6.0.0_actual branch via 85a3110

It will be in master once 6.0.0_actual is merged.

@BitRate27
Copy link
Contributor

I duplicated the issue on 6.0.0_actual. His changes appear to work in normal mode but not Studio mode.

@BitRate27
Copy link
Contributor

This is known issue with OBS. Or at least for me. This also happens with the VLC plugin for other video sources.

I would guess this is a "feature" in OBS, since in studio mode you have preview/program. And things in preview is not considered live right. So sources are only told that they are active when they are on the "program" output. The reason I think this is considered a feature, is that it makes it easier for people to build systems that will have a "tally" that tells that the source is in fact "live", and not only in a preview output.

Even if the source is not shown in program or preview, there is no way to determine this using the API. I dont think it is a feature and this problem leads to a tally bug where a source in program will show as also in preview, even if it it isn't.

@BitRate27
Copy link
Contributor

BitRate27 commented Aug 13, 2024

Do you know if this has been reported as an issue in OBS? I couldnt find one.

@Trouffman
Copy link
Collaborator

This is known issue with OBS. Or at least for me. This also happens with the VLC plugin for other video sources.

I would guess this is a "feature" in OBS, since in studio mode you have preview/program. And things in preview is not considered live right. So sources are only told that they are active when they are on the "program" output. The reason I think this is considered a feature, is that it makes it easier for people to build systems that will have a "tally" that tells that the source is in fact "live", and not only in a preview output.

Even if the source is not shown in program or preview, there is no way to determine this using the API. I dont think it is a feature and this problem leads to a tally bug where a source in program will show as also in preview, even if it it isn't.

This is why I changed the OBS source status trigger from source ".activated" = on stream/program to the source ".shown" = not visible on any screen.

Make sure that you do not have a filter on any of the scenes as well, because of an NDI filter is on a scene, this will trigger the "is visible".

If you could file a bug with the full log that will help diagnose.

@BitRate27
Copy link
Contributor

I created an issue: #1078

Was able to get the ndi_source_shown called when I unhide the source (not sure why it wasn't being called before).

It seems we need to call ndi_source_update to restart the thread when we get the ndi_source_shown call. It is also necessary to change the check for restarting the thread to look for obs_source_showing instead of obs_source_active as @Trouffman mentioned above.

Adding the following line in ndi_source_shown will fix the issue:
ndi_source_update(data, obs_source_get_settings(s->obs_source));

I would make a pull request, but unsure which branch to use.

@paulpv
Copy link
Member

paulpv commented Aug 15, 2024

Adding the following line in ndi_source_shown will fix the issue: ndi_source_update(data, obs_source_get_settings(s->obs_source));

I would make a pull request, but unsure which branch to use.

Create your PR from either master or 6.0.0_actual.
I prefer you create a PR from 6.0.0_actual.

There have been a lot of changes to the code in the past few days.
Retry what is there and then make a PR as you see appropriate.

@BitRate27
Copy link
Contributor

@paulpv I closed the issue I created because your latest commits to 6.0.0_actual fixed the problem. Actually, starting the thread in the ndi_source_shown callback is better than my overkill suggestion of calling the ndi_source_update. Nice work!

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.

4 participants