Skip to content

decklink-output-ui: Update Decklink output to use canvases#12442

Open
cg2121 wants to merge 1 commit intoobsproject:masterfrom
cg2121:decklink-canvasses
Open

decklink-output-ui: Update Decklink output to use canvases#12442
cg2121 wants to merge 1 commit intoobsproject:masterfrom
cg2121:decklink-canvasses

Conversation

@cg2121
Copy link
Contributor

@cg2121 cg2121 commented Jul 28, 2025

Description

Canvasses were introduced in 31.1.0. This updates the Decklink output to use them.

Motivation and Context

Much less code.

How Has This Been Tested?

Tested Decklink main and preview outputs.

Types of changes

  • Code cleanup (non-breaking change which makes code smaller or more readable)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@cg2121 cg2121 added Code Cleanup Non-breaking change which makes code smaller or more readable Seeking Testers Build artifacts on CI labels Jul 28, 2025
@RytoEX RytoEX self-assigned this Jul 28, 2025
@RytoEX RytoEX added this to the OBS Studio 32.0 milestone Jul 28, 2025
@norihiro
Copy link
Contributor

norihiro commented Aug 7, 2025

Just a nitpick: The plural of the word "canvas" is "canvases", not "canvasses".

@cg2121 cg2121 changed the title decklink-output-ui: Update Decklink output to use canvasses decklink-output-ui: Update Decklink output to use canvases Aug 9, 2025
@cg2121 cg2121 force-pushed the decklink-canvasses branch from abcf75a to 81d7723 Compare August 9, 2025 02:24
@cg2121
Copy link
Contributor Author

cg2121 commented Aug 9, 2025

Updated to fix the spelling of canvases

@ipatix
Copy link

ipatix commented Nov 8, 2025

I've cherry picked this PR on v32.0.2 and it works fine for me. My initialy impressions is also that there appears to be less of a performance penalty compared to the old code. Enabling it on 1920x1080 used to cost me ~3ms render time. Now it is probably more around 1ms.

Copy link
Member

@derrod derrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a DeckLink card to test, but it does look fine to me.

Copy link
Member

@PatTheMav PatTheMav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't have the hardware to test this, so I have to rely on other users confirming that. No further comments apart from code-style changes.

@cg2121 cg2121 force-pushed the decklink-canvasses branch 2 times, most recently from b376962 to 74c2244 Compare February 23, 2026 18:41
Copy link
Member

@PatTheMav PatTheMav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm missing something, but as far as I can tell on_preview_scene_changed was originally used with obs_frontend_remove_event_callback to register a frontend API callback.

But in the changed code I don't see any call to obs_frontend_remove_event_callback to do the same, which suggests to me that the existence of onPreviewSceneChanged is now superfluous?

Is the corresponding use case (handling a change of the preview scene) now handled implicitly?

Canvases were introduced in 31.1.0. This updates the Decklink
output to use them.
@cg2121 cg2121 force-pushed the decklink-canvasses branch from 74c2244 to 50f63e7 Compare February 26, 2026 05:22
@cg2121
Copy link
Contributor Author

cg2121 commented Feb 26, 2026

Updated to fix the missing event callback functions.

@ipatix
Copy link

ipatix commented Feb 27, 2026

If there were bugs regarding the preview, I guess I never tested it with my DeckLink card (I only tested program output). I can offer testing it, but if @cg2121 has one, it probably doesn't make much sense if I do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Code Cleanup Non-breaking change which makes code smaller or more readable Seeking Testers Build artifacts on CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants