Show user why NDI output failed #1148

BitRate27 · 2024-11-18T22:27:14Z

When the user selects a color format of P010, I010, P216 or P416, the output will not be started because the NDI SDK does not support HDR color formats. The only indication this is a problem is by looking for "unsupported pixel format" in the log file. To make it easier for the user to identify the problem, we will now report the last error in the DistroAV NDI Settings dialog for the main output.

This message will be seen when the user goes back to the settings dialog after noticing there is no NDI output.

This does not happen for preview or dedicated NDI output from a filter, so no changes were needed for those outputs.

In order to test and utilize this new message, this PR also fixes #1147 where OBS crashes after you have tried to output an unsupported format.

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.

Tested on Windows by testing that all formats gave the correct message if unsupported, or blank if supported.

RayneYoruka · 2024-11-18T22:30:16Z

I like the idea of this being added, please @paulpv or @Trouffman check it

src/main-output.cpp

Trouffman · 2024-12-04T00:13:14Z

Thanks for the extensive PR.
Is there a way that it could be displayed without being set in the config file?

I see we could re-use the pixel format detection as well if one day we can use the advanced SDK.

BitRate27 · 2024-12-04T21:57:46Z

I think it is possible to add a method to set the last error to the OutputSetting dialog and display that string instead when the dialog is displayed. I can redo that part.

BitRate27 · 2024-12-06T22:32:14Z

Added bb765a9 to the branch and the PR.

I removed the LastError from the config. Ended up adding last_error to the main-output context and making the last error message accessible from output-settings.

Also added translations for all the locales we support. I keyed off the phrase "Color Format" which is the name of the field the user changes. So the error message should point the user to what caused the error.

BitRate27 · 2024-12-07T22:21:36Z

I am looking into a way to keep the user from turning on main output if the color format not supported.

BitRate27 · 2024-12-10T21:57:17Z

After this latest commit ef7e0ec , the main output box is only enabled if the pixel format of the main output is supported by the NDI SDK. If it is not supported, the main output box is disabled and an error message appears showing why main output cannot be chosen.

Trouffman · 2024-12-17T16:20:45Z

Thank you for updating the behavior on this.

Next : testing & QC.

Trouffman · 2025-01-04T09:37:19Z

I just found a bug while investigating another behavior.

Documented bug at : #1180

Trouffman · 2025-02-10T03:28:50Z

@BitRate27 I merged from the latest master, please review to confirm that this still has the intended purpose.

Trouffman

Issue found : This is "Stopping" the Main Output when opening the DistroAV Settings and is not re-created if "cancelled".

Expected Behavior : The stream should NOT stop until there is a need for:

Stopping (aka not enabled)
Name or group name changed
All above only after the form has been submitted.

Error log shows : info: Output 'NDI Main Output': stopping

Trouffman · 2025-04-24T12:48:27Z

This is what happen when opening the NDI Settings dialog box :

