[Bug]: obs-ndi should recreate obs_output_t object if need

[walker-WSH]

Operating System Version

windows11

CPU

AMD

GPU

NVIDIA

NDI Devices

No response

OBS Version

30.2

OBS Installation Method

exe

DistroAV Version

4.11.1

DistroAV Installation Method

exe

[Extra] Installation Steps

No response

OBS Log [URL]

No response

NDI Version

4.11.1

Describe the bug

if user modify resolution or other parameters in settings window, then obs_reset_video is called and video_t in libobs will be free and recreated.

as obs's design, if obs_reset_video is called, all created obs_output_t should be recreated. otherwise, below variable is a deteled object and crash will happen when to access it.

struct obs_output
      video_t *video;

Steps to reproduce the problem

No response

Expected behavior

No response

Screenshots

No response

Additional context

No response

[walker-WSH]

I am not sure if this issue exits in latest code. please confirm it, thanks~

[walker-WSH]

Besides, for NDI output, everytime when to call obs_output_start, I think obs_output_set_media should be called firstly which ensure video_t saved in obs_output is not outdated.

void obs_output_set_media(obs_output_t *output, video_t *video, audio_t *audio)

I checked code in obs-ndi, there is no code about obs_output_set_media

[Trouffman]

Thank you for reporting this. There is quite a bunch of improvements that needs to happen on the output logic.

Did you experience the crash on the latest Master branch?

[walker-WSH]

hi @Trouffman ， latest version still include this crash that is caused by accessing outdated obs_output.

reproduce steps:

1. obs 30.2.3, install NDI 6.0.0
2. run obs -> open settings -> modify video resolution -> click OK
3. obs -> tool menu -> open ndi UI -> check "Main Output" -> click OK
4. crashed 100%

Reason: in step-2, all obs_video_mix are deleted; in step-3, outdated obs_output hold by ndi plugin is not recreated, including deleted video_t:

obs.dll!obs_output_start+0x260
   obs.dll!obs_output_actual_start+0x6b
      distroav.dll!ndi_output_start+0xd8
          obs.dll!video_output_get_format+0x14  (here access deleted video_t)

[walker-WSH]

I checked front event of obs, there is no event to notify that obs_reset_video will be called. so 3rd plugin of obs has no idea when should they recreate their obs_output.

So, I suggest that obs_output in ndi plugin should not be created until ndi does really need it.

[BitRate27]

Thanks for reporting this issue. I can duplicate it on the latest version of DistroAV. This appears to be similar to this bug and is addressed in https://github.com/DistroAV/DistroAV/pull/1148. I have verified OBS does not crash under this scenario when this PR is pulled.

This section of the PR description describes how the bug was fixed, and fixes the same problem you walked into, plus many other scenarios lurking out there.

_The problem existed because an output would be created even when no main output was selected. When the user went to change the color format, in the OBS Settings->Advanced dialog, this would invalidate the video output in DistroAV NDI Settings. Then when main output was turned back on, OBS would crash because output->video is corrupted.

This makes the changes to main-output look big but it was mainly removing an indented if condition. In summary we always call main_output_deinit in main_output_init, and then if it is not enabled, we return, otherwise we create the output and start it every time it is enabled._

If we don't use the error reporting logic, or disabling if not supported, I think we need to address this crash in the next release. Specifically, in https://github.com/DistroAV/DistroAV/pull/1148, the change to main_output_deinit in main-output.cpp fixes this problem.

[walker-WSH]

hi @BitRate27

thanks for your job. by the way, I have commited PR for OBS and add a front event to notify 3rd plugins to reset their obs_output or video_output. (wish to be merged~) https://github.com/obsproject/obs-studio/pull/11685

For obs, it is re-creating its obs_output after calling obs_reset_video, such as stream output, record output, and so on. of course, your method can also work well.

[tt2468]

Re-creating the output should be unnecessary. Simply re-apply the video_t* when a video reset happens.

[Lain-B]

Re-creating the output should be unnecessary. Simply re-apply the video_t* when a video reset happens.

Do we need a signal for this as what @walker-WSH made a pull request for?

Also, oddly, Matt says he can't reproduce.

[walker-WSH]

@tt2468 @Lain-B

（1）if obs_output of this plugin is binding with video_t of main view in libobs, after obs_reset_video, it just need reset its binded video_t again by "video_t obs_get_video()"

（2）if this plugin has created an obs_view_t and its output is binded with video_t returned by obs_view_add. in this scenario, after obs_reset_video it need to call obs_view_add again since old obs_video_mix will be freed.

for 3rd plugin developer, they do not know when to do (1) or (2). that is why I add new signals.

[BitRate27]

When distroav is outputting from main, or preview, the video output settings in OBS cannot be changed. What scenerio will trigger the new video_reset when distroav is ouputting?

[walker-WSH]

When distroav is outputting from main, or preview, the video output settings in OBS cannot be changed. What scenerio will trigger the new video_reset when distroav is ouputting?

reset won't be applied if any output is active.

if obs_view of ndi won't be added until starting, just ignore （2）.

[RayneYoruka]

I'll be closing this since there is already https://github.com/DistroAV/DistroAV/issues/1147 and https://github.com/DistroAV/DistroAV/pull/1148 tracking this. Reopen if necessary.