08:28:52.056: [distroav] +main_output_init()
08:28:52.057: [distroav] +main_output_deinit()
08:28:52.057: [distroav] main_output_deinit: NDI Main Output is not initialized
08:28:52.057: [distroav] -main_output_deinit()
08:28:52.057: [distroav] main_output_init: creating NDI Main Output 'OBS-PGM'
08:28:52.057: [distroav] Creating NDI Main Output : 'OBS-PGM'
08:28:52.057: [distroav] +ndi_output_getdefaults()
08:28:52.057: [distroav] -ndi_output_getdefaults()
08:28:52.057: [distroav] +ndi_output_create(name='OBS-PGM', groups='', ...)
08:28:52.057: [distroav] ndi_output_update(name='OBS-PGM', groups='', ...)
08:28:52.057: [distroav] NDI Output Updated. 'OBS-PGM'
08:28:52.057: [distroav] ndi_output_update(name='OBS-PGM', groups='', uses_video='true', uses_audio='true')
08:28:52.057: [distroav] -ndi_output_create(name='OBS-PGM', groups='', ...)
08:28:52.057: [distroav] main_output_init: created NDI Main Output 'OBS-PGM'
08:28:52.057: [distroav] +main_output_start()
08:28:52.057: [distroav] main_output_start: starting NDI Main Output 'OBS-PGM'
08:28:52.057: [distroav] +ndi_output_start(name='OBS-PGM', groups='', ...)
08:28:52.058: [distroav] +on_main_output_started()
08:28:52.058: [distroav] -on_main_output_started()
08:28:52.058: [distroav] NDI Main Output started
08:28:52.058: [distroav] 'OBS-PGM' ndi_output_start: ndi output started
08:28:52.058: [distroav] -ndi_output_start(name='OBS-PGM', groups=''...)
08:28:52.058: [distroav] main_output_start: successfully started NDI Main Output 'OBS-PGM'
08:28:52.058: [distroav] -main_output_start()
08:28:52.058: [distroav] -main_output_init()
08:28:52.058: [distroav] +main_output_deinit()
08:28:52.058: [distroav] +main_output_stop()
08:28:52.058: [distroav] main_output_stop: stopping NDI Main Output 'OBS-PGM'
08:28:52.058: [distroav] +ndi_output_stop(name='OBS-PGM', groups='', ...)
08:28:52.058: Output 'NDI Main Output': stopping
08:28:52.058: Output 'NDI Main Output': Total frames output: 0
08:28:52.058: Output 'NDI Main Output': Total drawn frames: 0
08:28:52.059: [distroav] +on_main_output_stopped()
08:28:52.059: [distroav] -on_main_output_stopped()
08:28:52.059: [distroav] NDI Main Output stopped
08:28:52.059: [distroav] ndi_output_stop: +ndiLib->send_destroy(o->ndi_sender)
08:28:52.063: [distroav] ndi_output_stop: -ndiLib->send_destroy(o->ndi_sender)
08:28:52.063: [distroav] NDI Output Stopped. 'OBS-PGM'
08:28:52.063: [distroav] -ndi_output_stop(name='OBS-PGM', groups='', ...)
08:28:52.063: [distroav] main_output_stop: successfully stopped NDI Main Output 'OBS-PGM'
08:28:52.063: [distroav] -main_output_stop()
08:28:52.063: [distroav] main_output_deinit: releasing NDI Main Output 'OBS-PGM'
08:28:52.063: [distroav] +ndi_output_destroy(name='OBS-PGM', groups='', ...)
08:28:52.063: [distroav] -ndi_output_destroy(name='OBS-PGM', groups='', ...)
08:28:52.063: [distroav] main_output_deinit: successfully released NDI Main Output ''
08:28:52.063: [distroav] -main_output_deinit()

This is because of the logic for main_output_is_supported() that will "test" by creating the output. But this conflict with the "on-going" output.

This should be triggered only when loading a new profile / starting OBS and not everytime the NDI Settings are opened.

Other after testing :

The box should be greyed ONLY if not supported (and that we update this based on "any" type of change that could have a different color format/resolution etc :

Change of color format
Change of Profile
Selecting a valid format after an invalid format was used.

When the box is greyed out, this is the only time when we should display the error message.
The error message space in UI should only be present if there is an error (not sure if this could be set back to height = 0)

BitRate27 · 2025-04-24T16:28:57Z

Thanks for looking at this. You brought up some good issues with this implementation. Clearly I need to rethink the logic here. I think we need to initialize a test output instead of using the main output. Not sure how to only show the "last error" row in the dialog if no error. Suggestions welcome.

Here are the scenarios:

New profile loaded with main output OFF and format NOT supported
- Turn off main output
- Main output: not started
- Dialog: Main output group disabled, not checked and error shown
New profile loaded with main output OFF and format supported
- Turn off main output
- Main output: not started
- Dialog: Main output group enabled, not checked and no error shown
  - OK and main output checked -> Main output started
  - OK and main output not checked -> Main output not started
  - Cancel -> Main output not started
New profile loaded with main output ON and format supported
- Turn off main output
- Main output: started
- Dialog: Main output group enabled, checked and no error shown
User changes to a format NOT supported
- Dialog: Main output group disabled, not checked and error shown
- OK/Cancel: Main output not started
User changes to a format supported
- Dialog: Main output group enabled, not checked and no error shown
  - OK and main output checked -> Main output started
  - OK and main output not checked -> Main output not started
  - Cancel -> Main output not started

BitRate27 · 2025-04-25T13:50:35Z

Update: was able to alter the main_output_is_supported to check without using main output. Also, figured out how to set the message height to zero if no error message. Only thing left is to work on the LOG_INFO and LOG_DEBUG messages. All tests in the scenarios above passed. Will also squash the branch to one commit as this went through a lot of changes.

src/forms/output-settings.cpp

src/main-output.cpp

Trouffman · 2025-05-05T00:53:20Z

Tested on Windows 11+ OBS 31 after the latest merge :

The default output does not work once a non compatible profile is used, same when going back to an expected to work profile.

Seem our logic here is not polished enough.

EDIT : Seems my test system was not behaving properly.

Will need to re-do test fully.

When wiping out my test OBS and re-using the one from this PR built. it did work as expected

BitRate27 · 2025-05-05T01:27:36Z

Looks like the one scenario I didnt list above. Will check it out. Thanks!

Trouffman · 2025-05-05T12:41:15Z

Tests results on MacOS 15.4

Scenarios :

New profile loaded with main output OFF and format NOT supported
    Turn off main output
    Main output: not started
    Dialog: Main output group disabled, not checked and error shown

-> OK

New profile loaded with main output OFF and format supported
    Turn off main output
    Main output: not started
    Dialog: Main output group enabled, not checked and no error shown
        OK and main output checked -> Main output started
        OK and main output not checked -> Main output not started
        Cancel -> Main output not started

-> OK

New profile loaded with main output ON and format supported
    Turn off main output
    Main output: started
    Dialog: Main output group enabled, checked and no error shown

-> OK

User changes to a format NOT supported
    Dialog: Main output group disabled, not checked and error shown
    OK/Cancel: Main output not started

-> OK

User changes to a format supported
    Dialog: Main output group enabled, not checked and no error shown
        OK and main output checked -> Main output started
        OK and main output not checked -> Main output not started
        Cancel -> Main output not started

->ok but UI space for error not reverted

Trouffman · 2025-05-05T13:16:29Z

Tests results on Windows 11

Scenarios :

New profile loaded with main output OFF and format NOT supported
    Turn off main output
    Main output: not started
    Dialog: Main output group disabled, not checked and error shown

-> OK

New profile loaded with main output OFF and format supported
    Turn off main output
    Main output: not started
    Dialog: Main output group enabled, not checked and no error shown
        OK and main output checked -> Main output started
        OK and main output not checked -> Main output not started
        Cancel -> Main output not started

-> OK

New profile loaded with main output ON and format supported
    Turn off main output
    Main output: started
    Dialog: Main output group enabled, checked and no error shown

-> OK

User changes to a format NOT supported
    Dialog: Main output group disabled, not checked and error shown
    OK/Cancel: Main output not started

-> OK

User changes to a format supported
    Dialog: Main output group enabled, not checked and no error shown
        OK and main output checked -> Main output started
        OK and main output not checked -> Main output not started
        Cancel -> Main output not started

->ok

Trouffman · 2025-05-05T19:16:04Z

Tests results on Ubuntu 24.04

Scenarios :

New profile loaded with main output OFF and format NOT supported
    Turn off main output
    Main output: not started
    Dialog: Main output group disabled, not checked and error shown

-> OK

New profile loaded with main output OFF and format supported
    Turn off main output
    Main output: not started
    Dialog: Main output group enabled, not checked and no error shown
        OK and main output checked -> Main output started
        OK and main output not checked -> Main output not started
        Cancel -> Main output not started

-> OK

New profile loaded with main output ON and format supported
    Turn off main output
    Main output: started
    Dialog: Main output group enabled, checked and no error shown

-> OK

User changes to a format NOT supported
    Dialog: Main output group disabled, not checked and error shown
    OK/Cancel: Main output not started

-> OK

User changes to a format supported
    Dialog: Main output group enabled, not checked and no error shown
        OK and main output checked -> Main output started
        OK and main output not checked -> Main output not started
        Cancel -> Main output not started

->ok

Where I have found an issue :

New profile loaded with main output ON and format NOT-supported
    Turn off main output
    Main output is still checked (as it was loaded as "active") BUT this is not running.
    User change Settings to a Supported pixel format
    The Main Output should restart, if the format is supported

Not sure when/where this is overlooked.

BitRate27 · 2025-05-06T15:37:03Z

New profile loaded with main output ON and format NOT-supported
    Turn off main output (?)
    Main output is still checked (as it was loaded as "active") BUT this is not running.
    User change Settings to a Supported pixel format
    The Main Output should restart, if the format is supported

(?) How do you turn off main output with this new code if the format is not supported? The checkbox should not be sensitive.

Note: I could do this with this sequence:
Select a supported profile
Turn on main output
OK Output settings dialog
Bring up Output settings dialog
Change to an unsupported profile
Turn off main output
OK Output settings dialog

Operates properly (main output terminated)

Trouffman · 2025-05-07T00:17:02Z

I could reproduce on Mac with this :

Open OBS with supported profile & Main Output ON
Load non-compliant profile
Behavior (expected) : Main output OFF, greyed out option, box is still ticked.
Go to Settings : change to a valid pixel format, save
Behavior : Main output OFF, option available, box is still ticked.

To re-enable the NDI output, the Main Output option must be disabled, saved, then re-enabled in the DistroAV setings

I think this need to be clear-er on the approach we take:

Should the option be "disabled" if a Pixel format is not supported
or
Should we build a (more complex) logic to re-enable the Main Output when the pixel format is changed AND the box ticked.

Option 1. would touch the configuration file
Option 2. would need to have a check for specifically this change as we do NOT want to stop the main output everytime we go in the OBS settings.

Trouffman · 2025-05-07T02:00:06Z

@BitRate27 This PR is proving to be more complex than expected.

While doing the review I found a couple extra comment (not blocking but would be good to have at some point)

Should we call main_output_is_supported() everytime we plan to use main_output_init ?

ie. : in plugin-main.cpp line 299

main_output_is_supported() is calling the obs_output_create with an "empty" ndi_name.

This did not create issues when testing, hopefully this would stay this way but having a clear "testing name" might help to identify in error log as well.

We could put it in a group called "DistroAV Config" so this does not even show up anywhere when testing.

Following on point 2. above : Giving the output a unique name might avoid weird issues too (even tho the OBS documentation says that it will make it unique).

This PR brings a lot of value as it will help for the support and anyone using HDR pixel format.

BitRate27 · 2025-05-07T15:34:50Z

Disabling main output in the config if the format is not supported, keeps the toggle in sync with the main output status in OBS.

Also, moving the check for support into the main_output_init allows us to turn it off if not supported and enabled.

Finally, with the output enabled in sync with the OBS main output state, main_output_init is only called when settings of main output are changed.

I verified the original scenarios still work, and the new scenario performs as it should. Also added a LOG_INFO message to let the user know the output was disabled because the format is not supported.

…s not supported.

Trouffman · 2025-05-07T16:12:55Z

Great! Thank you.

I updated the INFO to a WARNING and attributed ID 426 to easily flag it too.

Show last output error and fix crash when changing format

b14133d

Clang and change log message when creating output

611151b

RayneYoruka mentioned this pull request Nov 19, 2024

[RFE]: Support P010 and Rec. 2100(HLG) ("warning: unsupported pixel format 18") #1150

Open

Trouffman reviewed Dec 4, 2024

View reviewed changes

src/main-output.cpp Outdated Show resolved Hide resolved

BitRate27 added 3 commits December 6, 2024 16:14

Move last error out of config and add translations for error

bb765a9

Make error_string variable name snake case

abfd878

Clang format for main-output

8c057c2

Only allow selection of main output if pixel format supported

ef7e0ec

BitRate27 mentioned this pull request Dec 13, 2024

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

Closed

Merge branch 'master' into badpixel

5832c82

Merge branch 'master' into badpixel

0257daa

Trouffman added Seeking Testers PRs with this label will package the plugin so that others can test Output (Program/Preview) relate to NDI Output Progam or Preview HDR & Pixel Format labels Dec 30, 2024

Trouffman added 2 commits January 3, 2025 19:22

Merge branch 'master' into badpixel

bbb010a

Merge branch 'master' into badpixel

1b0e2bd

Trouffman mentioned this pull request Jan 4, 2025

[Dev-Bug]: Opening the Plugin Settings Dialog stops NDI Output on PR 1148 #1180

Closed

7 tasks

BitRate27 mentioned this pull request Feb 9, 2025

[Bug]: Enabling NDI output after changing "Output (Scaled) Resolution" fails #1195

Closed

7 tasks

Merge branch 'master' into badpixel

f03ba18

clang / gersemi Formating update

da73871

Trouffman mentioned this pull request Mar 10, 2025

Recreate main output every time it is turned on #1198

Merged

Trouffman reviewed Apr 24, 2025

View reviewed changes

Merge branch 'master' into badpixel

d5e7630

BitRate27 added 2 commits April 26, 2025 16:50

Check supported with test output

e8692c6

Clang format

f2875b3

Trouffman reviewed Apr 26, 2025

View reviewed changes

src/forms/output-settings.cpp Outdated Show resolved Hide resolved

Trouffman reviewed Apr 26, 2025

View reviewed changes

src/main-output.cpp Outdated Show resolved Hide resolved

BitRate27 and others added 4 commits April 26, 2025 17:43

Always log main_output_deinit call

4fefff2

Merge branch 'master' into badpixel

562934e

minor typo correction

24fb2ec

format update

59a6122

Trouffman marked this pull request as draft May 5, 2025 00:53

Trouffman self-assigned this May 5, 2025

Trouffman marked this pull request as ready for review May 5, 2025 03:16

Merge branch 'master' into badpixel

613cb57

Disable main output in config if not supported

2b07bc7

Additional log entry and upgrade to WARNING level when pixle format i…

4c0d568

…s not supported.

Trouffman merged commit 3b87beb into DistroAV:master May 7, 2025
6 checks passed

Uh oh!

Show user why NDI output failed #1148

Show user why NDI output failed #1148

Uh oh!

Conversation

BitRate27 commented Nov 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RayneYoruka commented Nov 18, 2024

Uh oh!

Uh oh!

Trouffman commented Dec 4, 2024

Uh oh!

BitRate27 commented Dec 4, 2024

Uh oh!

BitRate27 commented Dec 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BitRate27 commented Dec 7, 2024

Uh oh!

BitRate27 commented Dec 10, 2024

Uh oh!

Trouffman commented Dec 17, 2024

Uh oh!

Trouffman commented Jan 4, 2025

Uh oh!

Trouffman commented Feb 10, 2025

Uh oh!

Trouffman left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Trouffman commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BitRate27 commented Apr 24, 2025

Uh oh!

BitRate27 commented Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Trouffman commented May 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BitRate27 commented May 5, 2025

Uh oh!

Trouffman commented May 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Trouffman commented May 5, 2025

Uh oh!

Trouffman commented May 5, 2025

Uh oh!

BitRate27 commented May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Trouffman commented May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Trouffman commented May 7, 2025

Uh oh!

BitRate27 commented May 7, 2025

Uh oh!

Trouffman commented May 7, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

BitRate27 commented Nov 18, 2024 •

edited

Loading

BitRate27 commented Dec 6, 2024 •

edited

Loading

Trouffman left a comment •

edited

Loading

Trouffman commented Apr 24, 2025 •

edited

Loading

BitRate27 commented Apr 25, 2025 •

edited

Loading

Trouffman commented May 5, 2025 •

edited

Loading

Trouffman commented May 5, 2025 •

edited

Loading

BitRate27 commented May 6, 2025 •

edited

Loading

Trouffman commented May 7, 2025 •

edited

Loading